Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 8103a78

Browse filesBrowse files
pimterryaduh95
pimterry
authored and
aduh95
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. PR-URL: #59917 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
1 parent ee59127 commit 8103a78
Copy full SHA for 8103a78

8 files changed

+208-44Lines changed: 208 additions & 44 deletions
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎doc/api/http2.md‎

Copy file name to clipboardExpand all lines: doc/api/http2.md
+14Lines changed: 14 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -2793,6 +2793,9 @@ Throws `ERR_INVALID_ARG_TYPE` for invalid `settings` argument.
27932793
<!-- YAML
27942794
added: v8.4.0
27952795
changes:
2796+
- version: REPLACEME
2797+
pr-url: https://github.com/nodejs/node/pull/59917
2798+
description: Added the `strictSingleValueFields` option.
27962799
- version: REPLACEME
27972800
pr-url: https://github.com/nodejs/node/pull/61713
27982801
description: Added `http1Options` option. The `Http1IncomingMessage`
@@ -2952,6 +2955,10 @@ changes:
29522955
and trailing whitespace validation for HTTP/2 header field names and values
29532956
as per [RFC-9113](https://www.rfc-editor.org/rfc/rfc9113.html#section-8.2.1).
29542957
**Default:** `true`.
2958+
* `strictSingleValueFields` {boolean} If `true`, strict validation is used
2959+
for headers and trailers defined as having only a single value, such that
2960+
an error is thrown if multiple values are provided.
2961+
**Default:** `true`.
29552962
* `...options` {Object} Any [`net.createServer()`][] option can be provided.
29562963
* `onRequestHandler` {Function} See [Compatibility API][]
29572964
* Returns: {Http2Server}
@@ -3009,6 +3016,9 @@ server.listen(8000);
30093016
<!-- YAML
30103017
added: v8.4.0
30113018
changes:
3019+
- version: REPLACEME
3020+
pr-url: https://github.com/nodejs/node/pull/59917
3021+
description: Added the `strictSingleValueFields` option.
30123022
- version: REPLACEME
30133023
pr-url: https://github.com/nodejs/node/pull/61713
30143024
description: Added `http1Options` option.
@@ -3130,6 +3140,10 @@ changes:
31303140
and trailing whitespace validation for HTTP/2 header field names and values
31313141
as per [RFC-9113](https://www.rfc-editor.org/rfc/rfc9113.html#section-8.2.1).
31323142
**Default:** `true`.
3143+
* `strictSingleValueFields` {boolean} If `true`, strict validation is used
3144+
for headers and trailers defined as having only a single value, such that
3145+
an error is thrown if multiple values are provided.
3146+
**Default:** `true`.
31333147
* `http1Options` {Object} An options object for configuring the HTTP/1
31343148
fallback when `allowHTTP1` is `true`. These options are passed to the
31353149
underlying HTTP/1 server. See [`http.createServer()`][] for available
Collapse file

‎lib/internal/http2/core.js‎

Copy file name to clipboardExpand all lines: lib/internal/http2/core.js
+50-8Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ const {
136136
const {
137137
assertIsObject,
138138
assertIsArray,
139+
assertValidPseudoHeader,
139140
assertValidPseudoHeaderResponse,
140141
assertValidPseudoHeaderTrailer,
141142
assertWithinRange,
@@ -148,6 +149,7 @@ const {
148149
isPayloadMeaningless,
149150
kAuthority,
150151
kSensitiveHeaders,
152+
kStrictSingleValueFields,
151153
kSocket,
152154
kRequest,
153155
kProtocol,
@@ -1318,6 +1320,8 @@ class Http2Session extends EventEmitter {
13181320
this[kSocket] = socket;
13191321
this[kTimeout] = null;
13201322
this[kHandle] = undefined;
1323+
this[kStrictSingleValueFields] =
1324+
options.strictSingleValueFields;
13211325

13221326
// Do not use nagle's algorithm
13231327
if (typeof socket.setNoDelay === 'function')
@@ -2370,7 +2374,11 @@ class Http2Stream extends Duplex {
23702374

23712375
this[kUpdateTimer]();
23722376

2373-
const headersList = buildNgHeaderString(headers, assertValidPseudoHeaderTrailer);
2377+
const headersList = buildNgHeaderString(
2378+
headers,
2379+
assertValidPseudoHeaderTrailer,
2380+
this.session[kStrictSingleValueFields],
2381+
);
23742382
this[kSentTrailers] = headers;
23752383

23762384
// Send the trailers in setImmediate so we don't do it on nghttp2 stack.
@@ -2579,7 +2587,11 @@ function prepareResponseHeaders(stream, headersParam, options) {
25792587
stream[kSentHeaders] = headers;
25802588
}
25812589

2582-
const headersList = buildNgHeaderString(headers, assertValidPseudoHeaderResponse);
2590+
const headersList = buildNgHeaderString(
2591+
headers,
2592+
assertValidPseudoHeaderResponse,
2593+
stream.session[kStrictSingleValueFields],
2594+
);
25832595

25842596
return { headers, headersList, statusCode };
25852597
}
@@ -2682,7 +2694,11 @@ function processRespondWithFD(self, fd, headers, offset = 0, length = -1,
26822694

26832695
let headersList;
26842696
try {
2685-
headersList = buildNgHeaderString(headers, assertValidPseudoHeaderResponse);
2697+
headersList = buildNgHeaderString(
2698+
headers,
2699+
assertValidPseudoHeaderResponse,
2700+
self.session[kStrictSingleValueFields],
2701+
);
26862702
} catch (err) {
26872703
if (self.ownsFd)
26882704
tryClose(fd);
@@ -2909,7 +2925,11 @@ class ServerHttp2Stream extends Http2Stream {
29092925
if (headers[HTTP2_HEADER_METHOD] === HTTP2_METHOD_HEAD)
29102926
headRequest = options.endStream = true;
29112927

2912-
const headersList = buildNgHeaderString(headers);
2928+
const headersList = buildNgHeaderString(
2929+
headers,
2930+
assertValidPseudoHeader,
2931+
this.session[kStrictSingleValueFields],
2932+
);
29132933

29142934
const streamOptions = options.endStream ? STREAM_OPTION_EMPTY_PAYLOAD : 0;
29152935

@@ -3173,7 +3193,11 @@ class ServerHttp2Stream extends Http2Stream {
31733193

31743194
this[kUpdateTimer]();
31753195

3176-
const headersList = buildNgHeaderString(headers, assertValidPseudoHeaderResponse);
3196+
const headersList = buildNgHeaderString(
3197+
headers,
3198+
assertValidPseudoHeaderResponse,
3199+
this.session[kStrictSingleValueFields],
3200+
);
31773201
if (!this[kInfoHeaders])
31783202
this[kInfoHeaders] = [headers];
31793203
else
@@ -3326,21 +3350,30 @@ function initializeOptions(options) {
33263350
}
33273351

33283352
if (options.maxSessionInvalidFrames !== undefined)
3329-
validateUint32(options.maxSessionInvalidFrames, 'maxSessionInvalidFrames');
3353+
validateUint32(options.maxSessionInvalidFrames, 'options.maxSessionInvalidFrames');
33303354

33313355
if (options.maxSessionRejectedStreams !== undefined) {
33323356
validateUint32(
33333357
options.maxSessionRejectedStreams,
3334-
'maxSessionRejectedStreams',
3358+
'options.maxSessionRejectedStreams',
33353359
);
33363360
}
33373361

33383362
if (options.unknownProtocolTimeout !== undefined)
3339-
validateUint32(options.unknownProtocolTimeout, 'unknownProtocolTimeout');
3363+
validateUint32(options.unknownProtocolTimeout, 'options.unknownProtocolTimeout');
33403364
else
33413365
// TODO(danbev): is this a good default value?
33423366
options.unknownProtocolTimeout = 10000;
33433367

3368+
if (options.strictSingleValueFields !== undefined) {
3369+
validateBoolean(
3370+
options.strictSingleValueFields,
3371+
'options.strictSingleValueFields',
3372+
);
3373+
} else {
3374+
options.strictSingleValueFields = true;
3375+
}
3376+
33443377

33453378
// Initialize http1Options bag for HTTP/1 fallback when allowHTTP1 is true.
33463379
// This bag is passed to storeHTTPOptions() to configure HTTP/1 server
@@ -3553,6 +3586,15 @@ function connect(authority, options, listener) {
35533586
throw new ERR_HTTP2_TOO_MANY_CUSTOM_SETTINGS();
35543587
}
35553588

3589+
if (options.strictSingleValueFields !== undefined) {
3590+
validateBoolean(
3591+
options.strictSingleValueFields,
3592+
'options.strictSingleValueFields',
3593+
);
3594+
} else {
3595+
options.strictSingleValueFields = true;
3596+
}
3597+
35563598
if (typeof authority === 'string')
35573599
authority = new URL(authority);
35583600

Collapse file

‎lib/internal/http2/util.js‎

Copy file name to clipboardExpand all lines: lib/internal/http2/util.js
+21-8Lines 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,
Collapse file

‎lib/internal/quic/quic.js‎

Copy file name to clipboardExpand all lines: lib/internal/quic/quic.js
+7-1Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ const {
112112

113113
const {
114114
buildNgHeaderString,
115+
assertValidPseudoHeader,
115116
} = require('internal/http2/util');
116117

117118
const kEmptyObject = { __proto__: null };
@@ -887,8 +888,13 @@ class QuicStream {
887888
} else {
888889
debug(`stream ${this.id} sending headers`, headers);
889890
}
891+
const headerString = buildNgHeaderString(
892+
headers,
893+
assertValidPseudoHeader,
894+
true, // This could become an option in future
895+
);
890896
// TODO(@jasnell): Support differentiating between early headers, primary headers, etc
891-
return this.#handle.sendHeaders(1, buildNgHeaderString(headers), 1);
897+
return this.#handle.sendHeaders(1, headerString, 1);
892898
}
893899

894900
[kFinishClose](error) {
Collapse file

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

Copy file name to clipboard
+53Lines 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+
}));
Collapse file

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

Copy file name to clipboard
File renamed without changes.
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-http2-util-assert-valid-pseudoheader.js
+7-11Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.