Skip to content

Commit

Permalink
http2: improve compatibility with http/1
Browse files Browse the repository at this point in the history
When using the compatibility API the connection header is from now on
ignored instead of throwing an `ERR_HTTP2_INVALID_CONNECTION_HEADERS`
error.
This logs a warning in such case to notify the user about the ignored
header.

PR-URL: #23908
Fixes: #23748
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
sagitsofan authored and BethGriggs committed Apr 16, 2019
1 parent 281eb0f commit cc25f22
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 0 deletions.
25 changes: 25 additions & 0 deletions lib/internal/http2/compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ const {
} = constants;

let statusMessageWarned = false;
let statusConnectionHeaderWarned = false;

// Defines and implements an API compatibility layer on top of the core
// HTTP/2 implementation, intended to provide an interface that is as
Expand All @@ -60,6 +61,8 @@ function assertValidHeader(name, value) {
err = new ERR_HTTP2_PSEUDOHEADER_NOT_ALLOWED();
} else if (value === undefined || value === null) {
err = new ERR_HTTP2_INVALID_HEADER_VALUE(value, name);
} else if (!isConnectionHeaderAllowed(name, value)) {
connectionHeaderMessageWarn();
}
if (err !== undefined) {
Error.captureStackTrace(err, assertValidHeader);
Expand Down Expand Up @@ -90,6 +93,23 @@ function statusMessageWarn() {
}
}

function isConnectionHeaderAllowed(name, value) {
return name !== constants.HTTP2_HEADER_CONNECTION ||
value === 'trailers';
}

function connectionHeaderMessageWarn() {
if (statusConnectionHeaderWarned === false) {
process.emitWarning(
'The provided connection header is not valid, ' +
'the value will be dropped from the header and ' +
'will never be in use.',
'UnsupportedWarning'
);
statusConnectionHeaderWarned = true;
}
}

function onStreamData(chunk) {
const request = this[kRequest];
if (request !== undefined && !request.push(chunk))
Expand Down Expand Up @@ -541,6 +561,11 @@ class Http2ServerResponse extends Stream {
[kSetHeader](name, value) {
name = name.trim().toLowerCase();
assertValidHeader(name, value);

if (!isConnectionHeaderAllowed(name, value)) {
return;
}

this[kHeaders][name] = value;
}

Expand Down
7 changes: 7 additions & 0 deletions test/parallel/test-http2-server-set-header.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const body =
const server = http2.createServer((req, res) => {
res.setHeader('foobar', 'baz');
res.setHeader('X-POWERED-BY', 'node-test');
res.setHeader('connection', 'connection-test');
res.end(body);
});

Expand All @@ -34,4 +35,10 @@ server.listen(0, common.mustCall(() => {
req.end();
}));

const compatMsg = 'The provided connection header is not valid, ' +
'the value will be dropped from the header and ' +
'will never be in use.';

common.expectWarning('UnsupportedWarning', compatMsg);

server.on('error', common.mustNotCall());

0 comments on commit cc25f22

Please sign in to comment.