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
20 changes: 20 additions & 0 deletions doc/api/http.md
Expand Up @@ -2422,6 +2422,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: >-
Expand Down Expand Up @@ -2451,6 +2458,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,
marco-ippolito marked this conversation as resolved.
Show resolved Hide resolved
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 `, `.
Expand Down Expand Up @@ -3182,6 +3193,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 +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}

Expand Down Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions lib/_http_client.js
Expand Up @@ -82,6 +82,7 @@ const {
} = codes;
const {
validateInteger,
validateBoolean,
} = require('internal/validators');
const { getTimerDuration } = require('internal/timers');
const {
Expand Down Expand Up @@ -229,6 +230,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 +818,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
2 changes: 2 additions & 0 deletions lib/_http_common.js
Expand Up @@ -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;

Expand Down
12 changes: 11 additions & 1 deletion lib/_http_incoming.js
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 @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions lib/_http_server.js
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
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
@@ -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();
}));
});
}));
}