http2: fix compat bug where duplicate headers were lost in writeHead

Previously if an array of raw headers was provided to writeHead, and that array contained any duplicate keys, only the last value was recorded for that key and all others were lost. This changes bring the HTTP/2 compat logic in line with the logic for HTTP/1, so that writeHead headers overwrite any previous conflicting setHeader headers, but do not overwrite themselves.
nodejs · nodejs-github-bot · Jan 12, 2024 · Jan 8, 2024 · Jan 9, 2024 · Jan 9, 2024
commit c7367e4596fbcf74148cff56e4a189274a65254b
diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js
@@ -726,18 +726,41 @@ class Http2ServerResponse extends Stream {
 
     let i;
     if (ArrayIsArray(headers)) {
+      if (this[kHeaders]) {
+        // Headers in obj should override previous headers but still
+        // allow explicit duplicates. To do so, we first remove any
+        // existing conflicts, then use appendHeader. This is the
+        // slow path, which only applies when you use setHeader and
+        // then pass headers in writeHead too.
+
+        // We need to handle both the tuple and flat array formats, just
+        // like the logic further below.
+        if (headers.length && ArrayIsArray(headers[0])) {
+          for (let n = 0; n < headers.length; n += 1) {
+            const key = headers[n + 0][0];
+            this.removeHeader(key);
+          }
+        } else {
+          for (let n = 0; n < headers.length; n += 2) {
+            const key = headers[n + 0];
+            this.removeHeader(key);
+          }
+        }
+      }
+
+      // Append all the headers provided in the array:
       if (headers.length && ArrayIsArray(headers[0])) {
         for (i = 0; i < headers.length; i++) {
           const header = headers[i];
-          this[kSetHeader](header[0], header[1]);
+          this[kAppendHeader](header[0], header[1]);
         }
       } else {
         if (headers.length % 2 !== 0) {
           throw new ERR_INVALID_ARG_VALUE('headers', headers);
         }
 
         for (i = 0; i < headers.length; i += 2) {
-          this[kSetHeader](headers[i], headers[i + 1]);
+          this[kAppendHeader](headers[i], headers[i + 1]);
         }
       }
     } else if (typeof headers === 'object') {

diff --git a/test/parallel/test-http2-compat-serverresponse-writehead-array.js b/test/parallel/test-http2-compat-serverresponse-writehead-array.js
@@ -16,6 +16,7 @@ const http2 = require('http2');
     server.once('request', common.mustCall((request, response) => {
       const returnVal = response.writeHead(200, [
         ['foo', 'bar'],
+        ['foo', 'baz'],
         ['ABC', 123],
       ]);
       assert.strictEqual(returnVal, response);
@@ -26,7 +27,7 @@ const http2 = require('http2');
       const request = client.request();
 
       request.on('response', common.mustCall((headers) => {
-        assert.strictEqual(headers.foo, 'bar');
+        assert.strictEqual(headers.foo, 'bar, baz');
         assert.strictEqual(headers.abc, '123');
         assert.strictEqual(headers[':status'], 200);
       }, 1));
@@ -45,7 +46,7 @@ const http2 = require('http2');
     const port = server.address().port;
 
     server.once('request', common.mustCall((request, response) => {
-      const returnVal = response.writeHead(200, ['foo', 'bar', 'ABC', 123]);
+      const returnVal = response.writeHead(200, ['foo', 'bar', 'foo', 'baz', 'ABC', 123]);
       assert.strictEqual(returnVal, response);
       response.end(common.mustCall(() => { server.close(); }));
     }));
@@ -54,7 +55,7 @@ const http2 = require('http2');
       const request = client.request();
 
       request.on('response', common.mustCall((headers) => {
-        assert.strictEqual(headers.foo, 'bar');
+        assert.strictEqual(headers.foo, 'bar, baz');
         assert.strictEqual(headers.abc, '123');
         assert.strictEqual(headers[':status'], 200);
       }, 1));