Skip to content

Commit

Permalink
http2: improve errors thrown in header validation
Browse files Browse the repository at this point in the history
PR-URL: #16718
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
  • Loading branch information
joyeecheung authored and cjihrig committed Nov 6, 2017
1 parent 6074c8c commit 762a11f
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 12 deletions.
2 changes: 1 addition & 1 deletion doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -808,7 +808,7 @@ requests and responses.
<a id="ERR_HTTP2_INVALID_HEADER_VALUE"></a>
### ERR_HTTP2_INVALID_HEADER_VALUE

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

<a id="ERR_HTTP2_INVALID_INFO_STATUS"></a>
### ERR_HTTP2_INVALID_INFO_STATUS
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ E('ERR_HTTP2_INFO_STATUS_NOT_ALLOWED',
'Informational status codes cannot be used');
E('ERR_HTTP2_INVALID_CONNECTION_HEADERS',
'HTTP/1 Connection specific headers are forbidden: "%s"');
E('ERR_HTTP2_INVALID_HEADER_VALUE', 'Value must not be undefined or null');
E('ERR_HTTP2_INVALID_HEADER_VALUE', 'Invalid value "%s" for header "%s"');
E('ERR_HTTP2_INVALID_INFO_STATUS',
(code) => `Invalid informational status code: ${code}`);
E('ERR_HTTP2_INVALID_PACKED_SETTINGS_LENGTH',
Expand Down
18 changes: 12 additions & 6 deletions lib/internal/http2/compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,18 @@ let statusMessageWarned = false;
// close as possible to the current require('http') API

function assertValidHeader(name, value) {
if (name === '' || typeof name !== 'string')
throw new errors.TypeError('ERR_INVALID_HTTP_TOKEN', 'Header name', name);
if (isPseudoHeader(name))
throw new errors.Error('ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED');
if (value === undefined || value === null)
throw new errors.TypeError('ERR_HTTP2_INVALID_HEADER_VALUE');
let err;
if (name === '' || typeof name !== 'string') {
err = new errors.TypeError('ERR_INVALID_HTTP_TOKEN', 'Header name', name);
} else if (isPseudoHeader(name)) {
err = new errors.Error('ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED');
} else if (value === undefined || value === null) {
err = new errors.TypeError('ERR_HTTP2_INVALID_HEADER_VALUE', value, name);
}
if (err !== undefined) {
Error.captureStackTrace(err, assertValidHeader);
throw err;
}
}

function isPseudoHeader(name) {
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http2-compat-serverresponse-headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,14 @@ server.listen(0, common.mustCall(function() {
}, common.expectsError({
code: 'ERR_HTTP2_INVALID_HEADER_VALUE',
type: TypeError,
message: 'Value must not be undefined or null'
message: 'Invalid value "null" for header "foo-bar"'
}));
assert.throws(function() {
response.setHeader(real, undefined);
}, common.expectsError({
code: 'ERR_HTTP2_INVALID_HEADER_VALUE',
type: TypeError,
message: 'Value must not be undefined or null'
message: 'Invalid value "undefined" for header "foo-bar"'
}));
common.expectsError(
() => response.setHeader(), // header name undefined
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http2-compat-serverresponse-trailers.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ server.listen(0, common.mustCall(() => {
{
code: 'ERR_HTTP2_INVALID_HEADER_VALUE',
type: TypeError,
message: 'Value must not be undefined or null'
message: 'Invalid value "undefined" for header "test"'
}
);
common.expectsError(
() => response.setTrailer('test', null),
{
code: 'ERR_HTTP2_INVALID_HEADER_VALUE',
type: TypeError,
message: 'Value must not be undefined or null'
message: 'Invalid value "null" for header "test"'
}
);
common.expectsError(
Expand Down

0 comments on commit 762a11f

Please sign in to comment.