FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

http2: simplify mapToHeaders, stricter validation · nodejs/node@98b9705 · GitHub

/ node Public

Commit 98b9705

Browse files
apapirovski
authored and
gibfahn
committed
http2: simplify mapToHeaders, stricter validation
No longer check whether key is a symbol as Object.keys does not return symbols. No longer convert key to string as it is always a string. Validate that only one value is passed for each pseudo-header. Extend illegal connection header message to include the name of the problematic header. Extend tests to cover this behaviour. PR-URL: #16575 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent e592c32 commit 98b9705

4 files changed

Lines changed: 38 additions & 22 deletions

File tree

‎lib/internal/errors.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ E('ERR_HTTP2_INFO_HEADERS_AFTER_RESPOND',
184184
E('ERR_HTTP2_INFO_STATUS_NOT_ALLOWED',
185185
'Informational status codes cannot be used');
186186
E('ERR_HTTP2_INVALID_CONNECTION_HEADERS',
187-
'HTTP/1 Connection specific headers are forbidden');
187+
'HTTP/1 Connection specific headers are forbidden: "%s"');
188188
E('ERR_HTTP2_INVALID_HEADER_VALUE', 'Value must not be undefined or null');
189189
E('ERR_HTTP2_INVALID_INFO_STATUS',
190190
(code) => `Invalid informational status code: ${code}`);

‎lib/internal/http2/util.js‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -399,10 +399,10 @@ function mapToHeaders(map,
399399
for (var i = 0; i < keys.length; i++) {
400400
let key = keys[i];
401401
let value = map[key];
402-
let val;
403-
if (typeof key === 'symbol' || value === undefined || !key)
402+
if (value === undefined || key === '')
404403
continue;
405-
key = String(key).toLowerCase();
404+
key = key.toLowerCase();
405+
const isSingleValueHeader = kSingleValueHeaders.has(key);
406406
let isArray = Array.isArray(value);
407407
if (isArray) {
408408
switch (value.length) {
@@ -413,34 +413,35 @@ function mapToHeaders(map,
413413
isArray = false;
414414
break;
415415
default:
416-
if (kSingleValueHeaders.has(key))
416+
if (isSingleValueHeader)
417417
return new errors.Error('ERR_HTTP2_HEADER_SINGLE_VALUE', key);
418418
}
419+
} else {
420+
value = String(value);
421+
}
422+
if (isSingleValueHeader) {
423+
if (singles.has(key))
424+
return new errors.Error('ERR_HTTP2_HEADER_SINGLE_VALUE', key);
425+
singles.add(key);
419426
}
420427
if (key[0] === ':') {
421428
const err = assertValuePseudoHeader(key);
422429
if (err !== undefined)
423430
return err;
424-
ret = `${key}\0${String(value)}\0${ret}`;
431+
ret = `${key}\0${value}\0${ret}`;
425432
count++;
426433
} else {
427-
if (kSingleValueHeaders.has(key)) {
428-
if (singles.has(key))
429-
return new errors.Error('ERR_HTTP2_HEADER_SINGLE_VALUE', key);
430-
singles.add(key);
431-
}
432434
if (isIllegalConnectionSpecificHeader(key, value)) {
433-
return new errors.Error('ERR_HTTP2_INVALID_CONNECTION_HEADERS');
435+
return new errors.Error('ERR_HTTP2_INVALID_CONNECTION_HEADERS', key);
434436
}
435437
if (isArray) {
436438
for (var k = 0; k < value.length; k++) {
437-
val = String(value[k]);
439+
const val = String(value[k]);
438440
ret += `${key}\0${val}\0`;
439441
}
440442
count += value.length;
441443
} else {
442-
val = String(value);
443-
ret += `${key}\0${val}\0`;
444+
ret += `${key}\0${value}\0`;
444445
count++;
445446
}
446447
}

‎test/parallel/test-http2-server-push-stream-errors-args.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ server.on('stream', common.mustCall((stream, headers) => {
3232
() => stream.pushStream({ 'connection': 'test' }, {}, () => {}),
3333
{
3434
code: 'ERR_HTTP2_INVALID_CONNECTION_HEADERS',
35-
message: 'HTTP/1 Connection specific headers are forbidden'
35+
message: 'HTTP/1 Connection specific headers are forbidden: "connection"'
3636
}
3737
);
3838

‎test/parallel/test-http2-util-headers-list.js‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -158,14 +158,28 @@ const {
158158
// Arrays containing a single set-cookie value are handled correctly
159159
// (https://github.com/nodejs/node/issues/16452)
160160
const headers = {
161-
'set-cookie': 'foo=bar'
161+
'set-cookie': ['foo=bar']
162162
};
163163
assert.deepStrictEqual(
164164
mapToHeaders(headers),
165165
[ [ 'set-cookie', 'foo=bar', '' ].join('\0'), 1 ]
166166
);
167167
}
168168

169+
{
170+
// pseudo-headers are only allowed a single value
171+
const headers = {
172+
':status': 200,
173+
':statuS': 204,
174+
};
175+
176+
common.expectsError({
177+
code: 'ERR_HTTP2_HEADER_SINGLE_VALUE',
178+
type: Error,
179+
message: 'Header field ":status" must have only a single value'
180+
})(mapToHeaders(headers));
181+
}
182+
169183
// The following are not allowed to have multiple values
170184
[
171185
HTTP2_HEADER_STATUS,
@@ -248,8 +262,6 @@ const {
248262
assert(!(mapToHeaders({ [name]: [1, 2, 3] }) instanceof Error), name);
249263
});
250264

251-
const regex =
252-
/^HTTP\/1 Connection specific headers are forbidden$/;
253265
[
254266
HTTP2_HEADER_CONNECTION,
255267
HTTP2_HEADER_UPGRADE,
@@ -269,18 +281,21 @@ const regex =
269281
].forEach((name) => {
270282
common.expectsError({
271283
code: 'ERR_HTTP2_INVALID_CONNECTION_HEADERS',
272-
message: regex
284+
message: 'HTTP/1 Connection specific headers are forbidden: ' +
285+
`"${name.toLowerCase()}"`
273286
})(mapToHeaders({ [name]: 'abc' }));
274287
});
275288

276289
common.expectsError({
277290
code: 'ERR_HTTP2_INVALID_CONNECTION_HEADERS',
278-
message: regex
291+
message: 'HTTP/1 Connection specific headers are forbidden: ' +
292+
`"${HTTP2_HEADER_TE}"`
279293
})(mapToHeaders({ [HTTP2_HEADER_TE]: ['abc'] }));
280294

281295
common.expectsError({
282296
code: 'ERR_HTTP2_INVALID_CONNECTION_HEADERS',
283-
message: regex
297+
message: 'HTTP/1 Connection specific headers are forbidden: ' +
298+
`"${HTTP2_HEADER_TE}"`
284299
})(mapToHeaders({ [HTTP2_HEADER_TE]: ['abc', 'trailers'] }));
285300

286301
assert(!(mapToHeaders({ te: 'trailers' }) instanceof Error));

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL