http: join authorization headers

PR-URL: nodejs#45982 Fixes: nodejs#45699 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Paolo Insogna <[email protected]>
RafaelGSS · Jan 17, 2023 · 13410d0 · 13410d0
1 parent 9bcf3ca
commit 13410d0
Show file tree

Hide file tree

Showing 7 changed files with 129 additions and 1 deletion.
diff --git a/doc/api/http.md b/doc/api/http.md
@@ -2426,6 +2426,13 @@ as an argument to any listeners on the event.
 <!-- YAML
 added: v0.1.5
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/45982
+    description: >-
+     The `joinDuplicateHeaders` option in the `http.request()`
+     and `http.createServer()` functions ensures that duplicate
+     headers are not discarded, but rather combined using a
+     comma separator, in accordance with RFC 9110 Section 5.3.
   - version: v15.1.0
     pr-url: https://github.com/nodejs/node/pull/35281
     description: >-
@@ -2455,6 +2462,10 @@ header name:
   `etag`, `expires`, `from`, `host`, `if-modified-since`, `if-unmodified-since`,
   `last-modified`, `location`, `max-forwards`, `proxy-authorization`, `referer`,
   `retry-after`, `server`, or `user-agent` are discarded.
+  To allow duplicate values of the headers listed above to be joined,
+  use the option `joinDuplicateHeaders` in [`http.request()`][]
+  and [`http.createServer()`][]. See RFC 9110 Section 5.3 for more
+  information.
 * `set-cookie` is always an array. Duplicates are added to the array.
 * For duplicate `cookie` headers, the values are joined together with `; `.
 * For all other headers, the values are joined together with `, `.
@@ -3182,6 +3193,10 @@ changes:
   * `requestTimeout`: Sets the timeout value in milliseconds for receiving
     the entire request from the client.
     See [`server.requestTimeout`][] for more information.
+  * `joinDuplicateHeaders` {boolean} It joins the field line values of multiple
+    headers in a request with `, ` instead of discarding the duplicates.
+    See [`message.headers`][] for more information.
+    **Default:** `false`.
   * `ServerResponse` {http.ServerResponse} Specifies the `ServerResponse` class
     to be used. Useful for extending the original `ServerResponse`. **Default:**
     `ServerResponse`.
@@ -3437,6 +3452,10 @@ changes:
   * `uniqueHeaders` {Array} A list of request headers that should be sent
     only once. If the header's value is an array, the items will be joined
     using `; `.
+  * `joinDuplicateHeaders` {boolean} It joins the field line values of
+    multiple headers in a request with `, ` instead of discarding
+    the duplicates. See [`message.headers`][] for more information.
+    **Default:** `false`.
 * `callback` {Function}
 * Returns: {http.ClientRequest}
 
@@ -3750,6 +3769,7 @@ Set the maximum number of idle HTTP parsers. **Default:** `1000`.
 [`http.IncomingMessage`]: #class-httpincomingmessage
 [`http.ServerResponse`]: #class-httpserverresponse
 [`http.Server`]: #class-httpserver
+[`http.createServer()`]: #httpcreateserveroptions-requestlistener
 [`http.get()`]: #httpgetoptions-callback
 [`http.globalAgent`]: #httpglobalagent
 [`http.request()`]: #httprequestoptions-callback

diff --git a/lib/_http_client.js b/lib/_http_client.js
@@ -82,6 +82,7 @@ const {
 } = codes;
 const {
   validateInteger,
+  validateBoolean,
 } = require('internal/validators');
 const { getTimerDuration } = require('internal/timers');
 const {
@@ -229,6 +230,12 @@ function ClientRequest(input, options, cb) {
   }
   this.insecureHTTPParser = insecureHTTPParser;
 
+  if (options.joinDuplicateHeaders !== undefined) {
+    validateBoolean(options.joinDuplicateHeaders, 'options.joinDuplicateHeaders');
+  }
+
+  this.joinDuplicateHeaders = options.joinDuplicateHeaders;
+
   this.path = options.path || '/';
   if (cb) {
     this.once('response', cb);
@@ -811,6 +818,8 @@ function tickOnSocket(req, socket) {
     parser.maxHeaderPairs = req.maxHeadersCount << 1;
   }
 
+  parser.joinDuplicateHeaders = req.joinDuplicateHeaders;
+
   parser.onIncoming = parserOnIncomingClient;
   socket.on('error', socketErrorListener);
   socket.on('data', socketOnData);

diff --git a/lib/_http_common.js b/lib/_http_common.js
@@ -94,6 +94,8 @@ function parserOnHeadersComplete(versionMajor, versionMinor, headers, method,
   incoming.httpVersionMajor = versionMajor;
   incoming.httpVersionMinor = versionMinor;
   incoming.httpVersion = `${versionMajor}.${versionMinor}`;
+  incoming.joinDuplicateHeaders = socket?.server?.joinDuplicateHeaders ||
+                                  parser.joinDuplicateHeaders;
   incoming.url = url;
   incoming.upgrade = upgrade;
 

diff --git a/lib/_http_incoming.js b/lib/_http_incoming.js
@@ -75,7 +75,7 @@ function IncomingMessage(socket) {
   this[kTrailers] = null;
   this[kTrailersCount] = 0;
   this.rawTrailers = [];
-
+  this.joinDuplicateHeaders = false;
   this.aborted = false;
 
   this.upgrade = null;
@@ -400,6 +400,16 @@ function _addHeaderLine(field, value, dest) {
     } else {
       dest['set-cookie'] = [value];
     }
+  } else if (this.joinDuplicateHeaders) {
+    // RFC 9110 https://www.rfc-editor.org/rfc/rfc9110#section-5.2
+    // https://github.com/nodejs/node/issues/45699
+    // allow authorization multiple fields
+    // Make a delimited list
+    if (dest[field] === undefined) {
+      dest[field] = value;
+    } else {
+      dest[field] += ', ' + value;
+    }
   } else if (dest[field] === undefined) {
     // Drop duplicates
     dest[field] = value;

diff --git a/lib/_http_server.js b/lib/_http_server.js
@@ -469,6 +469,12 @@ function storeHTTPOptions(options) {
   } else {
     this.connectionsCheckingInterval = 30_000; // 30 seconds
   }
+
+  const joinDuplicateHeaders = options.joinDuplicateHeaders;
+  if (joinDuplicateHeaders !== undefined) {
+    validateBoolean(joinDuplicateHeaders, 'options.joinDuplicateHeaders');
+  }
+  this.joinDuplicateHeaders = joinDuplicateHeaders;
 }
 
 function setupConnectionsTracking(server) {

diff --git a/lib/http.js b/lib/http.js
@@ -52,6 +52,7 @@ let maxHeaderSize;
  *   ServerResponse?: ServerResponse;
  *   insecureHTTPParser?: boolean;
  *   maxHeaderSize?: number;
+ *   joinDuplicateHeaders?: boolean;
  *   }} [opts]
  * @param {Function} [requestListener]
  * @returns {Server}

diff --git a/test/parallel/test-http-request-join-authorization-headers.js b/test/parallel/test-http-request-join-authorization-headers.js
@@ -0,0 +1,80 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const http = require('http');
+
+{
+  const server = http.createServer({
+    joinDuplicateHeaders: true
+  }, common.mustCall((req, res) => {
+    assert.strictEqual(req.headers.authorization, '1, 2');
+    assert.strictEqual(req.headers.cookie, 'foo; bar');
+    res.writeHead(200, ['authorization', '3', 'authorization', '4', 'cookie', 'foo', 'cookie', 'bar']);
+    res.end();
+  }));
+
+  server.listen(0, common.mustCall(() => {
+    http.get({
+      port: server.address().port,
+      headers: ['authorization', '1', 'authorization', '2', 'cookie', 'foo', 'cookie', 'bar'],
+      joinDuplicateHeaders: true
+    }, (res) => {
+      assert.strictEqual(res.statusCode, 200);
+      assert.strictEqual(res.headers.authorization, '3, 4');
+      assert.strictEqual(res.headers.cookie, 'foo; bar');
+      res.resume().on('end', common.mustCall(() => {
+        server.close();
+      }));
+    });
+  }));
+}
+
+{
+  // Server joinDuplicateHeaders false
+  const server = http.createServer({
+    joinDuplicateHeaders: false
+  }, common.mustCall((req, res) => {
+    assert.strictEqual(req.headers.authorization, '1'); // non joined value
+    res.writeHead(200, ['authorization', '3', 'authorization', '4']);
+    res.end();
+  }));
+
+  server.listen(0, common.mustCall(() => {
+    http.get({
+      port: server.address().port,
+      headers: ['authorization', '1', 'authorization', '2'],
+      joinDuplicateHeaders: true
+    }, (res) => {
+      assert.strictEqual(res.statusCode, 200);
+      assert.strictEqual(res.headers.authorization, '3, 4');
+      res.resume().on('end', common.mustCall(() => {
+        server.close();
+      }));
+    });
+  }));
+}
+
+{
+  // Client joinDuplicateHeaders false
+  const server = http.createServer({
+    joinDuplicateHeaders: true
+  }, common.mustCall((req, res) => {
+    assert.strictEqual(req.headers.authorization, '1, 2');
+    res.writeHead(200, ['authorization', '3', 'authorization', '4']);
+    res.end();
+  }));
+
+  server.listen(0, common.mustCall(() => {
+    http.get({
+      port: server.address().port,
+      headers: ['authorization', '1', 'authorization', '2'],
+      joinDuplicateHeaders: false
+    }, (res) => {
+      assert.strictEqual(res.statusCode, 200);
+      assert.strictEqual(res.headers.authorization, '3'); // non joined value
+      res.resume().on('end', common.mustCall(() => {
+        server.close();
+      }));
+    });
+  }));
+}