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: #23908 Fixes: #23748 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anatoli Papirovski <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
nodejs · Mar 5, 2019 · 2595fbc · 2595fbc
1 parent fc9ba36
commit 2595fbc
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 0 deletions.
diff --git 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
@@ -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());