Skip to content

Commit

Permalink
http: improve errors thrown in header validation
Browse files Browse the repository at this point in the history
  • Loading branch information
joyeecheung committed Nov 3, 2017
1 parent 3d20190 commit a4c2e6e
Show file tree
Hide file tree
Showing 9 changed files with 49 additions and 43 deletions.
6 changes: 4 additions & 2 deletions doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -818,9 +818,11 @@ HTTP/1 connection specific headers are forbidden to be used in HTTP/2
requests and responses.

<a id="ERR_HTTP2_INVALID_HEADER_VALUE"></a>
### ERR_HTTP2_INVALID_HEADER_VALUE
### ERR_HTTP_INVALID_HEADER_VALUE
<a id="ERR_HTTP_INVALID_HEADER_VALUE"></a>
### ERR_HTTP_INVALID_HEADER_VALUE

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

<a id="ERR_HTTP2_INVALID_INFO_STATUS"></a>
### ERR_HTTP2_INVALID_INFO_STATUS
Expand Down
38 changes: 18 additions & 20 deletions lib/_http_outgoing.js
Original file line number Diff line number Diff line change
Expand Up @@ -437,16 +437,7 @@ function _storeHeader(firstLine, headers) {

function storeHeader(self, state, key, value, validate) {
if (validate) {
if (typeof key !== 'string' || !key || !checkIsHttpToken(key)) {
throw new errors.TypeError(
'ERR_INVALID_HTTP_TOKEN', 'Header name', key);
}
if (value === undefined) {
throw new errors.TypeError('ERR_MISSING_ARGS', `header "${key}"`);
} else if (checkInvalidHeaderChar(value)) {
debug('Header "%s" contains invalid characters', key);
throw new errors.TypeError('ERR_INVALID_CHAR', 'header content', key);
}
validateHeader(key, value);
}
state.header += key + ': ' + escapeHeaderValue(value) + CRLF;
matchHeader(self, state, key, value);
Expand Down Expand Up @@ -494,20 +485,27 @@ function matchHeader(self, state, field, value) {
}
}

function validateHeader(msg, name, value) {
if (typeof name !== 'string' || !name || !checkIsHttpToken(name))
throw new errors.TypeError('ERR_INVALID_HTTP_TOKEN', 'Header name', name);
if (value === undefined)
throw new errors.TypeError('ERR_MISSING_ARGS', 'value');
if (msg._header)
throw new errors.Error('ERR_HTTP_HEADERS_SENT', 'set');
if (checkInvalidHeaderChar(value)) {
function validateHeader(name, value) {
var err;
if (typeof name !== 'string' || !name || !checkIsHttpToken(name)) {
err = new errors.TypeError('ERR_INVALID_HTTP_TOKEN', 'Header name', name);
} else if (value === undefined) {
err = new errors.TypeError('ERR_HTTP_INVALID_HEADER_VALUE', value, name);
} else if (checkInvalidHeaderChar(value)) {
debug('Header "%s" contains invalid characters', name);
throw new errors.TypeError('ERR_INVALID_CHAR', 'header content', name);
err = new errors.TypeError('ERR_INVALID_CHAR', 'header content', name);
}
if (err) {
Error.captureStackTrace(err, validateHeader);
throw err;
}
}

OutgoingMessage.prototype.setHeader = function setHeader(name, value) {
validateHeader(this, name, value);
if (this._header) {
throw new errors.Error('ERR_HTTP_HEADERS_SENT', 'set');
}
validateHeader(name, value);

if (!this[outHeadersKey])
this[outHeadersKey] = {};
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,6 @@ 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_INFO_STATUS',
(code) => `Invalid informational status code: ${code}`);
E('ERR_HTTP2_INVALID_PACKED_SETTINGS_LENGTH',
Expand Down Expand Up @@ -241,6 +240,7 @@ E('ERR_HTTP2_UNSUPPORTED_PROTOCOL',
E('ERR_HTTP_HEADERS_SENT',
'Cannot %s headers after they are sent to the client');
E('ERR_HTTP_INVALID_CHAR', 'Invalid character in statusMessage.');
E('ERR_HTTP_INVALID_HEADER_VALUE', 'Invalid value "%s" for header "%s"');
E('ERR_HTTP_INVALID_STATUS_CODE',
(originalStatusCode) => `Invalid status code: ${originalStatusCode}`);
E('ERR_HTTP_TRAILER_INVALID',
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');
var 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_HTTP_INVALID_HEADER_VALUE', value, name);
}
if (err) {
Error.captureStackTrace(err, assertValidHeader);
throw err;
}
}

function isPseudoHeader(name) {
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http-mutable-headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ const s = http.createServer(common.mustCall((req, res) => {
common.expectsError(
() => res.setHeader('someHeader'),
{
code: 'ERR_MISSING_ARGS',
code: 'ERR_HTTP_INVALID_HEADER_VALUE',
type: TypeError,
message: 'The "value" argument must be specified'
message: 'Invalid value "undefined" for header "someHeader"'
}
);
common.expectsError(
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http-outgoing-proto.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ assert.throws(() => {
const outgoingMessage = new OutgoingMessage();
outgoingMessage.setHeader('test');
}, common.expectsError({
code: 'ERR_MISSING_ARGS',
code: 'ERR_HTTP_INVALID_HEADER_VALUE',
type: TypeError,
message: 'The "value" argument must be specified'
message: 'Invalid value "undefined" for header "test"'
}));

assert.throws(() => {
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http-write-head.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ const s = http.createServer(common.mustCall((req, res) => {
common.expectsError(
() => res.setHeader('foo', undefined),
{
code: 'ERR_MISSING_ARGS',
code: 'ERR_HTTP_INVALID_HEADER_VALUE',
type: TypeError,
message: 'The "value" argument must be specified'
message: 'Invalid value "undefined" for header "foo"'
}
);

Expand Down
8 changes: 4 additions & 4 deletions test/parallel/test-http2-compat-serverresponse-headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,16 +83,16 @@ server.listen(0, common.mustCall(function() {
assert.throws(function() {
response.setHeader(real, null);
}, common.expectsError({
code: 'ERR_HTTP2_INVALID_HEADER_VALUE',
code: 'ERR_HTTP_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',
code: 'ERR_HTTP_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
8 changes: 4 additions & 4 deletions test/parallel/test-http2-compat-serverresponse-trailers.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,17 @@ server.listen(0, common.mustCall(() => {
common.expectsError(
() => response.setTrailer('test', undefined),
{
code: 'ERR_HTTP2_INVALID_HEADER_VALUE',
code: 'ERR_HTTP_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',
code: 'ERR_HTTP_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 a4c2e6e

Please sign in to comment.