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

[v19.x backport] http: join authorization headers #46240

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions doc/api/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need to update this PR URL to be 46240 or add it as a second pr-url entry? Or do we not do that for backports?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 @@ -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 `, `.
Expand Down Expand Up @@ -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`.
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
Original file line number Diff line number Diff line change
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) {
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;

parser.onIncoming = parserOnIncomingClient;
socket.on('error', socketErrorListener);
socket.on('data', socketOnData);
Expand Down
2 changes: 2 additions & 0 deletions lib/_http_common.js
Original file line number Diff line number Diff line change
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
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 @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 1 addition & 0 deletions lib/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ let maxHeaderSize;
* ServerResponse?: ServerResponse;
* insecureHTTPParser?: boolean;
* maxHeaderSize?: number;
* joinDuplicateHeaders?: boolean;
* }} [opts]
* @param {Function} [requestListener]
* @returns {Server}
Expand Down
80 changes: 80 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,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();
}));
});
}));
}