From db6b9a42598b068a1e59692a6fe8a4f46d2c3e82 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Mon, 25 May 2026 23:14:44 +0200 Subject: [PATCH] http2: reject non-latin1 client request values --- doc/api/http2.md | 4 +++ lib/internal/http2/util.js | 30 ++++++++++++++++ .../test-http2-client-unescaped-path.js | 34 +++++++++---------- .../test-http2-invalidheaderfields-client.js | 6 ++++ test/parallel/test-http2-util-headers-list.js | 20 +++++++++++ 5 files changed, 76 insertions(+), 18 deletions(-) diff --git a/doc/api/http2.md b/doc/api/http2.md index d93ffbc5285ce8..8eefdd9da611b5 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -959,6 +959,10 @@ For HTTP/2 Client `Http2Session` instances only, the `http2session.request()` creates and returns an `Http2Stream` instance that can be used to send an HTTP/2 request to the connected server. +When sending a request, header values must not contain characters outside the +`latin1` encoding. The `:path` pseudo-header must not contain unescaped +characters. + When a `ClientHttp2Session` is first created, the socket may not yet be connected. if `clienthttp2session.request()` is called during this time, the actual request will be deferred until the socket is ready to go. diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index 4b0cc941a5e078..5752e0cba4bcc2 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -18,16 +18,23 @@ const { Symbol, } = primordials; +const { + _checkInvalidHeaderChar: checkInvalidHeaderChar, + _checkIsHttpToken: checkIsHttpToken, +} = require('_http_common'); + const binding = internalBinding('http2'); const { codes: { ERR_HTTP2_HEADER_SINGLE_VALUE, ERR_HTTP2_INVALID_CONNECTION_HEADERS, + ERR_HTTP2_INVALID_HEADER_VALUE, ERR_HTTP2_INVALID_PSEUDOHEADER: { HideStackFramesError: ERR_HTTP2_INVALID_PSEUDOHEADER }, ERR_HTTP2_INVALID_SETTING_VALUE, ERR_HTTP2_TOO_MANY_CUSTOM_SETTINGS, ERR_INVALID_ARG_TYPE, ERR_INVALID_HTTP_TOKEN, + ERR_UNESCAPED_CHARACTERS, }, getMessage, hideStackFrames, @@ -113,6 +120,18 @@ const kValidPseudoHeaders = new SafeSet([ HTTP2_HEADER_PROTOCOL, ]); +const INVALID_PATH_REGEX = /[^\u0021-\u00ff]/; + +function assertValidHeaderValue(name, value) { + if (name === HTTP2_HEADER_PATH && INVALID_PATH_REGEX.test(value)) { + throw new ERR_UNESCAPED_CHARACTERS('Request path'); + } + + if (checkInvalidHeaderChar(value)) { + throw new ERR_HTTP2_INVALID_HEADER_VALUE(value, name); + } +} + // This set contains headers that are permitted to have only a single // value. Multiple instances must not be specified. const kSingleValueHeaders = new SafeSet([ @@ -595,6 +614,8 @@ function mapToHeaders(map, let pseudoHeaders = ''; let count = 0; const keys = ObjectKeys(map); + const shouldValidateHeaderValue = + assertValuePseudoHeader === assertValidPseudoHeader; const singles = new SafeSet(); let i, j; let isArray; @@ -640,6 +661,9 @@ function mapToHeaders(map, err = assertValuePseudoHeader(key); if (err !== undefined) throw err; + if (shouldValidateHeaderValue) { + assertValidHeaderValue(key, value); + } pseudoHeaders += `${key}\0${value}\0${flags}`; count++; continue; @@ -653,11 +677,17 @@ function mapToHeaders(map, if (isArray) { for (j = 0; j < value.length; ++j) { const val = String(value[j]); + if (shouldValidateHeaderValue) { + assertValidHeaderValue(key, val); + } headers += `${key}\0${val}\0${flags}`; } count += value.length; continue; } + if (shouldValidateHeaderValue) { + assertValidHeaderValue(key, value); + } headers += `${key}\0${value}\0${flags}`; count++; } diff --git a/test/parallel/test-http2-client-unescaped-path.js b/test/parallel/test-http2-client-unescaped-path.js index ca061ccda484b6..a8c2defc026457 100644 --- a/test/parallel/test-http2-client-unescaped-path.js +++ b/test/parallel/test-http2-client-unescaped-path.js @@ -3,8 +3,8 @@ const common = require('../common'); if (!common.hasCrypto) common.skip('missing crypto'); +const assert = require('assert'); const http2 = require('http2'); -const Countdown = require('../common/countdown'); const server = http2.createServer(); @@ -14,24 +14,22 @@ const count = 32; server.listen(0, common.mustCall(() => { const client = http2.connect(`http://localhost:${server.address().port}`); - client.setMaxListeners(33); - const countdown = new Countdown(count + 1, () => { - server.close(); - client.close(); - }); - - // nghttp2 will catch the bad header value for us. - function doTest(i) { - const req = client.request({ ':path': `bad${String.fromCharCode(i)}path` }); - req.on('error', common.expectsError({ - code: 'ERR_HTTP2_STREAM_ERROR', - name: 'Error', - message: 'Stream closed with error code NGHTTP2_PROTOCOL_ERROR' - })); - req.on('close', common.mustCall(() => countdown.dec())); + for (let i = 0; i <= count; i += 1) { + const path = `bad${String.fromCharCode(i)}path`; + assert.throws(() => client.request({ ':path': path }), { + code: 'ERR_UNESCAPED_CHARACTERS', + name: 'TypeError', + message: 'Request path contains unescaped characters' + }); } - for (let i = 0; i <= count; i += 1) - doTest(i); + assert.throws(() => client.request({ ':path': 'bad\u0100path' }), { + code: 'ERR_UNESCAPED_CHARACTERS', + name: 'TypeError', + message: 'Request path contains unescaped characters' + }); + + client.close(); + server.close(); })); diff --git a/test/parallel/test-http2-invalidheaderfields-client.js b/test/parallel/test-http2-invalidheaderfields-client.js index a5681970faab27..7d1e261d5d1d22 100644 --- a/test/parallel/test-http2-invalidheaderfields-client.js +++ b/test/parallel/test-http2-invalidheaderfields-client.js @@ -14,8 +14,14 @@ server1.listen(0, common.mustCall(() => { }, { code: 'ERR_INVALID_HTTP_TOKEN' }); + assert.throws(() => { + session.request({ 'x-bad-char': 'oʊmɪɡə' }); + }, { + code: 'ERR_HTTP2_INVALID_HEADER_VALUE' + }); session.on('error', common.mustCall((e) => { assert.strictEqual(e.code, 'ERR_INVALID_HTTP_TOKEN'); + session.close(); server1.close(); })); })); diff --git a/test/parallel/test-http2-util-headers-list.js b/test/parallel/test-http2-util-headers-list.js index f4221f5c17e589..311d8f1dbf74cf 100644 --- a/test/parallel/test-http2-util-headers-list.js +++ b/test/parallel/test-http2-util-headers-list.js @@ -336,6 +336,26 @@ mapToHeaders({ te: ['trailers'] }); // Refs: https://github.com/nodejs/node/issues/29858 mapToHeaders({ [HTTP2_HEADER_HOST]: 'abc' }); +assert.throws(() => buildNgHeaderString( + { ':path': 'bad\u0100path' }, + assertValidPseudoHeader, + true +), { + code: 'ERR_UNESCAPED_CHARACTERS', + name: 'TypeError', + message: 'Request path contains unescaped characters' +}); + +assert.throws(() => buildNgHeaderString( + { 'x-bad-char': 'oʊmɪɡə' }, + assertValidPseudoHeader, + true +), { + code: 'ERR_HTTP2_INVALID_HEADER_VALUE', + name: 'TypeError', + message: 'Invalid value "oʊmɪɡə" for header "x-bad-char"' +}); + // If both are present, the latter has priority assert.strictEqual(getAuthority({ [HTTP2_HEADER_AUTHORITY]: 'abc',