Skip to content

Commit cfef7fb

Browse files
committed
http2: address review feedback on httpValidation option
- Disable strictFieldWhitespaceValidation in relaxed/insecure mode so it does not conflict with the relaxed behaviour (per @pimterry review). - Document that relaxed/insecure subsumes strictSingleValueFields and strictFieldWhitespaceValidation. - Add integration tests for the http2.connect() httpValidation option.
1 parent 844b172 commit cfef7fb

2 files changed

Lines changed: 113 additions & 4 deletions

File tree

‎doc/api/http2.md‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1129,8 +1129,8 @@ creates and returns an `Http2Stream` instance that can be used to send an
11291129
HTTP/2 request to the connected server.
11301130

11311131
When sending a request, header values must not contain characters outside the
1132-
`latin1` encoding. The `:path` pseudo-header must not contain unescaped
1133-
characters.
1132+
`latin1` encoding, and the `:path` pseudo-header must not contain unescaped
1133+
characters (this `:path` check is always enforced).
11341134

11351135
This strict validation can be relaxed via the `httpValidation` option of
11361136
[`http2.connect()`](#http2connectauthority-options-listener). The `'relaxed'`
@@ -3312,6 +3312,9 @@ server.listen(8443);
33123312
<!-- YAML
33133313
added: v8.4.0
33143314
changes:
3315+
- version: REPLACEME
3316+
pr-url: https://github.com/nodejs/node/pull/63571
3317+
description: Added the `httpValidation` option.
33153318
- version: v26.11.0
33163319
pr-url: https://github.com/nodejs/node/pull/65619
33173320
description: Added the `connectionWindowSize` option.
@@ -3441,8 +3444,9 @@ changes:
34413444
[Fetch specification][].
34423445
* `'insecure'`: Skips header value validation (equivalent to
34433446
`insecureHTTPParser` in HTTP/1).
3444-
When set to `'relaxed'` or `'insecure'`, `strictSingleValueFields` is
3445-
automatically disabled.
3447+
When set to `'relaxed'` or `'insecure'`, `strictSingleValueFields` and
3448+
`strictFieldWhitespaceValidation` are automatically disabled, as they
3449+
would otherwise conflict with the relaxed behaviour.
34463450
**Default:** `'strict'`.
34473451
* `listener` {Function} Will be registered as a one-time listener of the
34483452
[`'connect'`][] event.
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
'use strict';
2+
const common = require('../common');
3+
if (!common.hasCrypto) { common.skip('missing crypto'); }
4+
const assert = require('assert');
5+
const http2 = require('http2');
6+
7+
// Integration tests for the http2.connect() `httpValidation` option.
8+
// See also test-http2-util-headers-list.js for the per-value validation logic.
9+
10+
const server = http2.createServer();
11+
// Consume any stream that is actually created. Control characters and
12+
// duplicate pseudo-headers may be rejected by nghttp2 asynchronously, so
13+
// swallow any stream errors rather than letting them become uncaught.
14+
server.on('stream', (stream) => {
15+
stream.on('error', () => {});
16+
});
17+
18+
server.listen(0, common.mustCall(() => {
19+
const origin = `http://localhost:${server.address().port}`;
20+
21+
function swallow(stream) {
22+
stream.on('error', () => {});
23+
stream.destroy();
24+
return stream;
25+
}
26+
27+
// -- strict (default) --
28+
{
29+
const session = http2.connect(origin);
30+
session.on('error', () => {});
31+
// Non-latin1 characters are rejected by default.
32+
assert.throws(() => session.request({ 'x-bad-char': 'oʊmɪɡə' }), {
33+
code: 'ERR_HTTP2_INVALID_HEADER_VALUE',
34+
});
35+
// Control characters are rejected by default per the HTTP RFCs.
36+
assert.throws(() => session.request({ 'x-control': 'value\x01here' }), {
37+
code: 'ERR_HTTP2_INVALID_HEADER_VALUE',
38+
});
39+
// strictSingleValueFields is enabled by default.
40+
assert.throws(() => session.request([
41+
[':method', 'GET'],
42+
[':method', 'POST'],
43+
]), {
44+
code: 'ERR_HTTP2_HEADER_SINGLE_VALUE',
45+
});
46+
// The :path pseudo-header must not contain unescaped characters.
47+
assert.throws(() => session.request({ ':path': 'bad\u0100path' }), {
48+
code: 'ERR_UNESCAPED_CHARACTERS',
49+
});
50+
session.close();
51+
}
52+
53+
// -- relaxed --
54+
{
55+
const session = http2.connect(origin, { httpValidation: 'relaxed' });
56+
session.on('error', () => {});
57+
// Control characters permitted by the Fetch specification are allowed.
58+
swallow(session.request({ 'x-control': 'value\x01here' }));
59+
// relaxed/insecure mode subsumes and disables strictSingleValueFields.
60+
swallow(session.request([
61+
[':method', 'GET'],
62+
[':method', 'POST'],
63+
]));
64+
// Non-latin1 characters are still rejected in relaxed mode.
65+
assert.throws(() => session.request({ 'x-bad-char': 'oʊmɪɡə' }), {
66+
code: 'ERR_HTTP2_INVALID_HEADER_VALUE',
67+
});
68+
// NUL is always rejected.
69+
assert.throws(() => session.request({ 'x-nul': 'value\x00here' }), {
70+
code: 'ERR_HTTP2_INVALID_HEADER_VALUE',
71+
});
72+
// The :path pseudo-header check is always enforced.
73+
assert.throws(() => session.request({ ':path': 'bad\u0100path' }), {
74+
code: 'ERR_UNESCAPED_CHARACTERS',
75+
});
76+
session.close();
77+
}
78+
79+
// -- insecure --
80+
{
81+
const session = http2.connect(origin, { httpValidation: 'insecure' });
82+
session.on('error', () => {});
83+
// All header value validation is skipped, including newlines.
84+
swallow(session.request({ 'x-newline': 'value\nhere' }));
85+
// strictSingleValueFields is disabled in insecure mode too.
86+
swallow(session.request([
87+
[':method', 'GET'],
88+
[':method', 'POST'],
89+
]));
90+
// The :path pseudo-header check is still enforced.
91+
assert.throws(() => session.request({ ':path': 'bad\u0100path' }), {
92+
code: 'ERR_UNESCAPED_CHARACTERS',
93+
});
94+
session.close();
95+
}
96+
97+
// -- invalid option value --
98+
{
99+
assert.throws(() => http2.connect(origin, { httpValidation: 'bogus' }), {
100+
code: 'ERR_INVALID_ARG_VALUE',
101+
});
102+
}
103+
104+
server.close();
105+
}));

0 commit comments

Comments
 (0)