http2: add h2 compat support for appendHeader

PR-URL: #51412 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Marco Ippolito <[email protected]>
nodejs · Mar 25, 2024 · a2e7b77 · a2e7b77
1 parent 6d21701
commit a2e7b77
Show file tree

Hide file tree

Showing 5 changed files with 114 additions and 8 deletions.
diff --git a/doc/api/http.md b/doc/api/http.md
@@ -2991,12 +2991,12 @@ added:
 * `value` {string|string\[]} Header value
 * Returns: {this}
 
-Append a single header value for the header object.
+Append a single header value to the header object.
 
-If the value is an array, this is equivalent of calling this method multiple
+If the value is an array, this is equivalent to calling this method multiple
 times.
 
-If there were no previous value for the header, this is equivalent of calling
+If there were no previous values for the header, this is equivalent to calling
 [`outgoingMessage.setHeader(name, value)`][].
 
 Depending of the value of `options.uniqueHeaders` when the client request or the

diff --git a/doc/api/http2.md b/doc/api/http2.md
@@ -3659,6 +3659,36 @@ message) to the response.
 Attempting to set a header field name or value that contains invalid characters
 will result in a [`TypeError`][] being thrown.
 
+#### `response.appendHeader(name, value)`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+* `name` {string}
+* `value` {string|string\[]}
+
+Append a single header value to the header object.
+
+If the value is an array, this is equivalent to calling this method multiple
+times.
+
+If there were no previous values for the header, this is equivalent to calling
+[`response.setHeader()`][].
+
+Attempting to set a header field name or value that contains invalid characters
+will result in a [`TypeError`][] being thrown.
+
+```js
+// Returns headers including "set-cookie: a" and "set-cookie: b"
+const server = http2.createServer((req, res) => {
+  res.setHeader('set-cookie', 'a');
+  res.appendHeader('set-cookie', 'b');
+  res.writeHead(200);
+  res.end('ok');
+});
+```
+
 #### `response.connection`
 
 <!-- YAML

diff --git a/lib/internal/http2/compat.js b/lib/internal/http2/compat.js
@@ -76,6 +76,7 @@ const kRawHeaders = Symbol('rawHeaders');
 const kTrailers = Symbol('trailers');
 const kRawTrailers = Symbol('rawTrailers');
 const kSetHeader = Symbol('setHeader');
+const kAppendHeader = Symbol('appendHeader');
 const kAborted = Symbol('aborted');
 
 let statusMessageWarned = false;
@@ -652,6 +653,47 @@ class Http2ServerResponse extends Stream {
     this[kHeaders][name] = value;
   }
 
+  appendHeader(name, value) {
+    validateString(name, 'name');
+    if (this[kStream].headersSent)
+      throw new ERR_HTTP2_HEADERS_SENT();
+
+    this[kAppendHeader](name, value);
+  }
+
+  [kAppendHeader](name, value) {
+    name = StringPrototypeToLowerCase(StringPrototypeTrim(name));
+    assertValidHeader(name, value);
+
+    if (!isConnectionHeaderAllowed(name, value)) {
+      return;
+    }
+
+    if (name[0] === ':')
+      assertValidPseudoHeader(name);
+    else if (!checkIsHttpToken(name))
+      this.destroy(new ERR_INVALID_HTTP_TOKEN('Header name', name));
+
+    // Handle various possible cases the same as OutgoingMessage.appendHeader:
+    const headers = this[kHeaders];
+    if (headers === null || !headers[name]) {
+      return this.setHeader(name, value);
+    }
+
+    if (!ArrayIsArray(headers[name])) {
+      headers[name] = [headers[name]];
+    }
+
+    const existingValues = headers[name];
+    if (ArrayIsArray(value)) {
+      for (let i = 0, length = value.length; i < length; i++) {
+        existingValues.push(value[i]);
+      }
+    } else {
+      existingValues.push(value);
+    }
+  }
+
   get statusMessage() {
     statusMessageWarn();
 
@@ -684,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-headers.js b/test/parallel/test-http2-compat-serverresponse-headers.js
@@ -38,8 +38,18 @@ server.listen(0, common.mustCall(function() {
     response.setHeader(denormalised, expectedValue);
     assert.strictEqual(response.getHeader(denormalised), expectedValue);
     assert.strictEqual(response.hasHeader(denormalised), true);
+    assert.strictEqual(response.hasHeader(real), true);
+
+    response.appendHeader(real, expectedValue);
+    assert.deepStrictEqual(response.getHeader(real), [
+      expectedValue,
+      expectedValue,
+    ]);
+    assert.strictEqual(response.hasHeader(real), true);
+
     response.removeHeader(denormalised);
     assert.strictEqual(response.hasHeader(denormalised), false);
+    assert.strictEqual(response.hasHeader(real), false);
 
     ['hasHeader', 'getHeader', 'removeHeader'].forEach((fnName) => {
       assert.throws(

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));