From 43bf1f619acfdfe437a2568387b4304177046e44 Mon Sep 17 00:00:00 2001 From: Tim Perry Date: Thu, 17 Apr 2025 19:16:52 +0200 Subject: [PATCH] http2: add raw header array support to h2Session.request() This also notably changes error handling for request(). Previously some invalid header values (but not all) would cause the session to be unnecessarily destroyed automatically, e.g. passing an unparseable header name to request(). This is no longer the case: header validation failures will throw an error, but will not destroy the session or emit 'error' events. PR-URL: https://github.com/nodejs/node/pull/57917 Reviewed-By: Matteo Collina Reviewed-By: James M Snell --- doc/api/http2.md | 2 +- lib/internal/http2/core.js | 112 +++++----- lib/internal/http2/util.js | 202 +++++++++++++++--- src/node_http_common.h | 2 +- .../parallel/test-http2-invalidheaderfield.js | 21 +- .../test-http2-invalidheaderfields-client.js | 6 +- test/parallel/test-http2-raw-headers.js | 48 +++++ test/parallel/test-http2-sensitive-headers.js | 38 ++++ ...st-http2-util-assert-valid-pseudoheader.js | 18 +- test/parallel/test-http2-util-headers-list.js | 32 +-- 10 files changed, 352 insertions(+), 129 deletions(-) create mode 100644 test/parallel/test-http2-raw-headers.js diff --git a/doc/api/http2.md b/doc/api/http2.md index 1eb27ad19ea..ab18f0ca102 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -1078,7 +1078,7 @@ changes: `weight` option is deprecated. --> -* `headers` {HTTP/2 Headers Object} +* `headers` {HTTP/2 Headers Object} | {Array} * `options` {Object} * `endStream` {boolean} `true` if the `Http2Stream` _writable_ side should diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 767edafc791..b6504eefaf3 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -9,7 +9,6 @@ const { ObjectDefineProperty, ObjectEntries, ObjectHasOwn, - ObjectKeys, Promise, Proxy, ReflectApply, @@ -47,10 +46,7 @@ const { Duplex } = require('stream'); const tls = require('tls'); const { setImmediate, setTimeout, clearTimeout } = require('timers'); -const { - kIncomingMessage, - _checkIsHttpToken: checkIsHttpToken, -} = require('_http_common'); +const { kIncomingMessage } = require('_http_common'); const { kServerResponse, Server: HttpServer, httpServerPreClose, setupConnectionsTracking } = require('_http_server'); const JSStreamSocket = require('internal/js_stream_socket'); @@ -69,9 +65,6 @@ const { codes: { ERR_HTTP2_ALTSVC_INVALID_ORIGIN, ERR_HTTP2_ALTSVC_LENGTH, - ERR_HTTP2_CONNECT_AUTHORITY, - ERR_HTTP2_CONNECT_PATH, - ERR_HTTP2_CONNECT_SCHEME, ERR_HTTP2_GOAWAY_SESSION, ERR_HTTP2_HEADERS_AFTER_RESPOND, ERR_HTTP2_HEADERS_SENT, @@ -109,7 +102,6 @@ const { ERR_INVALID_ARG_TYPE, ERR_INVALID_ARG_VALUE, ERR_INVALID_CHAR, - ERR_INVALID_HTTP_TOKEN, ERR_OUT_OF_RANGE, ERR_SOCKET_CLOSED, }, @@ -138,23 +130,26 @@ const { const { assertIsObject, assertIsArray, - assertValidPseudoHeader, assertValidPseudoHeaderResponse, assertValidPseudoHeaderTrailer, assertWithinRange, + buildNgHeaderString, getAuthority, getDefaultSettings, getSessionState, getSettings, getStreamState, isPayloadMeaningless, + kAuthority, kSensitiveHeaders, kSocket, kRequest, + kProtocol, kProxySocket, - mapToHeaders, MAX_ADDITIONAL_SETTINGS, NghttpError, + prepareRequestHeadersArray, + prepareRequestHeadersObject, remoteCustomSettingsToBuffer, sessionName, toHeaderObject, @@ -242,7 +237,6 @@ const NETServer = net.Server; const TLSServer = tls.Server; const kAlpnProtocol = Symbol('alpnProtocol'); -const kAuthority = Symbol('authority'); const kEncrypted = Symbol('encrypted'); const kID = Symbol('id'); const kInit = Symbol('init'); @@ -254,7 +248,6 @@ const kOwner = owner_symbol; const kOrigin = Symbol('origin'); const kPendingRequestCalls = Symbol('kPendingRequestCalls'); const kProceed = Symbol('proceed'); -const kProtocol = Symbol('protocol'); const kRemoteSettings = Symbol('remote-settings'); const kRequestAsyncResource = Symbol('requestAsyncResource'); const kSentHeaders = Symbol('sent-headers'); @@ -297,7 +290,6 @@ const { HTTP2_HEADER_DATE, HTTP2_HEADER_METHOD, HTTP2_HEADER_PATH, - HTTP2_HEADER_PROTOCOL, HTTP2_HEADER_SCHEME, HTTP2_HEADER_STATUS, HTTP2_HEADER_CONTENT_LENGTH, @@ -312,7 +304,6 @@ const { HTTP2_METHOD_GET, HTTP2_METHOD_HEAD, - HTTP2_METHOD_CONNECT, HTTP_STATUS_CONTINUE, HTTP_STATUS_RESET_CONTENT, @@ -1808,7 +1799,7 @@ class ClientHttp2Session extends Http2Session { // Submits a new HTTP2 request to the connected peer. Returns the // associated Http2Stream instance. - request(headers, options) { + request(headersParam, options) { debugSessionObj(this, 'initiating request'); if (this.destroyed) @@ -1819,62 +1810,61 @@ class ClientHttp2Session extends Http2Session { this[kUpdateTimer](); - if (headers !== null && headers !== undefined) { - const keys = ObjectKeys(headers); - for (let i = 0; i < keys.length; i++) { - const header = keys[i]; - if (header[0] === ':') { - assertValidPseudoHeader(header); - } else if (header && !checkIsHttpToken(header)) - this.destroy(new ERR_INVALID_HTTP_TOKEN('Header name', header)); - } - } + let headersList; + let headersObject; + let scheme; + let authority; + let method; - assertIsObject(headers, 'headers'); - assertIsObject(options, 'options'); - - headers = ObjectAssign({ __proto__: null }, headers); - options = { ...options }; - - if (headers[HTTP2_HEADER_METHOD] === undefined) - headers[HTTP2_HEADER_METHOD] = HTTP2_METHOD_GET; - - const connect = headers[HTTP2_HEADER_METHOD] === HTTP2_METHOD_CONNECT; - - if (!connect || headers[HTTP2_HEADER_PROTOCOL] !== undefined) { - if (getAuthority(headers) === undefined) - headers[HTTP2_HEADER_AUTHORITY] = this[kAuthority]; - if (headers[HTTP2_HEADER_SCHEME] === undefined) - headers[HTTP2_HEADER_SCHEME] = this[kProtocol].slice(0, -1); - if (headers[HTTP2_HEADER_PATH] === undefined) - headers[HTTP2_HEADER_PATH] = '/'; + if (ArrayIsArray(headersParam)) { + ({ + headersList, + scheme, + authority, + method, + } = prepareRequestHeadersArray(headersParam, this)); + } else if (!!headersParam && typeof headersParam === 'object') { + ({ + headersObject, + headersList, + scheme, + authority, + method, + } = prepareRequestHeadersObject(headersParam, this)); + } else if (headersParam === undefined) { + ({ + headersObject, + headersList, + scheme, + authority, + method, + } = prepareRequestHeadersObject({}, this)); } else { - if (headers[HTTP2_HEADER_AUTHORITY] === undefined) - throw new ERR_HTTP2_CONNECT_AUTHORITY(); - if (headers[HTTP2_HEADER_SCHEME] !== undefined) - throw new ERR_HTTP2_CONNECT_SCHEME(); - if (headers[HTTP2_HEADER_PATH] !== undefined) - throw new ERR_HTTP2_CONNECT_PATH(); + throw new ERR_INVALID_ARG_TYPE.HideStackFramesError( + 'headers', + ['Object', 'Array'], + headersParam, + ); } + assertIsObject(options, 'options'); + options = { ...options }; + setAndValidatePriorityOptions(options); if (options.endStream === undefined) { // For some methods, we know that a payload is meaningless, so end the // stream by default if the user has not specifically indicated a // preference. - options.endStream = isPayloadMeaningless(headers[HTTP2_HEADER_METHOD]); + options.endStream = isPayloadMeaningless(method); } else { validateBoolean(options.endStream, 'options.endStream'); } - const headersList = mapToHeaders(headers); - // eslint-disable-next-line no-use-before-define const stream = new ClientHttp2Stream(this, undefined, undefined, {}); - stream[kSentHeaders] = headers; - stream[kOrigin] = `${headers[HTTP2_HEADER_SCHEME]}://` + - `${getAuthority(headers)}`; + stream[kSentHeaders] = headersObject; // N.b. Only set for object headers, not raw headers + stream[kOrigin] = `${scheme}://${authority}`; const reqAsync = new AsyncResource('PendingRequest'); stream[kRequestAsyncResource] = reqAsync; @@ -2350,7 +2340,7 @@ class Http2Stream extends Duplex { this[kUpdateTimer](); - const headersList = mapToHeaders(headers, assertValidPseudoHeaderTrailer); + const headersList = buildNgHeaderString(headers, assertValidPseudoHeaderTrailer); this[kSentTrailers] = headers; // Send the trailers in setImmediate so we don't do it on nghttp2 stack. @@ -2598,7 +2588,7 @@ function processRespondWithFD(self, fd, headers, offset = 0, length = -1, let headersList; try { - headersList = mapToHeaders(headers, assertValidPseudoHeaderResponse); + headersList = buildNgHeaderString(headers, assertValidPseudoHeaderResponse); } catch (err) { self.destroy(err); return; @@ -2822,7 +2812,7 @@ class ServerHttp2Stream extends Http2Stream { if (headers[HTTP2_HEADER_METHOD] === HTTP2_METHOD_HEAD) headRequest = options.endStream = true; - const headersList = mapToHeaders(headers); + const headersList = buildNgHeaderString(headers); const streamOptions = options.endStream ? STREAM_OPTION_EMPTY_PAYLOAD : 0; @@ -2902,7 +2892,7 @@ class ServerHttp2Stream extends Http2Stream { } headers = processHeaders(headers, options); - const headersList = mapToHeaders(headers, assertValidPseudoHeaderResponse); + const headersList = buildNgHeaderString(headers, assertValidPseudoHeaderResponse); this[kSentHeaders] = headers; state.flags |= STREAM_FLAGS_HEADERS_SENT; @@ -3079,7 +3069,7 @@ class ServerHttp2Stream extends Http2Stream { this[kUpdateTimer](); - const headersList = mapToHeaders(headers, assertValidPseudoHeaderResponse); + const headersList = buildNgHeaderString(headers, assertValidPseudoHeaderResponse); if (!this[kInfoHeaders]) this[kInfoHeaders] = [headers]; else diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index 4062dcef180..396623d3b9d 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -6,6 +6,7 @@ const { MathMax, Number, NumberIsNaN, + ObjectAssign, ObjectKeys, SafeSet, String, @@ -13,9 +14,16 @@ const { Symbol, } = primordials; +const { + _checkIsHttpToken: checkIsHttpToken, +} = require('_http_common'); + const binding = internalBinding('http2'); const { codes: { + ERR_HTTP2_CONNECT_AUTHORITY, + ERR_HTTP2_CONNECT_PATH, + ERR_HTTP2_CONNECT_SCHEME, ERR_HTTP2_HEADER_SINGLE_VALUE, ERR_HTTP2_INVALID_CONNECTION_HEADERS, ERR_HTTP2_INVALID_PSEUDOHEADER: { HideStackFramesError: ERR_HTTP2_INVALID_PSEUDOHEADER }, @@ -29,8 +37,10 @@ const { kIsNodeError, } = require('internal/errors'); +const kAuthority = Symbol('authority'); const kSensitiveHeaders = Symbol('sensitiveHeaders'); const kSocket = Symbol('socket'); +const kProtocol = Symbol('protocol'); const kProxySocket = Symbol('proxySocket'); const kRequest = Symbol('request'); @@ -91,6 +101,7 @@ const { HTTP2_HEADER_KEEP_ALIVE, HTTP2_HEADER_PROXY_CONNECTION, + HTTP2_METHOD_CONNECT, HTTP2_METHOD_DELETE, HTTP2_METHOD_GET, HTTP2_METHOD_HEAD, @@ -601,35 +612,155 @@ const assertValidPseudoHeaderTrailer = hideStackFrames((key) => { throw new ERR_HTTP2_INVALID_PSEUDOHEADER(key); }); +/** + * Takes a request headers array, validates it and sets defaults, and returns + * the resulting headers in NgHeaders string list format. + */ +function prepareRequestHeadersArray(headers, session) { + let method; + let scheme; + let authority; + let path; + let protocol; + + // Extract the key psuedo header values from the headers array + for (let i = 0; i < headers.length; i += 2) { + if (headers[i][0] !== ':') { + continue; + } + + const header = headers[i].toLowerCase(); + const value = headers[i + 1]; + + if (header === HTTP2_HEADER_METHOD) { + method = value; + } else if (header === HTTP2_HEADER_SCHEME) { + scheme = value; + } else if (header === HTTP2_HEADER_AUTHORITY) { + authority = value; + } else if (header === HTTP2_HEADER_PATH) { + path = value; + } else if (header === HTTP2_HEADER_PROTOCOL) { + protocol = value; + } + } + + // We then build an array of any missing pseudo headers, to prepend + // default values to the given header array: + const additionalPsuedoHeaders = []; + + if (method === undefined) { + method = HTTP2_METHOD_GET; + additionalPsuedoHeaders.push(HTTP2_HEADER_METHOD, method); + } + + const connect = method === HTTP2_METHOD_CONNECT; + + if (!connect || protocol !== undefined) { + if (authority === undefined && headers[HTTP2_HEADER_HOST] === undefined) { + authority = session[kAuthority]; + additionalPsuedoHeaders.push(HTTP2_HEADER_AUTHORITY, authority); + } + if (scheme === undefined) { + scheme = session[kProtocol].slice(0, -1); + additionalPsuedoHeaders.push(HTTP2_HEADER_SCHEME, scheme); + } + if (path === undefined) { + additionalPsuedoHeaders.push(HTTP2_HEADER_PATH, '/'); + } + } else { + if (authority === undefined) + throw new ERR_HTTP2_CONNECT_AUTHORITY(); + if (scheme !== undefined) + throw new ERR_HTTP2_CONNECT_SCHEME(); + if (path !== undefined) + throw new ERR_HTTP2_CONNECT_PATH(); + } + + const headersList = buildNgHeaderString( + additionalPsuedoHeaders.length ? + additionalPsuedoHeaders.concat(headers) : + headers, + assertValidPseudoHeader, + headers[kSensitiveHeaders], + ); + + return { + headersList, + scheme, + authority: authority ?? headers[HTTP2_HEADER_HOST], + method, + }; +} + +/** + * Takes a request headers object, validates it and sets defaults, and returns + * the resulting headers in object format and NgHeaders string list format. + */ +function prepareRequestHeadersObject(headers, session) { + const headersObject = ObjectAssign({ __proto__: null }, headers); + + if (headersObject[HTTP2_HEADER_METHOD] === undefined) { + headersObject[HTTP2_HEADER_METHOD] = HTTP2_METHOD_GET; + } + + const connect = headersObject[HTTP2_HEADER_METHOD] === HTTP2_METHOD_CONNECT; + + if (!connect || headersObject[HTTP2_HEADER_PROTOCOL] !== undefined) { + if (getAuthority(headersObject) === undefined) + headersObject[HTTP2_HEADER_AUTHORITY] = session[kAuthority]; + if (headersObject[HTTP2_HEADER_SCHEME] === undefined) + headersObject[HTTP2_HEADER_SCHEME] = session[kProtocol].slice(0, -1); + if (headersObject[HTTP2_HEADER_PATH] === undefined) + headersObject[HTTP2_HEADER_PATH] = '/'; + } else { + if (headersObject[HTTP2_HEADER_AUTHORITY] === undefined) + throw new ERR_HTTP2_CONNECT_AUTHORITY(); + if (headersObject[HTTP2_HEADER_SCHEME] !== undefined) + throw new ERR_HTTP2_CONNECT_SCHEME(); + if (headersObject[HTTP2_HEADER_PATH] !== undefined) + throw new ERR_HTTP2_CONNECT_PATH(); + } + + const headersList = buildNgHeaderString(headersObject); + + return { + headersObject, + headersList, + scheme: headersObject[HTTP2_HEADER_SCHEME], + authority: getAuthority(headersObject), + method: headersObject[HTTP2_HEADER_METHOD], + }; +} + const emptyArray = []; const kNeverIndexFlag = StringFromCharCode(NGHTTP2_NV_FLAG_NO_INDEX); const kNoHeaderFlags = StringFromCharCode(NGHTTP2_NV_FLAG_NONE); -function mapToHeaders(map, - assertValuePseudoHeader = assertValidPseudoHeader) { + +/** + * Builds an NgHeader string + header count value, validating the header key + * format, rejecting illegal header configurations, and marking sensitive headers + * that should not be indexed en route. This takes either a flat map of + * raw headers ([k1, v1, k2, v2]) or a header object ({ k1: v1, k2: [v2, v3] }). + */ +function buildNgHeaderString(arrayOrMap, + assertValuePseudoHeader = assertValidPseudoHeader, + sensitiveHeaders = arrayOrMap[kSensitiveHeaders]) { let headers = ''; let pseudoHeaders = ''; let count = 0; - const keys = ObjectKeys(map); + const singles = new SafeSet(); - let i, j; - let isArray; - let key; - let value; - let isSingleValueHeader; - let err; - const neverIndex = (map[kSensitiveHeaders] || emptyArray).map((v) => v.toLowerCase()); - for (i = 0; i < keys.length; ++i) { - key = keys[i]; - value = map[key]; - if (value === undefined || key === '') - continue; + const neverIndex = (sensitiveHeaders || emptyArray).map((v) => v.toLowerCase()); + + function processHeader(key, value) { key = key.toLowerCase(); - isSingleValueHeader = kSingleValueHeaders.has(key); - isArray = ArrayIsArray(value); + const isSingleValueHeader = kSingleValueHeaders.has(key); + let isArray = ArrayIsArray(value); if (isArray) { switch (value.length) { case 0: - continue; + return; case 1: value = String(value[0]); isArray = false; @@ -650,31 +781,50 @@ function mapToHeaders(map, kNeverIndexFlag : kNoHeaderFlags; if (key[0] === ':') { - err = assertValuePseudoHeader(key); + const err = assertValuePseudoHeader(key); if (err !== undefined) throw err; pseudoHeaders += `${key}\0${value}\0${flags}`; count++; - continue; + return; } - if (key.includes(' ')) { + if (!checkIsHttpToken(key)) { throw new ERR_INVALID_HTTP_TOKEN('Header name', key); } if (isIllegalConnectionSpecificHeader(key, value)) { throw new ERR_HTTP2_INVALID_CONNECTION_HEADERS(key); } if (isArray) { - for (j = 0; j < value.length; ++j) { + for (let j = 0; j < value.length; ++j) { const val = String(value[j]); headers += `${key}\0${val}\0${flags}`; } count += value.length; - continue; + return; } headers += `${key}\0${value}\0${flags}`; count++; } + if (ArrayIsArray(arrayOrMap)) { + for (let i = 0; i < arrayOrMap.length; i += 2) { + const key = arrayOrMap[i]; + const value = arrayOrMap[i + 1]; + if (value === undefined || key === '') + continue; + processHeader(key, value); + } + } else { + const keys = ObjectKeys(arrayOrMap); + for (let i = 0; i < keys.length; ++i) { + const key = keys[i]; + const value = arrayOrMap[key]; + if (value === undefined || key === '') + continue; + processHeader(key, value); + } + } + return [pseudoHeaders + headers, count]; } @@ -801,19 +951,23 @@ module.exports = { assertValidPseudoHeaderResponse, assertValidPseudoHeaderTrailer, assertWithinRange, + buildNgHeaderString, getAuthority, getDefaultSettings, getSessionState, getSettings, getStreamState, isPayloadMeaningless, + kAuthority, kSensitiveHeaders, kSocket, + kProtocol, kProxySocket, kRequest, - mapToHeaders, MAX_ADDITIONAL_SETTINGS, NghttpError, + prepareRequestHeadersArray, + prepareRequestHeadersObject, remoteCustomSettingsToBuffer, sessionName, toHeaderObject, diff --git a/src/node_http_common.h b/src/node_http_common.h index e2e13e9972f..5afb6b97bec 100644 --- a/src/node_http_common.h +++ b/src/node_http_common.h @@ -240,7 +240,7 @@ enum http_status_codes { V(VERSION_CONTROL, "VERSION-CONTROL") // NgHeaders takes as input a block of headers provided by the -// JavaScript side (see http2's mapToHeaders function) and +// JavaScript side (see http2's buildNgHeaderString function) and // converts it into a array of ng header structs. This is done // generically to handle both http/2 and (in the future) http/3, // which use nearly identical structs. The template parameter diff --git a/test/parallel/test-http2-invalidheaderfield.js b/test/parallel/test-http2-invalidheaderfield.js index bc18f2ba426..d6799160f3f 100644 --- a/test/parallel/test-http2-invalidheaderfield.js +++ b/test/parallel/test-http2-invalidheaderfield.js @@ -8,7 +8,7 @@ if (!common.hasCrypto) { common.skip('missing crypto'); } // Capitalized headers const http2 = require('http2'); -const { throws, strictEqual } = require('assert'); +const { throws } = require('assert'); { const server = http2.createServer(common.mustCall((req, res) => { @@ -42,45 +42,40 @@ const { throws, strictEqual } = require('assert'); const server = http2.createServer(); server.listen(0, common.mustCall(() => { const session = http2.connect(`http://localhost:${server.address().port}`); - session.on('error', common.mustCall((e) => { - strictEqual(e.code, 'ERR_INVALID_HTTP_TOKEN'); - server.close(); - })); throws(() => { session.request({ 't est': 123 }); }, { code: 'ERR_INVALID_HTTP_TOKEN' }); + session.close(); + server.close(); })); } - { const server = http2.createServer(); server.listen(0, common.mustCall(() => { const session = http2.connect(`http://localhost:${server.address().port}`); - session.on('error', common.mustCall((e) => { - strictEqual(e.code, 'ERR_INVALID_HTTP_TOKEN'); - server.close(); - })); throws(() => { session.request({ ' test': 123 }); }, { code: 'ERR_INVALID_HTTP_TOKEN' }); + session.close(); + server.close(); })); } { const server = http2.createServer(); server.listen(0, common.mustCall(() => { - const session4 = http2.connect(`http://localhost:${server.address().port}`); + const session = http2.connect(`http://localhost:${server.address().port}`); throws(() => { - session4.request({ ':test': 123 }); + session.request({ ':test': 123 }); }, { code: 'ERR_HTTP2_INVALID_PSEUDOHEADER' }); - session4.close(); + session.close(); server.close(); })); } diff --git a/test/parallel/test-http2-invalidheaderfields-client.js b/test/parallel/test-http2-invalidheaderfields-client.js index a5681970faa..cfca6c30b28 100644 --- a/test/parallel/test-http2-invalidheaderfields-client.js +++ b/test/parallel/test-http2-invalidheaderfields-client.js @@ -14,10 +14,8 @@ server1.listen(0, common.mustCall(() => { }, { code: 'ERR_INVALID_HTTP_TOKEN' }); - session.on('error', common.mustCall((e) => { - assert.strictEqual(e.code, 'ERR_INVALID_HTTP_TOKEN'); - server1.close(); - })); + session.close(); + server1.close(); })); const server2 = http2.createServer(common.mustCall((req, res) => { diff --git a/test/parallel/test-http2-raw-headers.js b/test/parallel/test-http2-raw-headers.js new file mode 100644 index 00000000000..cbcc73692b0 --- /dev/null +++ b/test/parallel/test-http2-raw-headers.js @@ -0,0 +1,48 @@ +'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, [ + ':path', '/foobar', + ':scheme', 'http', + ':authority', `localhost:${server.address().port}`, + ':method', 'GET', + 'a', 'b', + 'x-foo', 'bar', + 'a', 'c', + ]); + stream.respond({ + ':status': 200 + }); + stream.end(); + })); + + + server.listen(0, common.mustCall(() => { + const port = server.address().port; + const client = http2.connect(`http://localhost:${port}`); + + const req = client.request([ + ':path', '/foobar', + ':scheme', 'http', + ':authority', `localhost:${server.address().port}`, + ':method', 'GET', + 'a', 'b', + 'x-FOO', 'bar', + 'a', 'c', + ]).end(); + + req.on('response', common.mustCall((headers) => { + assert.strictEqual(headers[':status'], 200); + client.close(); + server.close(); + })); + })); +} diff --git a/test/parallel/test-http2-sensitive-headers.js b/test/parallel/test-http2-sensitive-headers.js index aadedc1ba83..bbd00ae0cdd 100644 --- a/test/parallel/test-http2-sensitive-headers.js +++ b/test/parallel/test-http2-sensitive-headers.js @@ -45,3 +45,41 @@ const { duplexPair } = require('stream'); req.resume(); req.end(); } + +{ + const server = http2.createServer(); + server.on('stream', common.mustCall((stream, headers) => { + assert.deepStrictEqual( + headers[http2.sensitiveHeaders], + ['secret'] + ); + stream.respond({ ':status': 200 }); + stream.end(); + })); + + const [ clientSide, serverSide ] = duplexPair(); + server.emit('connection', serverSide); + + const client = http2.connect('http://localhost:80', { + createConnection: common.mustCall(() => clientSide) + }); + + const rawHeaders = [ + ':path', '/', + 'secret', 'secret-value', + ]; + rawHeaders[http2.sensitiveHeaders] = ['secret']; + + const req = client.request(rawHeaders); + + req.on('response', common.mustCall((headers) => { + assert.strictEqual(headers[':status'], 200); + })); + + req.on('end', common.mustCall(() => { + clientSide.destroy(); + clientSide.end(); + })); + req.resume(); + req.end(); +} diff --git a/test/parallel/test-http2-util-assert-valid-pseudoheader.js b/test/parallel/test-http2-util-assert-valid-pseudoheader.js index f86793c69ef..bd995f0338f 100644 --- a/test/parallel/test-http2-util-assert-valid-pseudoheader.js +++ b/test/parallel/test-http2-util-assert-valid-pseudoheader.js @@ -5,19 +5,19 @@ require('../common'); const assert = require('assert'); // Tests the assertValidPseudoHeader function that is used within the -// mapToHeaders function. The assert function is not exported so we -// have to test it through mapToHeaders +// buildNgHeaderString function. The assert function is not exported so we +// have to test it through buildNgHeaderString -const { mapToHeaders } = require('internal/http2/util'); +const { buildNgHeaderString } = require('internal/http2/util'); // These should not throw -mapToHeaders({ ':status': 'a' }); -mapToHeaders({ ':path': 'a' }); -mapToHeaders({ ':authority': 'a' }); -mapToHeaders({ ':scheme': 'a' }); -mapToHeaders({ ':method': 'a' }); +buildNgHeaderString({ ':status': 'a' }); +buildNgHeaderString({ ':path': 'a' }); +buildNgHeaderString({ ':authority': 'a' }); +buildNgHeaderString({ ':scheme': 'a' }); +buildNgHeaderString({ ':method': 'a' }); -assert.throws(() => mapToHeaders({ ':foo': 'a' }), { +assert.throws(() => buildNgHeaderString({ ':foo': 'a' }), { code: 'ERR_HTTP2_INVALID_PSEUDOHEADER', name: 'TypeError', message: '":foo" is an invalid pseudoheader or is used incorrectly' diff --git a/test/parallel/test-http2-util-headers-list.js b/test/parallel/test-http2-util-headers-list.js index f4221f5c17e..0d1ac1d1b8c 100644 --- a/test/parallel/test-http2-util-headers-list.js +++ b/test/parallel/test-http2-util-headers-list.js @@ -10,7 +10,7 @@ if (!common.hasCrypto) const assert = require('assert'); const { getAuthority, - mapToHeaders, + buildNgHeaderString, toHeaderObject } = require('internal/http2/util'); const { sensitiveHeaders } = require('http2'); @@ -106,7 +106,7 @@ const { }; assert.deepStrictEqual( - mapToHeaders(headers), + buildNgHeaderString(headers), [ [ ':path', 'abc\0', ':status', '200\0', 'abc', '1\0', 'xyz', '1\0', 'xyz', '2\0', 'xyz', '3\0', 'xyz', '4\0', 'bar', '1\0', '' ].join('\0'), 8 ] @@ -123,7 +123,7 @@ const { }; assert.deepStrictEqual( - mapToHeaders(headers), + buildNgHeaderString(headers), [ [ ':status', '200\0', ':path', 'abc\0', 'abc', '1\0', 'xyz', '1\0', 'xyz', '2\0', 'xyz', '3\0', 'xyz', '4\0', '' ].join('\0'), 7 ] ); @@ -140,7 +140,7 @@ const { }; assert.deepStrictEqual( - mapToHeaders(headers), + buildNgHeaderString(headers), [ [ ':status', '200\0', ':path', 'abc\0', 'abc', '1\0', 'xyz', '1\0', 'xyz', '2\0', 'xyz', '3\0', 'xyz', '4\0', '' ].join('\0'), 7 ] ); @@ -156,7 +156,7 @@ const { headers[':path'] = 'abc'; assert.deepStrictEqual( - mapToHeaders(headers), + buildNgHeaderString(headers), [ [ ':status', '200\0', ':path', 'abc\0', 'xyz', '1\0', 'xyz', '2\0', 'xyz', '3\0', 'xyz', '4\0', '' ].join('\0'), 6 ] ); @@ -169,7 +169,7 @@ const { 'set-cookie': ['foo=bar'] }; assert.deepStrictEqual( - mapToHeaders(headers), + buildNgHeaderString(headers), [ [ 'set-cookie', 'foo=bar\0', '' ].join('\0'), 1 ] ); } @@ -181,7 +181,7 @@ const { ':statuS': 204, }; - assert.throws(() => mapToHeaders(headers), { + assert.throws(() => buildNgHeaderString(headers), { code: 'ERR_HTTP2_HEADER_SINGLE_VALUE', name: 'TypeError', message: 'Header field ":status" must only have a single value' @@ -199,7 +199,7 @@ const { }; assert.deepStrictEqual( - mapToHeaders(headers), + buildNgHeaderString(headers), [ ':status\x00200\x00\x00:path\x00abc\x00\x00abc\x001\x00\x00' + 'xyz\x001\x00\x01xyz\x002\x00\x01xyz\x003\x00\x01xyz\x004\x00\x01', 7 ] ); @@ -248,7 +248,7 @@ const { HTTP2_HEADER_X_CONTENT_TYPE_OPTIONS, ].forEach((name) => { const msg = `Header field "${name}" must only have a single value`; - assert.throws(() => mapToHeaders({ [name]: [1, 2, 3] }), { + assert.throws(() => buildNgHeaderString({ [name]: [1, 2, 3] }), { code: 'ERR_HTTP2_HEADER_SINGLE_VALUE', message: msg }); @@ -285,7 +285,7 @@ const { HTTP2_HEADER_WWW_AUTHENTICATE, HTTP2_HEADER_X_FRAME_OPTIONS, ].forEach((name) => { - assert(!(mapToHeaders({ [name]: [1, 2, 3] }) instanceof Error), name); + assert(!(buildNgHeaderString({ [name]: [1, 2, 3] }) instanceof Error), name); }); [ @@ -304,7 +304,7 @@ const { 'Proxy-Connection', 'Keep-Alive', ].forEach((name) => { - assert.throws(() => mapToHeaders({ [name]: 'abc' }), { + assert.throws(() => buildNgHeaderString({ [name]: 'abc' }), { code: 'ERR_HTTP2_INVALID_CONNECTION_HEADERS', name: 'TypeError', message: 'HTTP/1 Connection specific headers are forbidden: ' + @@ -312,7 +312,7 @@ const { }); }); -assert.throws(() => mapToHeaders({ [HTTP2_HEADER_TE]: ['abc'] }), { +assert.throws(() => buildNgHeaderString({ [HTTP2_HEADER_TE]: ['abc'] }), { code: 'ERR_HTTP2_INVALID_CONNECTION_HEADERS', name: 'TypeError', message: 'HTTP/1 Connection specific headers are forbidden: ' + @@ -320,7 +320,7 @@ assert.throws(() => mapToHeaders({ [HTTP2_HEADER_TE]: ['abc'] }), { }); assert.throws( - () => mapToHeaders({ [HTTP2_HEADER_TE]: ['abc', 'trailers'] }), { + () => buildNgHeaderString({ [HTTP2_HEADER_TE]: ['abc', 'trailers'] }), { code: 'ERR_HTTP2_INVALID_CONNECTION_HEADERS', name: 'TypeError', message: 'HTTP/1 Connection specific headers are forbidden: ' + @@ -328,13 +328,13 @@ assert.throws( }); // These should not throw -mapToHeaders({ te: 'trailers' }); -mapToHeaders({ te: ['trailers'] }); +buildNgHeaderString({ te: 'trailers' }); +buildNgHeaderString({ te: ['trailers'] }); // HTTP/2 encourages use of Host instead of :authority when converting // from HTTP/1 to HTTP/2, so we no longer disallow it. // Refs: https://github.com/nodejs/node/issues/29858 -mapToHeaders({ [HTTP2_HEADER_HOST]: 'abc' }); +buildNgHeaderString({ [HTTP2_HEADER_HOST]: 'abc' }); // If both are present, the latter has priority assert.strictEqual(getAuthority({