Skip to content

Commit ed3edd4

Browse files
pimterrypimterry
committed
http2: add strictSingleValueFields option to relax header validation
Previously it was impossible to send multiple values for any header or trailer defined officially as supporting only a single value. This is a good default, but in practice many of these headers are used in weird & wonderful ways where this can be problematic. This new option allows for relaxing this restriction to support those cases where required. This option defaults to true so validation will still be applied as before, rejecting multiple single-value fields, unless explicitly disabled.
1 parent f6ea5bf commit ed3edd4

File tree

8 files changed

+208
-44
lines changed

8 files changed

+208
-44
lines changed

‎doc/api/http2.md‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2792,6 +2792,9 @@ Throws `ERR_INVALID_ARG_TYPE` for invalid `settings` argument.
27922792
<!-- YAML
27932793
added: v8.4.0
27942794
changes:
2795+
- version: REPLACEME
2796+
pr-url: https://github.com/nodejs/node/pull/59917
2797+
description: Added the `strictSingleValueFields` option.
27952798
- version:
27962799
- v23.0.0
27972800
- v22.10.0
@@ -2929,6 +2932,10 @@ changes:
29292932
and trailing whitespace validation for HTTP/2 header field names and values
29302933
as per [RFC-9113](https://www.rfc-editor.org/rfc/rfc9113.html#section-8.2.1).
29312934
**Default:** `true`.
2935+
* `strictSingleValueFields` {boolean} If `true`, strict validation is used
2936+
for headers and trailers defined as having only a single value, such that
2937+
an error is thrown if multiple values are provided.
2938+
**Default:** `true`.
29322939
* `...options` {Object} Any [`net.createServer()`][] option can be provided.
29332940
* `onRequestHandler` {Function} See [Compatibility API][]
29342941
* Returns: {Http2Server}
@@ -2986,6 +2993,9 @@ server.listen(8000);
29862993
<!-- YAML
29872994
added: v8.4.0
29882995
changes:
2996+
- version: REPLACEME
2997+
pr-url: https://github.com/nodejs/node/pull/59917
2998+
description: Added the `strictSingleValueFields` option.
29892999
- version:
29903000
- v15.10.0
29913001
- v14.16.0
@@ -3104,6 +3114,10 @@ changes:
31043114
and trailing whitespace validation for HTTP/2 header field names and values
31053115
as per [RFC-9113](https://www.rfc-editor.org/rfc/rfc9113.html#section-8.2.1).
31063116
**Default:** `true`.
3117+
* `strictSingleValueFields` {boolean} If `true`, strict validation is used
3118+
for headers and trailers defined as having only a single value, such that
3119+
an error is thrown if multiple values are provided.
3120+
**Default:** `true`.
31073121
* `onRequestHandler` {Function} See [Compatibility API][]
31083122
* Returns: {Http2SecureServer}
31093123

‎lib/internal/http2/core.js‎

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ const {
132132
const {
133133
assertIsObject,
134134
assertIsArray,
135+
assertValidPseudoHeader,
135136
assertValidPseudoHeaderResponse,
136137
assertValidPseudoHeaderTrailer,
137138
assertWithinRange,
@@ -144,6 +145,7 @@ const {
144145
isPayloadMeaningless,
145146
kAuthority,
146147
kSensitiveHeaders,
148+
kStrictSingleValueFields,
147149
kSocket,
148150
kRequest,
149151
kProtocol,
@@ -1311,6 +1313,8 @@ class Http2Session extends EventEmitter {
13111313
this[kSocket] = socket;
13121314
this[kTimeout] = null;
13131315
this[kHandle] = undefined;
1316+
this[kStrictSingleValueFields] =
1317+
options.strictSingleValueFields;
13141318

13151319
// Do not use nagle's algorithm
13161320
if (typeof socket.setNoDelay === 'function')
@@ -2350,7 +2354,11 @@ class Http2Stream extends Duplex {
23502354

23512355
this[kUpdateTimer]();
23522356

2353-
const headersList = buildNgHeaderString(headers, assertValidPseudoHeaderTrailer);
2357+
const headersList = buildNgHeaderString(
2358+
headers,
2359+
assertValidPseudoHeaderTrailer,
2360+
this.session[kStrictSingleValueFields],
2361+
);
23542362
this[kSentTrailers] = headers;
23552363

23562364
// Send the trailers in setImmediate so we don't do it on nghttp2 stack.
@@ -2559,7 +2567,11 @@ function prepareResponseHeaders(stream, headersParam, options) {
25592567
stream[kSentHeaders] = headers;
25602568
}
25612569

2562-
const headersList = buildNgHeaderString(headers, assertValidPseudoHeaderResponse);
2570+
const headersList = buildNgHeaderString(
2571+
headers,
2572+
assertValidPseudoHeaderResponse,
2573+
stream.session[kStrictSingleValueFields],
2574+
);
25632575

25642576
return { headers, headersList, statusCode };
25652577
}
@@ -2662,7 +2674,11 @@ function processRespondWithFD(self, fd, headers, offset = 0, length = -1,
26622674

26632675
let headersList;
26642676
try {
2665-
headersList = buildNgHeaderString(headers, assertValidPseudoHeaderResponse);
2677+
headersList = buildNgHeaderString(
2678+
headers,
2679+
assertValidPseudoHeaderResponse,
2680+
self.session[kStrictSingleValueFields],
2681+
);
26662682
} catch (err) {
26672683
self.destroy(err);
26682684
return;
@@ -2886,7 +2902,11 @@ class ServerHttp2Stream extends Http2Stream {
28862902
if (headers[HTTP2_HEADER_METHOD] === HTTP2_METHOD_HEAD)
28872903
headRequest = options.endStream = true;
28882904

2889-
const headersList = buildNgHeaderString(headers);
2905+
const headersList = buildNgHeaderString(
2906+
headers,
2907+
assertValidPseudoHeader,
2908+
this.session[kStrictSingleValueFields],
2909+
);
28902910

28912911
const streamOptions = options.endStream ? STREAM_OPTION_EMPTY_PAYLOAD : 0;
28922912

@@ -3150,7 +3170,11 @@ class ServerHttp2Stream extends Http2Stream {
31503170

31513171
this[kUpdateTimer]();
31523172

3153-
const headersList = buildNgHeaderString(headers, assertValidPseudoHeaderResponse);
3173+
const headersList = buildNgHeaderString(
3174+
headers,
3175+
assertValidPseudoHeaderResponse,
3176+
this.session[kStrictSingleValueFields],
3177+
);
31543178
if (!this[kInfoHeaders])
31553179
this[kInfoHeaders] = [headers];
31563180
else
@@ -3305,21 +3329,30 @@ function initializeOptions(options) {
33053329
}
33063330

33073331
if (options.maxSessionInvalidFrames !== undefined)
3308-
validateUint32(options.maxSessionInvalidFrames, 'maxSessionInvalidFrames');
3332+
validateUint32(options.maxSessionInvalidFrames, 'options.maxSessionInvalidFrames');
33093333

33103334
if (options.maxSessionRejectedStreams !== undefined) {
33113335
validateUint32(
33123336
options.maxSessionRejectedStreams,
3313-
'maxSessionRejectedStreams',
3337+
'options.maxSessionRejectedStreams',
33143338
);
33153339
}
33163340

33173341
if (options.unknownProtocolTimeout !== undefined)
3318-
validateUint32(options.unknownProtocolTimeout, 'unknownProtocolTimeout');
3342+
validateUint32(options.unknownProtocolTimeout, 'options.unknownProtocolTimeout');
33193343
else
33203344
// TODO(danbev): is this a good default value?
33213345
options.unknownProtocolTimeout = 10000;
33223346

3347+
if (options.strictSingleValueFields !== undefined) {
3348+
validateBoolean(
3349+
options.strictSingleValueFields,
3350+
'options.strictSingleValueFields',
3351+
);
3352+
} else {
3353+
options.strictSingleValueFields = true;
3354+
}
3355+
33233356

33243357
// Used only with allowHTTP1
33253358
options.Http1IncomingMessage ||= http.IncomingMessage;
@@ -3522,6 +3555,15 @@ function connect(authority, options, listener) {
35223555
throw new ERR_HTTP2_TOO_MANY_CUSTOM_SETTINGS();
35233556
}
35243557

3558+
if (options.strictSingleValueFields !== undefined) {
3559+
validateBoolean(
3560+
options.strictSingleValueFields,
3561+
'options.strictSingleValueFields',
3562+
);
3563+
} else {
3564+
options.strictSingleValueFields = true;
3565+
}
3566+
35253567
if (typeof authority === 'string')
35263568
authority = new URL(authority);
35273569

‎lib/internal/http2/util.js‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ const {
3939

4040
const kAuthority = Symbol('authority');
4141
const kSensitiveHeaders = Symbol('sensitiveHeaders');
42+
const kStrictSingleValueFields = Symbol('strictSingleValueFields');
4243
const kSocket = Symbol('socket');
4344
const kProtocol = Symbol('protocol');
4445
const kProxySocket = Symbol('proxySocket');
@@ -121,7 +122,7 @@ const kValidPseudoHeaders = new SafeSet([
121122

122123
// This set contains headers that are permitted to have only a single
123124
// value. Multiple instances must not be specified.
124-
const kSingleValueHeaders = new SafeSet([
125+
const kSingleValueFields = new SafeSet([
125126
HTTP2_HEADER_STATUS,
126127
HTTP2_HEADER_METHOD,
127128
HTTP2_HEADER_AUTHORITY,
@@ -690,6 +691,7 @@ function prepareRequestHeadersArray(headers, session) {
690691
const headersList = buildNgHeaderString(
691692
rawHeaders,
692693
assertValidPseudoHeader,
694+
session[kStrictSingleValueFields],
693695
);
694696

695697
return {
@@ -731,7 +733,11 @@ function prepareRequestHeadersObject(headers, session) {
731733
throw new ERR_HTTP2_CONNECT_PATH();
732734
}
733735

734-
const headersList = buildNgHeaderString(headersObject);
736+
const headersList = buildNgHeaderString(
737+
headersObject,
738+
assertValidPseudoHeader,
739+
session[kStrictSingleValueFields],
740+
);
735741

736742
return {
737743
headersObject,
@@ -751,10 +757,15 @@ const kNoHeaderFlags = StringFromCharCode(NGHTTP2_NV_FLAG_NONE);
751757
* format, rejecting illegal header configurations, and marking sensitive headers
752758
* that should not be indexed en route. This takes either a flat map of
753759
* raw headers ([k1, v1, k2, v2]) or a header object ({ k1: v1, k2: [v2, v3] }).
760+
*
761+
* Takes a validation function to check the pseudo-headers allowed for this
762+
* message, and a boolean indicating whether to enforce strict single-value
763+
* header validation.
754764
* @returns {[string, number]}
755765
*/
756766
function buildNgHeaderString(arrayOrMap,
757-
assertValuePseudoHeader = assertValidPseudoHeader) {
767+
validatePseudoHeaderValue,
768+
strictSingleValueFields) {
758769
let headers = '';
759770
let pseudoHeaders = '';
760771
let count = 0;
@@ -765,7 +776,8 @@ function buildNgHeaderString(arrayOrMap,
765776

766777
function processHeader(key, value) {
767778
key = key.toLowerCase();
768-
const isSingleValueHeader = kSingleValueHeaders.has(key);
779+
const isStrictSingleValueField = strictSingleValueFields &&
780+
kSingleValueFields.has(key);
769781
let isArray = ArrayIsArray(value);
770782
if (isArray) {
771783
switch (value.length) {
@@ -776,13 +788,13 @@ function buildNgHeaderString(arrayOrMap,
776788
isArray = false;
777789
break;
778790
default:
779-
if (isSingleValueHeader)
791+
if (isStrictSingleValueField)
780792
throw new ERR_HTTP2_HEADER_SINGLE_VALUE(key);
781793
}
782794
} else {
783795
value = String(value);
784796
}
785-
if (isSingleValueHeader) {
797+
if (isStrictSingleValueField) {
786798
if (singles.has(key))
787799
throw new ERR_HTTP2_HEADER_SINGLE_VALUE(key);
788800
singles.add(key);
@@ -791,7 +803,7 @@ function buildNgHeaderString(arrayOrMap,
791803
kNeverIndexFlag :
792804
kNoHeaderFlags;
793805
if (key[0] === ':') {
794-
const err = assertValuePseudoHeader(key);
806+
const err = validatePseudoHeaderValue(key);
795807
if (err !== undefined)
796808
throw err;
797809
pseudoHeaders += `${key}\0${value}\0${flags}`;
@@ -893,7 +905,7 @@ function toHeaderObject(headers, sensitiveHeaders) {
893905
const existing = obj[name];
894906
if (existing === undefined) {
895907
obj[name] = name === HTTP2_HEADER_SET_COOKIE ? [value] : value;
896-
} else if (!kSingleValueHeaders.has(name)) {
908+
} else if (!kSingleValueFields.has(name)) {
897909
switch (name) {
898910
case HTTP2_HEADER_COOKIE:
899911
// https://tools.ietf.org/html/rfc7540#section-8.1.2.5
@@ -970,6 +982,7 @@ module.exports = {
970982
isPayloadMeaningless,
971983
kAuthority,
972984
kSensitiveHeaders,
985+
kStrictSingleValueFields,
973986
kSocket,
974987
kProtocol,
975988
kProxySocket,

‎lib/internal/quic/quic.js‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ const {
109109

110110
const {
111111
buildNgHeaderString,
112+
assertValidPseudoHeader,
112113
} = require('internal/http2/util');
113114

114115
const kEmptyObject = { __proto__: null };
@@ -783,8 +784,13 @@ class QuicStream {
783784
} else {
784785
debug(`stream ${this.id} sending headers`, headers);
785786
}
787+
const headerString = buildNgHeaderString(
788+
headers,
789+
assertValidPseudoHeader,
790+
true, // This could become an option in future
791+
);
786792
// TODO(@jasnell): Support differentiating between early headers, primary headers, etc
787-
return this.#handle.sendHeaders(1, buildNgHeaderString(headers), 1);
793+
return this.#handle.sendHeaders(1, headerString, 1);
788794
}
789795

790796
[kFinishClose](error) {

‎test/parallel/test-http2-single-headers-validation-disabled.js‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
const assert = require('assert');
7+
const http2 = require('http2');
8+
9+
const server = http2.createServer({
10+
strictSingleValueFields: false
11+
});
12+
13+
server.on('stream', common.mustCall((stream, _headers, _flags, rawHeaders) => {
14+
assert.deepStrictEqual(rawHeaders.slice(8), [
15+
'user-agent', 'abc',
16+
'user-agent', 'xyz',
17+
'referer', 'qwe',
18+
'referer', 'asd',
19+
]);
20+
21+
stream.respond({
22+
':status': 200,
23+
'expires': 'Thu, 01 Jan 1970 00:00:00 GMT',
24+
'EXPIRES': 'Thu, 01 Jan 1970 00:00:00 GMT',
25+
'content-type': ['a', 'b'],
26+
});
27+
28+
stream.end();
29+
}));
30+
31+
server.listen(0, common.mustCall(() => {
32+
const client = http2.connect(`http://localhost:${server.address().port}`, {
33+
strictSingleValueFields: false
34+
});
35+
36+
const res = client.request({
37+
'user-agent': 'abc',
38+
'USER-AGENT': 'xyz',
39+
'referer': ['qwe', 'asd'],
40+
});
41+
42+
res.on('response', common.mustCall((_headers, _flags, rawHeaders) => {
43+
assert.deepStrictEqual(rawHeaders.slice(2, 10), [
44+
'expires', 'Thu, 01 Jan 1970 00:00:00 GMT',
45+
'expires', 'Thu, 01 Jan 1970 00:00:00 GMT',
46+
'content-type', 'a',
47+
'content-type', 'b',
48+
]);
49+
50+
server.close();
51+
client.close();
52+
}));
53+
}));

test/parallel/test-http2-single-headers.js renamed to test/parallel/test-http2-single-headers-validation.js

File renamed without changes.

‎test/parallel/test-http2-util-assert-valid-pseudoheader.js‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,16 @@
44
require('../common');
55
const assert = require('assert');
66

7-
// Tests the assertValidPseudoHeader function that is used within the
8-
// buildNgHeaderString function. The assert function is not exported so we
9-
// have to test it through buildNgHeaderString
10-
11-
const { buildNgHeaderString } = require('internal/http2/util');
7+
const { assertValidPseudoHeader } = require('internal/http2/util');
128

139
// These should not throw
14-
buildNgHeaderString({ ':status': 'a' });
15-
buildNgHeaderString({ ':path': 'a' });
16-
buildNgHeaderString({ ':authority': 'a' });
17-
buildNgHeaderString({ ':scheme': 'a' });
18-
buildNgHeaderString({ ':method': 'a' });
10+
assertValidPseudoHeader(':status');
11+
assertValidPseudoHeader(':path');
12+
assertValidPseudoHeader(':authority');
13+
assertValidPseudoHeader(':scheme');
14+
assertValidPseudoHeader(':method');
1915

20-
assert.throws(() => buildNgHeaderString({ ':foo': 'a' }), {
16+
assert.throws(() => assertValidPseudoHeader(':foo'), {
2117
code: 'ERR_HTTP2_INVALID_PSEUDOHEADER',
2218
name: 'TypeError',
2319
message: '":foo" is an invalid pseudoheader or is used incorrectly'

0 commit comments

Comments
 (0)