Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http: join all duplicate headers #45982

Merged
merged 12 commits into from
Jan 3, 2023
12 changes: 12 additions & 0 deletions doc/api/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -2451,6 +2451,9 @@ 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,
marco-ippolito marked this conversation as resolved.
Show resolved Hide resolved
use the option `joinDuplicateHeaders` in [`http.request()`][]
and [`http.createServer()`][].
marco-ippolito marked this conversation as resolved.
Show resolved Hide resolved
* `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 `, `.
Expand Down Expand Up @@ -3182,6 +3185,10 @@ changes:
a 400 (Bad Request) status code to any HTTP/1.1 request message
that lacks a Host header (as mandated by the specification).
**Default:** `true`.
* `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`.
Expand Down Expand Up @@ -3437,6 +3444,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
marco-ippolito marked this conversation as resolved.
Show resolved Hide resolved
the duplicates. See [`message.headers`][] for more information.
**Default:** `false`.
* `callback` {Function}
* Returns: {http.ClientRequest}

Expand Down Expand Up @@ -3750,6 +3761,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
Expand Down
10 changes: 9 additions & 1 deletion lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ const {
ERR_UNESCAPED_CHARACTERS
} = codes;
const {
validateInteger,
validateInteger, validateBoolean,
marco-ippolito marked this conversation as resolved.
Show resolved Hide resolved
} = require('internal/validators');
const { getTimerDuration } = require('internal/timers');
const {
Expand Down Expand Up @@ -229,6 +229,12 @@ function ClientRequest(input, options, cb) {
}
this.insecureHTTPParser = insecureHTTPParser;

if (options.joinDuplicateHeaders !== undefined) {
anonrig marked this conversation as resolved.
Show resolved Hide resolved
validateBoolean(options.joinDuplicateHeaders, 'options.joinDuplicateHeaders');
}

this.joinDuplicateHeaders = options.joinDuplicateHeaders;

this.path = options.path || '/';
if (cb) {
this.once('response', cb);
Expand Down Expand Up @@ -811,6 +817,8 @@ function tickOnSocket(req, socket) {
parser.maxHeaderPairs = req.maxHeadersCount << 1;
}

parser.joinDuplicateHeaders = req.joinDuplicateHeaders;
mcollina marked this conversation as resolved.
Show resolved Hide resolved

parser.onIncoming = parserOnIncomingClient;
socket.on('error', socketErrorListener);
socket.on('data', socketOnData);
Expand Down
3 changes: 3 additions & 0 deletions lib/_http_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ function parserOnHeadersComplete(versionMajor, versionMinor, headers, method,
incoming.httpVersionMajor = versionMajor;
incoming.httpVersionMinor = versionMinor;
incoming.httpVersion = `${versionMajor}.${versionMinor}`;
incoming.joinDuplicateHeaders = (socket && socket.server &&
marco-ippolito marked this conversation as resolved.
Show resolved Hide resolved
socket.server.joinDuplicateHeaders) ||
parser.joinDuplicateHeaders;
incoming.url = url;
incoming.upgrade = upgrade;

Expand Down
13 changes: 11 additions & 2 deletions lib/_http_incoming.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ function IncomingMessage(socket) {
this[kTrailers] = null;
this[kTrailersCount] = 0;
this.rawTrailers = [];

this.joinDuplicateHeaders = false;
this.aborted = false;

this.upgrade = null;
Expand Down Expand Up @@ -115,7 +115,6 @@ ObjectDefineProperty(IncomingMessage.prototype, 'headers', {

const src = this.rawHeaders;
const dst = this[kHeaders];

marco-ippolito marked this conversation as resolved.
Show resolved Hide resolved
for (let n = 0; n < this[kHeadersCount]; n += 2) {
this._addHeaderLine(src[n + 0], src[n + 1], dst);
}
Expand Down Expand Up @@ -400,6 +399,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 (typeof dest[field] === 'string') {
marco-ippolito marked this conversation as resolved.
Show resolved Hide resolved
dest[field] += ', ' + value;
marco-ippolito marked this conversation as resolved.
Show resolved Hide resolved
} else {
dest[field] = value;
}
} else if (dest[field] === undefined) {
// Drop duplicates
dest[field] = value;
Expand Down
6 changes: 6 additions & 0 deletions lib/_http_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,12 @@ function storeHTTPOptions(options) {
} else {
this.requireHostHeader = true;
}

const joinDuplicateHeaders = options.joinDuplicateHeaders;
if (joinDuplicateHeaders !== undefined) {
anonrig marked this conversation as resolved.
Show resolved Hide resolved
validateBoolean(joinDuplicateHeaders, 'options.joinDuplicateHeaders');
}
this.joinDuplicateHeaders = joinDuplicateHeaders;
}

function setupConnectionsTracking(server) {
Expand Down
3 changes: 2 additions & 1 deletion lib/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ let maxHeaderSize;
* ServerResponse?: ServerResponse;
* insecureHTTPParser?: boolean;
* maxHeaderSize?: number;
* requireHostHeader?: boolean
* requireHostHeader?: boolean;
* joinDuplicateHeaders?: boolean;
* }} [opts]
* @param {Function} [requestListener]
* @returns {Server}
Expand Down
83 changes: 83 additions & 0 deletions test/parallel/test-http-request-join-authorization-headers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const http = require('http');

{
const server = http.createServer({
requireHostHeader: false,
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({
requireHostHeader: false,
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();
}));
});
}));
}
marco-ippolito marked this conversation as resolved.
Show resolved Hide resolved

{
// Client joinDuplicateHeaders false
const server = http.createServer({
requireHostHeader: false,
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();
}));
});
}));
}