Repository: nodejs/node
Pull request: http2: add support for raw header arrays in h2Stream.respond()

This PR adds raw header array support to `serverStream.respond(headers)`, continuing slowly on from #57917 (which added this client side to `clientSession.request()`).

There's quite a few other HTTP/2 APIs where raw header support will also be relevant that this doesn't touch, I'm intending to keep working through these incrementally as they become up in my other work. I notably haven't included `respondWithFile` and `respondWithFD` here in this change though, and probably won't in future, since these are just wrappers around `respond` (so less likely to be relevant for most raw header use cases) and they do some non-trivial messing around with the headers themselves (e.g. mutating with `statCheck` and injecting file size into content-length) which makes it a bit messy to support both raw & object headers there.

<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: doc/api/http2.md
patch: @@ -1082,6 +1082,11 @@ changes:
     pr-url: https://github.com/nodejs/node/pull/58313
     description: Following the deprecation of priority signaling as of RFC 9113,
                  `weight` option is deprecated.
+  - version:
+      - v24.0.0
+      - v22.17.0
+    pr-url: https://github.com/nodejs/node/pull/57917
+    description: Allow passing headers in raw array format.
 -->
 
 * `headers` {HTTP/2 Headers Object|Array}
@@ -1856,14 +1861,18 @@ and will throw an error.
 <!-- YAML
 added: v8.4.0
 changes:
+  - version:
+    - REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/59455
+    description: Allow passing headers in raw array format.
   - version:
     - v14.5.0
     - v12.19.0
     pr-url: https://github.com/nodejs/node/pull/33160
     description: Allow explicitly setting date headers.
 -->
 
-* `headers` {HTTP/2 Headers Object}
+* `headers` {HTTP/2 Headers Object|Array}
 * `options` {Object}
   * `endStream` {boolean} Set to `true` to indicate that the response will not
     include payload data.

filename: lib/internal/http2/core.js
patch: @@ -2541,8 +2541,31 @@ function callStreamClose(stream) {
   stream.close();
 }
 
-function processHeaders(oldHeaders, options) {
-  assertIsObject(oldHeaders, 'headers');
+function prepareResponseHeaders(stream, headersParam, options) {
+  let headers;
+  let statusCode;
+
+  if (ArrayIsArray(headersParam)) {
+    ({
+      headers,
+      statusCode,
+    } = prepareResponseHeadersArray(headersParam, options));
+    stream[kRawHeaders] = headers;
+  } else {
+    ({
+      headers,
+      statusCode,
+    } = prepareResponseHeadersObject(headersParam, options));
+    stream[kSentHeaders] = headers;
+  }
+
+  const headersList = buildNgHeaderString(headers, assertValidPseudoHeaderResponse);
+
+  return { headers, headersList, statusCode };
+}
+
+function prepareResponseHeadersObject(oldHeaders, options) {
+  assertIsObject(oldHeaders, 'headers', ['Object', 'Array']);
   const headers = { __proto__: null };
 
   if (oldHeaders !== null && oldHeaders !== undefined) {
@@ -2563,23 +2586,58 @@ function processHeaders(oldHeaders, options) {
     headers[HTTP2_HEADER_DATE] ??= utcDate();
   }
 
+  validatePreparedResponseHeaders(headers, statusCode);
+
+  return {
+    headers,
+    statusCode: headers[HTTP2_HEADER_STATUS],
+  };
+}
+
+function prepareResponseHeadersArray(headers, options) {
+  let statusCode;
+  let isDateSet = false;
+
+  for (let i = 0; i < headers.length; i += 2) {
+    const header = headers[i].toLowerCase();
+    const value = headers[i + 1];
+
+    if (header === HTTP2_HEADER_STATUS) {
+      statusCode = value | 0;
+    } else if (header === HTTP2_HEADER_DATE) {
+      isDateSet = true;
+    }
+  }
+
+  if (!statusCode) {
+    statusCode = HTTP_STATUS_OK;
+    headers.unshift(HTTP2_HEADER_STATUS, statusCode);
+  }
+
+  if (!isDateSet && (options.sendDate == null || options.sendDate)) {
+    headers.push(HTTP2_HEADER_DATE, utcDate());
+  }
+
+  validatePreparedResponseHeaders(headers, statusCode);
+
+  return { headers, statusCode };
+}
+
+function validatePreparedResponseHeaders(headers, statusCode) {
   // This is intentionally stricter than the HTTP/1 implementation, which
   // allows values between 100 and 999 (inclusive) in order to allow for
   // backwards compatibility with non-spec compliant code. With HTTP/2,
   // we have the opportunity to start fresh with stricter spec compliance.
   // This will have an impact on the compatibility layer for anyone using
   // non-standard, non-compliant status codes.
   if (statusCode < 200 || statusCode > 599)
-    throw new ERR_HTTP2_STATUS_INVALID(headers[HTTP2_HEADER_STATUS]);
+    throw new ERR_HTTP2_STATUS_INVALID(statusCode);
 
   const neverIndex = headers[kSensitiveHeaders];
   if (neverIndex !== undefined && !ArrayIsArray(neverIndex))
     throw new ERR_INVALID_ARG_VALUE('headers[http2.neverIndex]', neverIndex);
-
-  return headers;
 }
 
-
 function onFileUnpipe() {
   const stream = this.sink[kOwner];
   if (stream.ownsFd)
@@ -2882,7 +2940,7 @@ class ServerHttp2Stream extends Http2Stream {
   }
 
   // Initiate a response on this Http2Stream
-  respond(headers, options) {
+  respond(headersParam, options) {
     if (this.destroyed || this.closed)
       throw new ERR_HTTP2_INVALID_STREAM();
     if (this.headersSent)
@@ -2907,15 +2965,16 @@ class ServerHttp2Stream extends Http2Stream {
       state.flags |= STREAM_FLAGS_HAS_TRAILERS;
     }
 
-    headers = processHeaders(headers, options);
-    const headersList = buildNgHeaderString(headers, assertValidPseudoHeaderResponse);
-    this[kSentHeaders] = headers;
+    const {
+      headers,
+      headersList,
+      statusCode,
+    } = prepareResponseHeaders(this, headersParam, options);
 
     state.flags |= STREAM_FLAGS_HEADERS_SENT;
 
     // Close the writable side if the endStream option is set or status
     // is one of known codes with no payload, or it's a head request
-    const statusCode = headers[HTTP2_HEADER_STATUS] | 0;
     if (!!options.endStream ||
         statusCode === HTTP_STATUS_NO_CONTENT ||
         statusCode === HTTP_STATUS_RESET_CONTENT ||
@@ -2945,7 +3004,7 @@ class ServerHttp2Stream extends Http2Stream {
   // regular file, here the fd is passed directly. If the underlying
   // mechanism is not able to read from the fd, then the stream will be
   // reset with an error code.
-  respondWithFD(fd, headers, options) {
+  respondWithFD(fd, headersParam, options) {
     if (this.destroyed || this.closed)
       throw new ERR_HTTP2_INVALID_STREAM();
     if (this.headersSent)
@@ -2982,8 +3041,11 @@ class ServerHttp2Stream extends Http2Stream {
     this[kUpdateTimer]();
     this.ownsFd = false;
 
-    headers = processHeaders(headers, options);
-    const statusCode = headers[HTTP2_HEADER_STATUS] |= 0;
+    const {
+      headers,
+      statusCode,
+    } = prepareResponseHeadersObject(headersParam, options);
+
     // Payload/DATA frames are not permitted in these cases
     if (statusCode === HTTP_STATUS_NO_CONTENT ||
         statusCode === HTTP_STATUS_RESET_CONTENT ||
@@ -3011,7 +3073,7 @@ class ServerHttp2Stream extends Http2Stream {
   // giving the user an opportunity to verify the details and set additional
   // headers. If statCheck returns false, the operation is aborted and no
   // file details are sent.
-  respondWithFile(path, headers, options) {
+  respondWithFile(path, headersParam, options) {
     if (this.destroyed || this.closed)
       throw new ERR_HTTP2_INVALID_STREAM();
     if (this.headersSent)
@@ -3042,8 +3104,11 @@ class ServerHttp2Stream extends Http2Stream {
     this[kUpdateTimer]();
     this.ownsFd = true;
 
-    headers = processHeaders(headers, options);
-    const statusCode = headers[HTTP2_HEADER_STATUS] |= 0;
+    const {
+      headers,
+      statusCode,
+    } = prepareResponseHeadersObject(headersParam, options);
+
     // Payload/DATA frames are not permitted in these cases
     if (statusCode === HTTP_STATUS_NO_CONTENT ||
         statusCode === HTTP_STATUS_RESET_CONTENT ||

filename: lib/internal/http2/util.js
patch: @@ -690,7 +690,6 @@ function prepareRequestHeadersArray(headers, session) {
   const headersList = buildNgHeaderString(
     rawHeaders,
     assertValidPseudoHeader,
-    headers[kSensitiveHeaders],
   );
 
   return {
@@ -755,14 +754,14 @@ const kNoHeaderFlags = StringFromCharCode(NGHTTP2_NV_FLAG_NONE);
  * @returns {[string, number]}
  */
 function buildNgHeaderString(arrayOrMap,
-                             assertValuePseudoHeader = assertValidPseudoHeader,
-                             sensitiveHeaders = arrayOrMap[kSensitiveHeaders]) {
+                             assertValuePseudoHeader = assertValidPseudoHeader) {
   let headers = '';
   let pseudoHeaders = '';
   let count = 0;
 
   const singles = new SafeSet();
-  const neverIndex = (sensitiveHeaders || emptyArray).map((v) => v.toLowerCase());
+  const sensitiveHeaders = arrayOrMap[kSensitiveHeaders] || emptyArray;
+  const neverIndex = sensitiveHeaders.map((v) => v.toLowerCase());
 
   function processHeader(key, value) {
     key = key.toLowerCase();

filename: test/parallel/test-http2-raw-headers-defaults.js
patch: @@ -0,0 +1,76 @@
+'use strict';
+
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+const assert = require('assert');
+const http2 = require('http2');
+
+{
+  const server = http2.createServer();
+  server.on('stream', common.mustCall((stream, _headers, _flags, rawHeaders) => {
+    assert.deepStrictEqual(rawHeaders, [
+      ':method', 'GET',
+      ':authority', `localhost:${server.address().port}`,
+      ':scheme', 'http',
+      ':path', '/',
+      'a', 'b',
+      'x-foo', 'bar', // Lowercased as required for HTTP/2
+      'a', 'c', // Duplicate header order preserved
+    ]);
+    stream.respond([
+      'x', '1',
+      'x-FOO', 'bar',
+      'x', '2',
+    ]);
+
+    assert.partialDeepStrictEqual(stream.sentHeaders, {
+      '__proto__': null,
+      ':status': 200,
+      'x': [ '1', '2' ],
+      'x-FOO': 'bar',
+    });
+
+    assert.strictEqual(typeof stream.sentHeaders.date, 'string');
+
+    stream.end();
+  }));
+
+
+  server.listen(0, common.mustCall(() => {
+    const port = server.address().port;
+    const client = http2.connect(`http://localhost:${port}`);
+
+    const req = client.request([
+      'a', 'b',
+      'x-FOO', 'bar',
+      'a', 'c',
+    ]).end();
+
+    assert.deepStrictEqual(req.sentHeaders, {
+      '__proto__': null,
+      ':path': '/',
+      ':scheme': 'http',
+      ':authority': `localhost:${server.address().port}`,
+      ':method': 'GET',
+      'a': [ 'b', 'c' ],
+      'x-FOO': 'bar',
+    });
+
+    req.on('response', common.mustCall((_headers, _flags, rawHeaders) => {
+      assert.strictEqual(rawHeaders.length, 10);
+      assert.deepStrictEqual(rawHeaders.slice(0, 8), [
+        ':status', '200',
+        'x', '1',
+        'x-foo', 'bar', // Lowercased as required for HTTP/2
+        'x', '2', // Duplicate header order preserved
+      ]);
+
+      assert.strictEqual(rawHeaders[8], 'date');
+      assert.strictEqual(typeof rawHeaders[9], 'string');
+
+      client.close();
+      server.close();
+    }));
+  }));
+}

filename: test/parallel/test-http2-raw-headers.js
patch: @@ -8,19 +8,33 @@ const http2 = require('http2');
 
 {
   const server = http2.createServer();
-  server.on('stream', common.mustCall((stream, headers, flags, rawHeaders) => {
+  server.on('stream', common.mustCall((stream, _headers, _flags, rawHeaders) => {
     assert.deepStrictEqual(rawHeaders, [
       ':path', '/foobar',
       ':scheme', 'http',
-      ':authority', `localhost:${server.address().port}`,
-      ':method', 'GET',
+      ':authority', `test.invalid:${server.address().port}`,
+      ':method', 'POST',
       'a', 'b',
-      'x-foo', 'bar',
-      'a', 'c',
+      'x-foo', 'bar', // Lowercased as required for HTTP/2
+      'a', 'c', // Duplicate header order preserved
+    ]);
+
+    stream.respond([
+      ':status', '404',
+      'x', '1',
+      'x-FOO', 'bar',
+      'x', '2',
+      'DATE', '0000',
     ]);
-    stream.respond({
-      ':status': 200
+
+    assert.deepStrictEqual(stream.sentHeaders, {
+      '__proto__': null,
+      ':status': '404',
+      'x': [ '1', '2' ],
+      'x-FOO': 'bar',
+      'DATE': '0000',
     });
+
     stream.end();
   }));
 
@@ -32,8 +46,8 @@ const http2 = require('http2');
     const req = client.request([
       ':path', '/foobar',
       ':scheme', 'http',
-      ':authority', `localhost:${server.address().port}`,
-      ':method', 'GET',
+      ':authority', `test.invalid:${server.address().port}`,
+      ':method', 'POST',
       'a', 'b',
       'x-FOO', 'bar',
       'a', 'c',
@@ -43,14 +57,20 @@ const http2 = require('http2');
       '__proto__': null,
       ':path': '/foobar',
       ':scheme': 'http',
-      ':authority': `localhost:${server.address().port}`,
-      ':method': 'GET',
+      ':authority': `test.invalid:${server.address().port}`,
+      ':method': 'POST',
       'a': [ 'b', 'c' ],
       'x-FOO': 'bar',
     });
 
-    req.on('response', common.mustCall((headers) => {
-      assert.strictEqual(headers[':status'], 200);
+    req.on('response', common.mustCall((_headers, _flags, rawHeaders) => {
+      assert.deepStrictEqual(rawHeaders, [
+        ':status', '404',
+        'x', '1',
+        'x-foo', 'bar', // Lowercased as required for HTTP/2
+        'x', '2', // Duplicate header order preserved
+        'date', '0000', // Server doesn't automatically set its own value
+      ]);
       client.close();
       server.close();
     }));