Skip to content

Commit 762a11f

Browse files
joyeecheungcjihrig
authored andcommitted
http2: improve errors thrown in header validation
PR-URL: #16718 Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Luigi Pinca <[email protected]>
1 parent 6074c8c commit 762a11f

File tree

5 files changed

+18
-12
lines changed

5 files changed

+18
-12
lines changed

doc/api/errors.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -808,7 +808,7 @@ requests and responses.
808808
<a id="ERR_HTTP2_INVALID_HEADER_VALUE"></a>
809809
### ERR_HTTP2_INVALID_HEADER_VALUE
810810

811-
Used to indicate that an invalid HTTP/2 header value has been specified.
811+
Used to indicate that an invalid HTTP2 header value has been specified.
812812

813813
<a id="ERR_HTTP2_INVALID_INFO_STATUS"></a>
814814
### ERR_HTTP2_INVALID_INFO_STATUS

lib/internal/errors.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ E('ERR_HTTP2_INFO_STATUS_NOT_ALLOWED',
202202
'Informational status codes cannot be used');
203203
E('ERR_HTTP2_INVALID_CONNECTION_HEADERS',
204204
'HTTP/1 Connection specific headers are forbidden: "%s"');
205-
E('ERR_HTTP2_INVALID_HEADER_VALUE', 'Value must not be undefined or null');
205+
E('ERR_HTTP2_INVALID_HEADER_VALUE', 'Invalid value "%s" for header "%s"');
206206
E('ERR_HTTP2_INVALID_INFO_STATUS',
207207
(code) => `Invalid informational status code: ${code}`);
208208
E('ERR_HTTP2_INVALID_PACKED_SETTINGS_LENGTH',

lib/internal/http2/compat.js

+12-6
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,18 @@ let statusMessageWarned = false;
4040
// close as possible to the current require('http') API
4141

4242
function assertValidHeader(name, value) {
43-
if (name === '' || typeof name !== 'string')
44-
throw new errors.TypeError('ERR_INVALID_HTTP_TOKEN', 'Header name', name);
45-
if (isPseudoHeader(name))
46-
throw new errors.Error('ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED');
47-
if (value === undefined || value === null)
48-
throw new errors.TypeError('ERR_HTTP2_INVALID_HEADER_VALUE');
43+
let err;
44+
if (name === '' || typeof name !== 'string') {
45+
err = new errors.TypeError('ERR_INVALID_HTTP_TOKEN', 'Header name', name);
46+
} else if (isPseudoHeader(name)) {
47+
err = new errors.Error('ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED');
48+
} else if (value === undefined || value === null) {
49+
err = new errors.TypeError('ERR_HTTP2_INVALID_HEADER_VALUE', value, name);
50+
}
51+
if (err !== undefined) {
52+
Error.captureStackTrace(err, assertValidHeader);
53+
throw err;
54+
}
4955
}
5056

5157
function isPseudoHeader(name) {

test/parallel/test-http2-compat-serverresponse-headers.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -85,14 +85,14 @@ server.listen(0, common.mustCall(function() {
8585
}, common.expectsError({
8686
code: 'ERR_HTTP2_INVALID_HEADER_VALUE',
8787
type: TypeError,
88-
message: 'Value must not be undefined or null'
88+
message: 'Invalid value "null" for header "foo-bar"'
8989
}));
9090
assert.throws(function() {
9191
response.setHeader(real, undefined);
9292
}, common.expectsError({
9393
code: 'ERR_HTTP2_INVALID_HEADER_VALUE',
9494
type: TypeError,
95-
message: 'Value must not be undefined or null'
95+
message: 'Invalid value "undefined" for header "foo-bar"'
9696
}));
9797
common.expectsError(
9898
() => response.setHeader(), // header name undefined

test/parallel/test-http2-compat-serverresponse-trailers.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,15 @@ server.listen(0, common.mustCall(() => {
2828
{
2929
code: 'ERR_HTTP2_INVALID_HEADER_VALUE',
3030
type: TypeError,
31-
message: 'Value must not be undefined or null'
31+
message: 'Invalid value "undefined" for header "test"'
3232
}
3333
);
3434
common.expectsError(
3535
() => response.setTrailer('test', null),
3636
{
3737
code: 'ERR_HTTP2_INVALID_HEADER_VALUE',
3838
type: TypeError,
39-
message: 'Value must not be undefined or null'
39+
message: 'Invalid value "null" for header "test"'
4040
}
4141
);
4242
common.expectsError(

0 commit comments

Comments
 (0)