From 2595fbc8b1b70fc5fa6d88177f35ef7df0ecf079 Mon Sep 17 00:00:00 2001 From: Sagi Tsofan Date: Sun, 21 Oct 2018 21:48:05 +0300 Subject: [PATCH] http2: improve compatibility with http/1 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: https://github.com/nodejs/node/pull/23908 Fixes: https://github.com/nodejs/node/issues/23748 Reviewed-By: James M Snell Reviewed-By: Anatoli Papirovski Reviewed-By: Matteo Collina Reviewed-By: Ruben Bridgewater --- lib/internal/http2/compat.js | 25 +++++++++++++++++++ test/parallel/test-http2-server-set-header.js | 7 ++++++ 2 files changed, 32 insertions(+) diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js index da88f4d880ad67..286bc51683e494 100644 --- a/lib/internal/http2/compat.js +++ b/lib/internal/http2/compat.js @@ -45,6 +45,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 @@ -58,6 +59,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); @@ -88,6 +91,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)) @@ -539,6 +559,11 @@ class Http2ServerResponse extends Stream { [kSetHeader](name, value) { name = name.trim().toLowerCase(); assertValidHeader(name, value); + + if (!isConnectionHeaderAllowed(name, value)) { + return; + } + this[kHeaders][name] = value; } diff --git a/test/parallel/test-http2-server-set-header.js b/test/parallel/test-http2-server-set-header.js index 83f373ec21b314..13ca1142fa3892 100644 --- a/test/parallel/test-http2-server-set-header.js +++ b/test/parallel/test-http2-server-set-header.js @@ -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); }); @@ -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());