From d354e2985aaddad96de15a19c47dfeff84d32919 Mon Sep 17 00:00:00 2001 From: Tim Perry Date: Tue, 12 Aug 2025 18:10:06 +0200 Subject: [PATCH 1/5] http2: add support for raw header arrays in h2Stream.respond() --- doc/api/http2.md | 2 +- lib/internal/http2/core.js | 98 +++++++++++++++++++++---- lib/internal/http2/util.js | 7 +- test/parallel/test-http2-raw-headers.js | 26 +++++-- 4 files changed, 107 insertions(+), 26 deletions(-) diff --git a/doc/api/http2.md b/doc/api/http2.md index decbdecf6e550c..f8f7a84bdca026 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -1863,7 +1863,7 @@ changes: 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. diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 6bf2edd1487d49..7a92fdd3166004 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -2541,7 +2541,30 @@ function callStreamClose(stream) { stream.close(); } -function processHeaders(oldHeaders, options) { +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'); const headers = { __proto__: null }; @@ -2576,9 +2599,51 @@ function processHeaders(oldHeaders, options) { if (neverIndex !== undefined && !ArrayIsArray(neverIndex)) throw new ERR_INVALID_ARG_VALUE('headers[http2.neverIndex]', neverIndex); - return headers; + 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()); + } + + // 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(statusCode); + + const neverIndex = headers[kSensitiveHeaders]; + if (neverIndex !== undefined && !ArrayIsArray(neverIndex)) + throw new ERR_INVALID_ARG_VALUE('headers[http2.neverIndex]', neverIndex); + + return { headers, statusCode }; +} function onFileUnpipe() { const stream = this.sink[kOwner]; @@ -2882,7 +2947,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 +2972,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 +3011,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 +3048,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 +3080,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 +3111,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 || diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index 19cbc08f8a9c7d..77e2386c1bf4aa 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -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(); diff --git a/test/parallel/test-http2-raw-headers.js b/test/parallel/test-http2-raw-headers.js index 8a84542a130fae..207e7322d20601 100644 --- a/test/parallel/test-http2-raw-headers.js +++ b/test/parallel/test-http2-raw-headers.js @@ -8,19 +8,23 @@ 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', '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', '200', + 'x', '1', + 'x-FOO', 'bar', + 'x', '2', + 'DATE', '0000', ]); - stream.respond({ - ':status': 200 - }); stream.end(); })); @@ -49,8 +53,14 @@ const http2 = require('http2'); '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', '200', + '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(); })); From b347c7c47ee129762546fb285a5405246c3e33c6 Mon Sep 17 00:00:00 2001 From: Tim Perry Date: Wed, 13 Aug 2025 11:19:51 +0200 Subject: [PATCH 2/5] Expand HTTP/2 raw tests to cover request & respond defaults --- .../test-http2-raw-headers-defaults.js | 66 +++++++++++++++++++ test/parallel/test-http2-raw-headers.js | 26 +++++--- 2 files changed, 84 insertions(+), 8 deletions(-) create mode 100644 test/parallel/test-http2-raw-headers-defaults.js diff --git a/test/parallel/test-http2-raw-headers-defaults.js b/test/parallel/test-http2-raw-headers-defaults.js new file mode 100644 index 00000000000000..78fb4d7a007844 --- /dev/null +++ b/test/parallel/test-http2-raw-headers-defaults.js @@ -0,0 +1,66 @@ +'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', + ]); + 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(); + })); + })); +} diff --git a/test/parallel/test-http2-raw-headers.js b/test/parallel/test-http2-raw-headers.js index 207e7322d20601..a77fe2db515962 100644 --- a/test/parallel/test-http2-raw-headers.js +++ b/test/parallel/test-http2-raw-headers.js @@ -12,19 +12,29 @@ const http2 = require('http2'); 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', // Lowercased as required for HTTP/2 'a', 'c', // Duplicate header order preserved ]); + stream.respond([ - ':status', '200', + ':status', '404', 'x', '1', 'x-FOO', 'bar', 'x', '2', 'DATE', '0000', ]); + + assert.deepStrictEqual(stream.sentHeaders, { + '__proto__': null, + ':status': '404', + 'x': [ '1', '2' ], + 'x-FOO': 'bar', + 'DATE': '0000', + }); + stream.end(); })); @@ -36,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', @@ -47,15 +57,15 @@ 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, _flags, rawHeaders) => { assert.deepStrictEqual(rawHeaders, [ - ':status', '200', + ':status', '404', 'x', '1', 'x-foo', 'bar', // Lowercased as required for HTTP/2 'x', '2', // Duplicate header order preserved From e3fe9beac31d62957fa84ccce13f1ed109b92385 Mon Sep 17 00:00:00 2001 From: Tim Perry Date: Thu, 14 Aug 2025 19:54:53 +0200 Subject: [PATCH 3/5] Add 'changes' to h2Stream.respond() for raw headers --- doc/api/http2.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/api/http2.md b/doc/api/http2.md index f8f7a84bdca026..54294cdcb62b28 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -1856,6 +1856,10 @@ and will throw an error. * `headers` {HTTP/2 Headers Object|Array} From 4df82be40a166197abab0c7768326e97423ad110 Mon Sep 17 00:00:00 2001 From: Tim Perry Date: Tue, 19 Aug 2025 14:09:45 +0200 Subject: [PATCH 5/5] Handle review comments on respond header prep & sentHeader testing --- lib/internal/http2/core.js | 23 +++++++------------ .../test-http2-raw-headers-defaults.js | 10 ++++++++ 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 7a92fdd3166004..6ed61c90f0c6ae 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -2565,7 +2565,7 @@ function prepareResponseHeaders(stream, headersParam, options) { } function prepareResponseHeadersObject(oldHeaders, options) { - assertIsObject(oldHeaders, 'headers'); + assertIsObject(oldHeaders, 'headers', ['Object', 'Array']); const headers = { __proto__: null }; if (oldHeaders !== null && oldHeaders !== undefined) { @@ -2586,18 +2586,7 @@ function prepareResponseHeadersObject(oldHeaders, options) { headers[HTTP2_HEADER_DATE] ??= utcDate(); } - // 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]); - - const neverIndex = headers[kSensitiveHeaders]; - if (neverIndex !== undefined && !ArrayIsArray(neverIndex)) - throw new ERR_INVALID_ARG_VALUE('headers[http2.neverIndex]', neverIndex); + validatePreparedResponseHeaders(headers, statusCode); return { headers, @@ -2629,6 +2618,12 @@ function prepareResponseHeadersArray(headers, options) { 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, @@ -2641,8 +2636,6 @@ function prepareResponseHeadersArray(headers, options) { const neverIndex = headers[kSensitiveHeaders]; if (neverIndex !== undefined && !ArrayIsArray(neverIndex)) throw new ERR_INVALID_ARG_VALUE('headers[http2.neverIndex]', neverIndex); - - return { headers, statusCode }; } function onFileUnpipe() { diff --git a/test/parallel/test-http2-raw-headers-defaults.js b/test/parallel/test-http2-raw-headers-defaults.js index 78fb4d7a007844..fa742605a02a9a 100644 --- a/test/parallel/test-http2-raw-headers-defaults.js +++ b/test/parallel/test-http2-raw-headers-defaults.js @@ -23,6 +23,16 @@ const http2 = require('http2'); '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(); }));