From 9539cfa35817ea3ad61eccd2ed0572cc5c449d03 Mon Sep 17 00:00:00 2001
From: Shogun <ShogunPanda@users.noreply.github.com>
Date: Mon, 10 Jan 2022 21:50:58 +0100
Subject: [PATCH] http: add uniqueHeaders option to request and createServer

PR-URL: https://github.com/nodejs/node/pull/41397
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
---
 doc/api/http.md                             |  72 +++++++-
 lib/_http_client.js                         |   8 +-
 lib/_http_incoming.js                       |  50 ++++++
 lib/_http_outgoing.js                       |  82 +++++++++-
 lib/_http_server.js                         |   8 +-
 test/parallel/test-http-multiple-headers.js | 173 ++++++++++++++++++++
 6 files changed, 380 insertions(+), 13 deletions(-)
 create mode 100644 test/parallel/test-http-multiple-headers.js

diff --git a/doc/api/http.md b/doc/api/http.md
index 01c97ec4e671db..838cdd0e05f9be 100644
--- a/doc/api/http.md
+++ b/doc/api/http.md
@@ -2364,8 +2364,28 @@ header name:
   `last-modified`, `location`, `max-forwards`, `proxy-authorization`, `referer`,
   `retry-after`, `server`, or `user-agent` are discarded.
 * `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 ', '.
+* For duplicate `cookie` headers, the values are joined together with `; `.
+* For all other headers, the values are joined together with `, `.
+
+### `message.headersDistinct`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+* {Object}
+
+Similar to [`message.headers`][], but there is no join logic and the values are
+always arrays of strings, even for headers received just once.
+
+```js
+// Prints something like:
+//
+// { 'user-agent': ['curl/7.22.0'],
+//   host: ['127.0.0.1:8000'],
+//   accept: ['*/*'] }
+console.log(request.headersDistinct);
+```
 
 ### `message.httpVersion`
 
@@ -2499,6 +2519,18 @@ added: v0.3.0
 
 The request/response trailers object. Only populated at the `'end'` event.
 
+### `message.trailersDistinct`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+* {Object}
+
+Similar to [`message.trailers`][], but there is no join logic and the values are
+always arrays of strings, even for headers received just once.
+Only populated at the `'end'` event.
+
 ### `message.url`
 
 <!-- YAML
@@ -2596,7 +2628,7 @@ Adds HTTP trailers (headers but at the end of the message) to the message.
 Trailers will **only** be emitted if the message is chunked encoded. If not,
 the trailers will be silently discarded.
 
-HTTP requires the  `Trailer` header to be sent to emit trailers,
+HTTP requires the `Trailer` header to be sent to emit trailers,
 with a list of header field names in its value, e.g.
 
 ```js
@@ -2610,6 +2642,28 @@ message.end();
 Attempting to set a header field name or value that contains invalid characters
 will result in a `TypeError` being thrown.
 
+### `outgoingMessage.appendHeader(name, value)`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+* `name` {string} Header name
+* `value` {string|string\[]} Header value
+* Returns: {this}
+
+Append a single header value for the header object.
+
+If the value is an array, this is equivalent of calling this method multiple
+times.
+
+If there were no previous value for the header, this is equivalent of calling
+[`outgoingMessage.setHeader(name, value)`][].
+
+Depending of the value of `options.uniqueHeaders` when the client request or the
+server were created, this will end up in the header being sent multiple times or
+a single time with values joined using `; `.
+
 ### `outgoingMessage.connection`
 
 <!-- YAML
@@ -3026,6 +3080,9 @@ changes:
   * `keepAliveInitialDelay` {number} If set to a positive number, it sets the
     initial delay before the first keepalive probe is sent on an idle socket.
     **Default:** `0`.
+  * `uniqueHeaders` {Array} A list of response headers that should be sent only
+    once. If the header's value is an array, the items will be joined
+    using `; `.
 
 * `requestListener` {Function}
 
@@ -3260,12 +3317,15 @@ changes:
   * `protocol` {string} Protocol to use. **Default:** `'http:'`.
   * `setHost` {boolean}: Specifies whether or not to automatically add the
     `Host` header. Defaults to `true`.
+  * `signal` {AbortSignal}: An AbortSignal that may be used to abort an ongoing
+    request.
   * `socketPath` {string} Unix domain socket. Cannot be used if one of `host`
     or `port` is specified, as those specify a TCP Socket.
   * `timeout` {number}: A number specifying the socket timeout in milliseconds.
     This will set the timeout before the socket is connected.
-  * `signal` {AbortSignal}: An AbortSignal that may be used to abort an ongoing
-    request.
+  * `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 `; `.
 * `callback` {Function}
 * Returns: {http.ClientRequest}
 
@@ -3571,11 +3631,13 @@ try {
 [`http.request()`]: #httprequestoptions-callback
 [`message.headers`]: #messageheaders
 [`message.socket`]: #messagesocket
+[`message.trailers`]: #messagetrailers
 [`net.Server.close()`]: net.md#serverclosecallback
 [`net.Server`]: net.md#class-netserver
 [`net.Socket`]: net.md#class-netsocket
 [`net.createConnection()`]: net.md#netcreateconnectionoptions-connectlistener
 [`new URL()`]: url.md#new-urlinput-base
+[`outgoingMessage.setHeader(name, value)`]: #outgoingmessagesetheadername-value
 [`outgoingMessage.socket`]: #outgoingmessagesocket
 [`removeHeader(name)`]: #requestremoveheadername
 [`request.destroy()`]: #requestdestroyerror
diff --git a/lib/_http_client.js b/lib/_http_client.js
index a4f7a255a99671..41f183aa52497a 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -52,7 +52,11 @@ const {
   isLenient,
   prepareError,
 } = require('_http_common');
-const { OutgoingMessage } = require('_http_outgoing');
+const {
+  kUniqueHeaders,
+  parseUniqueHeadersOption,
+  OutgoingMessage
+} = require('_http_outgoing');
 const Agent = require('_http_agent');
 const { Buffer } = require('buffer');
 const { defaultTriggerAsyncIdScope } = require('internal/async_hooks');
@@ -300,6 +304,8 @@ function ClientRequest(input, options, cb) {
                       options.headers);
   }
 
+  this[kUniqueHeaders] = parseUniqueHeadersOption(options.uniqueHeaders);
+
   let optsWithoutSignal = options;
   if (optsWithoutSignal.signal) {
     optsWithoutSignal = ObjectAssign({}, options);
diff --git a/lib/_http_incoming.js b/lib/_http_incoming.js
index 34de1c559c1a9f..f1e6b7f54ff7d7 100644
--- a/lib/_http_incoming.js
+++ b/lib/_http_incoming.js
@@ -33,8 +33,10 @@ const {
 const { Readable, finished } = require('stream');
 
 const kHeaders = Symbol('kHeaders');
+const kHeadersDistinct = Symbol('kHeadersDistinct');
 const kHeadersCount = Symbol('kHeadersCount');
 const kTrailers = Symbol('kTrailers');
+const kTrailersDistinct = Symbol('kTrailersDistinct');
 const kTrailersCount = Symbol('kTrailersCount');
 
 function readStart(socket) {
@@ -123,6 +125,25 @@ ObjectDefineProperty(IncomingMessage.prototype, 'headers', {
   }
 });
 
+ObjectDefineProperty(IncomingMessage.prototype, 'headersDistinct', {
+  get: function() {
+    if (!this[kHeadersDistinct]) {
+      this[kHeadersDistinct] = {};
+
+      const src = this.rawHeaders;
+      const dst = this[kHeadersDistinct];
+
+      for (let n = 0; n < this[kHeadersCount]; n += 2) {
+        this._addHeaderLineDistinct(src[n + 0], src[n + 1], dst);
+      }
+    }
+    return this[kHeadersDistinct];
+  },
+  set: function(val) {
+    this[kHeadersDistinct] = val;
+  }
+});
+
 ObjectDefineProperty(IncomingMessage.prototype, 'trailers', {
   get: function() {
     if (!this[kTrailers]) {
@@ -142,6 +163,25 @@ ObjectDefineProperty(IncomingMessage.prototype, 'trailers', {
   }
 });
 
+ObjectDefineProperty(IncomingMessage.prototype, 'trailersDistinct', {
+  get: function() {
+    if (!this[kTrailersDistinct]) {
+      this[kTrailersDistinct] = {};
+
+      const src = this.rawTrailers;
+      const dst = this[kTrailersDistinct];
+
+      for (let n = 0; n < this[kTrailersCount]; n += 2) {
+        this._addHeaderLineDistinct(src[n + 0], src[n + 1], dst);
+      }
+    }
+    return this[kTrailersDistinct];
+  },
+  set: function(val) {
+    this[kTrailersDistinct] = val;
+  }
+});
+
 IncomingMessage.prototype.setTimeout = function setTimeout(msecs, callback) {
   if (callback)
     this.on('timeout', callback);
@@ -361,6 +401,16 @@ function _addHeaderLine(field, value, dest) {
   }
 }
 
+IncomingMessage.prototype._addHeaderLineDistinct = _addHeaderLineDistinct;
+function _addHeaderLineDistinct(field, value, dest) {
+  field = StringPrototypeToLowerCase(field);
+  if (!dest[field]) {
+    dest[field] = [value];
+  } else {
+    dest[field].push(value);
+  }
+}
+
 
 // Call this instead of resume() if we want to just
 // dump all the data to /dev/null
diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js
index 71f6e3e72376a5..8b2d24bbf694e6 100644
--- a/lib/_http_outgoing.js
+++ b/lib/_http_outgoing.js
@@ -34,6 +34,7 @@ const {
   ObjectPrototypeHasOwnProperty,
   ObjectSetPrototypeOf,
   RegExpPrototypeTest,
+  SafeSet,
   StringPrototypeToLowerCase,
   Symbol,
 } = primordials;
@@ -82,6 +83,7 @@ let debug = require('internal/util/debuglog').debuglog('http', (fn) => {
 const HIGH_WATER_MARK = getDefaultHighWaterMark();
 
 const kCorked = Symbol('corked');
+const kUniqueHeaders = Symbol('kUniqueHeaders');
 
 const nop = () => {};
 
@@ -502,7 +504,10 @@ function processHeader(self, state, key, value, validate) {
   if (validate)
     validateHeaderName(key);
   if (ArrayIsArray(value)) {
-    if (value.length < 2 || !isCookieField(key)) {
+    if (
+      (value.length < 2 || !isCookieField(key)) &&
+      (!self[kUniqueHeaders] || !self[kUniqueHeaders].has(StringPrototypeToLowerCase(key)))
+    ) {
       // Retain for(;;) loop for performance reasons
       // Refs: https://github.com/nodejs/node/pull/30958
       for (let i = 0; i < value.length; i++)
@@ -571,6 +576,20 @@ const validateHeaderValue = hideStackFrames((name, value) => {
   }
 });
 
+function parseUniqueHeadersOption(headers) {
+  if (!ArrayIsArray(headers)) {
+    return null;
+  }
+
+  const unique = new SafeSet();
+  const l = headers.length;
+  for (let i = 0; i < l; i++) {
+    unique.add(StringPrototypeToLowerCase(headers[i]));
+  }
+
+  return unique;
+}
+
 OutgoingMessage.prototype.setHeader = function setHeader(name, value) {
   if (this._header) {
     throw new ERR_HTTP_HEADERS_SENT('set');
@@ -586,6 +605,36 @@ OutgoingMessage.prototype.setHeader = function setHeader(name, value) {
   return this;
 };
 
+OutgoingMessage.prototype.appendHeader = function appendHeader(name, value) {
+  if (this._header) {
+    throw new ERR_HTTP_HEADERS_SENT('append');
+  }
+  validateHeaderName(name);
+  validateHeaderValue(name, value);
+
+  const field = StringPrototypeToLowerCase(name);
+  const headers = this[kOutHeaders];
+  if (headers === null || !headers[field]) {
+    return this.setHeader(name, value);
+  }
+
+  // Prepare the field for appending, if required
+  if (!ArrayIsArray(headers[field][1])) {
+    headers[field][1] = [headers[field][1]];
+  }
+
+  const existingValues = headers[field][1];
+  if (ArrayIsArray(value)) {
+    for (let i = 0, length = value.length; i < length; i++) {
+      existingValues.push(value[i]);
+    }
+  } else {
+    existingValues.push(value);
+  }
+
+  return this;
+};
+
 
 OutgoingMessage.prototype.getHeader = function getHeader(name) {
   validateString(name, 'name');
@@ -797,7 +846,6 @@ function connectionCorkNT(conn) {
   conn.uncork();
 }
 
-
 OutgoingMessage.prototype.addTrailers = function addTrailers(headers) {
   this._trailer = '';
   const keys = ObjectKeys(headers);
@@ -817,11 +865,31 @@ OutgoingMessage.prototype.addTrailers = function addTrailers(headers) {
     if (typeof field !== 'string' || !field || !checkIsHttpToken(field)) {
       throw new ERR_INVALID_HTTP_TOKEN('Trailer name', field);
     }
-    if (checkInvalidHeaderChar(value)) {
-      debug('Trailer "%s" contains invalid characters', field);
-      throw new ERR_INVALID_CHAR('trailer content', field);
+
+    // Check if the field must be sent several times
+    const isArrayValue = ArrayIsArray(value);
+    if (
+      isArrayValue && value.length > 1 &&
+      (!this[kUniqueHeaders] || !this[kUniqueHeaders].has(StringPrototypeToLowerCase(field)))
+    ) {
+      for (let j = 0, l = value.length; j < l; j++) {
+        if (checkInvalidHeaderChar(value[j])) {
+          debug('Trailer "%s"[%d] contains invalid characters', field, j);
+          throw new ERR_INVALID_CHAR('trailer content', field);
+        }
+        this._trailer += field + ': ' + value[j] + '\r\n';
+      }
+    } else {
+      if (isArrayValue) {
+        value = ArrayPrototypeJoin(value, '; ');
+      }
+
+      if (checkInvalidHeaderChar(value)) {
+        debug('Trailer "%s" contains invalid characters', field);
+        throw new ERR_INVALID_CHAR('trailer content', field);
+      }
+      this._trailer += field + ': ' + value + '\r\n';
     }
-    this._trailer += field + ': ' + value + '\r\n';
   }
 };
 
@@ -997,6 +1065,8 @@ function(err, event) {
 };
 
 module.exports = {
+  kUniqueHeaders,
+  parseUniqueHeadersOption,
   validateHeaderName,
   validateHeaderValue,
   OutgoingMessage
diff --git a/lib/_http_server.js b/lib/_http_server.js
index c8dc22929bfabd..615ee4c670c352 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -47,7 +47,11 @@ const {
   prepareError,
 } = require('_http_common');
 const { ConnectionsList } = internalBinding('http_parser');
-const { OutgoingMessage } = require('_http_outgoing');
+const {
+  kUniqueHeaders,
+  parseUniqueHeadersOption,
+  OutgoingMessage
+} = require('_http_outgoing');
 const {
   kOutHeaders,
   kNeedDrain,
@@ -450,6 +454,7 @@ function Server(options, requestListener) {
   this.maxHeadersCount = null;
   this.maxRequestsPerSocket = 0;
   setupConnectionsTracking(this);
+  this[kUniqueHeaders] = parseUniqueHeadersOption(options.uniqueHeaders);
 }
 ObjectSetPrototypeOf(Server.prototype, net.Server.prototype);
 ObjectSetPrototypeOf(Server, net.Server);
@@ -916,6 +921,7 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
                                                socket, state);
 
   res.shouldKeepAlive = keepAlive;
+  res[kUniqueHeaders] = server[kUniqueHeaders];
   DTRACE_HTTP_SERVER_REQUEST(req, socket);
 
   if (onRequestStartChannel.hasSubscribers) {
diff --git a/test/parallel/test-http-multiple-headers.js b/test/parallel/test-http-multiple-headers.js
new file mode 100644
index 00000000000000..1ebd290a614b1b
--- /dev/null
+++ b/test/parallel/test-http-multiple-headers.js
@@ -0,0 +1,173 @@
+'use strict';
+
+// TODO@PI: Run all tests
+const common = require('../common');
+const assert = require('assert');
+const { createServer, request } = require('http');
+
+const server = createServer(
+  { uniqueHeaders: ['x-res-b', 'x-res-d', 'x-res-y'] },
+  common.mustCall((req, res) => {
+    const host = `127.0.0.1:${server.address().port}`;
+
+    assert.deepStrictEqual(req.rawHeaders, [
+      'connection', 'close',
+      'X-Req-a', 'eee',
+      'X-Req-a', 'fff',
+      'X-Req-a', 'ggg',
+      'X-Req-a', 'hhh',
+      'X-Req-b', 'iii; jjj; kkk; lll',
+      'Host', host,
+      'Transfer-Encoding', 'chunked',
+    ]);
+    assert.deepStrictEqual(req.headers, {
+      'connection': 'close',
+      'x-req-a': 'eee, fff, ggg, hhh',
+      'x-req-b': 'iii; jjj; kkk; lll',
+      host,
+      'transfer-encoding': 'chunked'
+    });
+    assert.deepStrictEqual(req.headersDistinct, {
+      'connection': ['close'],
+      'x-req-a': ['eee', 'fff', 'ggg', 'hhh'],
+      'x-req-b': ['iii; jjj; kkk; lll'],
+      'host': [host],
+      'transfer-encoding': ['chunked']
+    });
+
+    req.on('end', function() {
+      assert.deepStrictEqual(req.rawTrailers, [
+        'x-req-x', 'xxx',
+        'x-req-x', 'yyy',
+        'X-req-Y', 'zzz; www',
+      ]);
+      assert.deepStrictEqual(
+        req.trailers, { 'x-req-x': 'xxx, yyy', 'x-req-y': 'zzz; www' }
+      );
+      assert.deepStrictEqual(
+        req.trailersDistinct,
+        { 'x-req-x': ['xxx', 'yyy'], 'x-req-y': ['zzz; www'] }
+      );
+
+      res.setHeader('X-Res-a', 'AAA');
+      res.appendHeader('x-res-a', ['BBB', 'CCC']);
+      res.setHeader('X-Res-b', ['DDD', 'EEE']);
+      res.appendHeader('x-res-b', ['FFF', 'GGG']);
+      res.removeHeader('date');
+      res.writeHead(200, {
+        'Connection': 'close', 'x-res-c': ['HHH', 'III'],
+        'x-res-d': ['JJJ', 'KKK', 'LLL']
+      });
+      res.addTrailers({
+        'x-res-x': ['XXX', 'YYY'],
+        'X-Res-Y': ['ZZZ', 'WWW']
+      });
+      res.write('BODY');
+      res.end();
+
+      assert.deepStrictEqual(res.getHeader('X-Res-a'), ['AAA', 'BBB', 'CCC']);
+      assert.deepStrictEqual(res.getHeader('x-res-a'), ['AAA', 'BBB', 'CCC']);
+      assert.deepStrictEqual(
+        res.getHeader('x-res-b'), ['DDD', 'EEE', 'FFF', 'GGG']
+      );
+      assert.deepStrictEqual(res.getHeader('x-res-c'), ['HHH', 'III']);
+      assert.strictEqual(res.getHeader('connection'), 'close');
+      assert.deepStrictEqual(
+        res.getHeaderNames(),
+        ['x-res-a', 'x-res-b', 'connection', 'x-res-c', 'x-res-d']
+      );
+      assert.deepStrictEqual(
+        res.getRawHeaderNames(),
+        ['X-Res-a', 'X-Res-b', 'Connection', 'x-res-c', 'x-res-d']
+      );
+
+      const headers = Object.create(null);
+      Object.assign(headers, {
+        'x-res-a': [ 'AAA', 'BBB', 'CCC' ],
+        'x-res-b': [ 'DDD', 'EEE', 'FFF', 'GGG' ],
+        'connection': 'close',
+        'x-res-c': [ 'HHH', 'III' ],
+        'x-res-d': [ 'JJJ', 'KKK', 'LLL' ]
+      });
+      assert.deepStrictEqual(res.getHeaders(), headers);
+    });
+
+    req.resume();
+  }
+  ));
+
+server.listen(0, common.mustCall(() => {
+  const req = request({
+    host: '127.0.0.1',
+    port: server.address().port,
+    path: '/',
+    method: 'POST',
+    headers: {
+      'connection': 'close',
+      'x-req-a': 'aaa',
+      'X-Req-a': 'bbb',
+      'X-Req-b': ['ccc', 'ddd']
+    },
+    uniqueHeaders: ['x-req-b', 'x-req-y']
+  }, common.mustCall((res) => {
+    assert.deepStrictEqual(res.rawHeaders, [
+      'X-Res-a', 'AAA',
+      'X-Res-a', 'BBB',
+      'X-Res-a', 'CCC',
+      'X-Res-b', 'DDD; EEE; FFF; GGG',
+      'Connection', 'close',
+      'x-res-c', 'HHH',
+      'x-res-c', 'III',
+      'x-res-d', 'JJJ; KKK; LLL',
+      'Transfer-Encoding', 'chunked',
+    ]);
+    assert.deepStrictEqual(res.headers, {
+      'x-res-a': 'AAA, BBB, CCC',
+      'x-res-b': 'DDD; EEE; FFF; GGG',
+      'connection': 'close',
+      'x-res-c': 'HHH, III',
+      'x-res-d': 'JJJ; KKK; LLL',
+      'transfer-encoding': 'chunked'
+    });
+    assert.deepStrictEqual(res.headersDistinct, {
+      'x-res-a': [ 'AAA', 'BBB', 'CCC' ],
+      'x-res-b': [ 'DDD; EEE; FFF; GGG' ],
+      'connection': [ 'close' ],
+      'x-res-c': [ 'HHH', 'III' ],
+      'x-res-d': [ 'JJJ; KKK; LLL' ],
+      'transfer-encoding': [ 'chunked' ]
+    });
+
+    res.on('end', function() {
+      assert.deepStrictEqual(res.rawTrailers, [
+        'x-res-x', 'XXX',
+        'x-res-x', 'YYY',
+        'X-Res-Y', 'ZZZ; WWW',
+      ]);
+      assert.deepStrictEqual(
+        res.trailers,
+        { 'x-res-x': 'XXX, YYY', 'x-res-y': 'ZZZ; WWW' }
+      );
+      assert.deepStrictEqual(
+        res.trailersDistinct,
+        { 'x-res-x': ['XXX', 'YYY'], 'x-res-y': ['ZZZ; WWW'] }
+      );
+      server.close();
+    });
+    res.resume();
+  }));
+
+  req.setHeader('X-Req-a', ['eee', 'fff']);
+  req.appendHeader('X-req-a', ['ggg', 'hhh']);
+  req.setHeader('X-Req-b', ['iii', 'jjj']);
+  req.appendHeader('x-req-b', ['kkk', 'lll']);
+
+  req.addTrailers({
+    'x-req-x': ['xxx', 'yyy'],
+    'X-req-Y': ['zzz', 'www']
+  });
+
+  req.write('BODY');
+
+  req.end();
+}));