http2: reject non-latin1 client request values

doc/api/http2.md

@@ -1128,6 +1128,15 @@ 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, and the `:path` pseudo-header must not contain unescaped
+characters (this `:path` check is always enforced).
+
+This strict validation can be relaxed via the `httpValidation` option of
+[`http2.connect()`](#http2connectauthority-options-listener). The `'relaxed'`
+mode allows control characters permitted by the Fetch specification, and the
+`'insecure'` mode skips header value validation.
+
 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.
@@ -3303,6 +3312,9 @@ server.listen(8443);
 <!-- YAML
 added: v8.4.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/63571
+    description: Added the `httpValidation` option.
   - version: v26.11.0
     pr-url: https://github.com/nodejs/node/pull/65619
     description: Added the `connectionWindowSize` option.
@@ -3424,6 +3436,18 @@ changes:
     and trailing whitespace validation for HTTP/2 header field names and values
     as per [RFC-9113](https://www.rfc-editor.org/rfc/rfc9113.html#section-8.2.1).
     **Default:** `true`.
+  * `httpValidation` {string} Controls HTTP header value validation strictness
+    for outgoing HTTP/2 requests. Accepted values are:
+    * `'strict'`: Rejects non-`latin1` characters and disallowed control
+      characters in header values (default).
+    * `'relaxed'`: Allows control characters permitted by the
+      [Fetch specification][].
+    * `'insecure'`: Skips header value validation (equivalent to
+      `insecureHTTPParser` in HTTP/1).
+      When set to `'relaxed'` or `'insecure'`, `strictSingleValueFields` and
+      `strictFieldWhitespaceValidation` are automatically disabled, as they
+      would otherwise conflict with the relaxed behaviour.
+      **Default:** `'strict'`.
 * `listener` {Function} Will be registered as a one-time listener of the
   [`'connect'`][] event.
 * Returns: {ClientHttp2Session}
@@ -5271,6 +5295,7 @@ you need to implement any fall-back behavior yourself.
 [ALPN negotiation]: #alpn-negotiation
 [Compatibility API]: #compatibility-api
 [DEP0202]: deprecations.md#dep0202-http1incomingmessage-and-http1serverresponse-options-of-http2-servers
+[Fetch specification]: https://fetch.spec.whatwg.org/
 [HTTP/1]: http.md
 [HTTP/2]: https://tools.ietf.org/html/rfc7540
 [HTTP/2 Headers Object]: #headers-object

lib/internal/http2/core.js

@@ -119,6 +119,7 @@ const {
   isUint32,
   validateAbortSignal,
   validateBoolean,
+  validateOneOf,
   validateBuffer,
   validateFunction,
   validateInt32,
@@ -152,6 +153,7 @@ const {
   kAuthority,
   kAutoEmptyTrailers,
   kDisableAutoTrailers,
+  kHttpValidation,
   kSensitiveHeaders,
   kStrictSingleValueFields,
   kSocket,
@@ -1410,6 +1412,8 @@ class Http2Session extends EventEmitter {
     this[kHandle] = undefined;
     this[kStrictSingleValueFields] =
       options.strictSingleValueFields;
+    this[kHttpValidation] =
+      options.httpValidation;
 
     // Do not use nagle's algorithm
     if (typeof socket.setNoDelay === 'function')
@@ -3873,6 +3877,20 @@ function connect(authority, options, listener) {
 
   validateSessionOptions(options);
 
+  const httpValidation = options.httpValidation;
+  if (httpValidation !== undefined) {
+    validateOneOf(httpValidation, 'options.httpValidation',
+                  ['strict', 'relaxed', 'insecure']);
+    if (httpValidation !== 'strict') {
+      // In relaxed/insecure mode, the strict subset validation options
+      // (strictSingleValueFields, strictFieldWhitespaceValidation) are
+      // subsumed by httpValidation and disabled, as they would otherwise
+      // conflict with the relaxed behaviour.
+      options.strictSingleValueFields = false;
+      options.strictFieldWhitespaceValidation = false;
+    }
+  }
+
   if (typeof authority === 'string')
     authority = new URL(authority);
 

lib/internal/http2/util.js

@@ -15,6 +15,7 @@ const {
 } = primordials;
 
 const {
+  _checkInvalidHeaderChar: checkInvalidHeaderChar,
   _checkIsHttpToken: checkIsHttpToken,
 } = require('internal/http/common');
 
@@ -26,11 +27,13 @@ const {
     ERR_HTTP2_CONNECT_SCHEME,
     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,
@@ -42,6 +45,7 @@ const kAutoEmptyTrailers = Symbol('autoEmptyTrailers');
 const kDisableAutoTrailers = Symbol('disableAutoTrailers');
 const kSensitiveHeaders = Symbol('sensitiveHeaders');
 const kStrictSingleValueFields = Symbol('strictSingleValueFields');
+const kHttpValidation = Symbol('httpValidation');
 const kSocket = Symbol('socket');
 const kProtocol = Symbol('protocol');
 const kProxySocket = Symbol('proxySocket');
@@ -122,6 +126,23 @@ const kValidPseudoHeaders = new SafeSet([
   HTTP2_HEADER_PROTOCOL,
 ]);
 
+const INVALID_PATH_REGEX = /[^\u0021-\u00ff]/;
+
+function assertValidHeaderValue(name, value, httpValidation) {
+  if (name === ':path' && INVALID_PATH_REGEX.test(value)) {
+    throw new ERR_UNESCAPED_CHARACTERS('Request path');
+  }
+
+  if (httpValidation === 'insecure') {
+    return;
+  }
+
+  const lenient = httpValidation === 'relaxed';
+  if (checkInvalidHeaderChar(value, lenient)) {
+    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 kSingleValueFields = new SafeSet([
@@ -700,6 +721,7 @@ function prepareRequestHeadersArray(headers, session) {
     rawHeaders,
     assertValidPseudoHeader,
     session[kStrictSingleValueFields],
+    session[kHttpValidation] ?? 'strict',
   );
 
   return {
@@ -745,6 +767,7 @@ function prepareRequestHeadersObject(headers, session) {
     headersObject,
     assertValidPseudoHeader,
     session[kStrictSingleValueFields],
+    session[kHttpValidation] ?? 'strict',
   );
 
   return {
@@ -773,7 +796,9 @@ const kNoHeaderFlags = StringFromCharCode(NGHTTP2_NV_FLAG_NONE);
  */
 function buildNgHeaderString(arrayOrMap,
                              validatePseudoHeaderValue,
-                             strictSingleValueFields) {
+                             strictSingleValueFields,
+                             httpValidation) {
+  const validateHeaderValues = httpValidation !== undefined;
   let headers = '';
   let pseudoHeaders = '';
   let count = 0;
@@ -820,6 +845,8 @@ function buildNgHeaderString(arrayOrMap,
       const err = validatePseudoHeaderValue(key);
       if (err !== undefined)
         throw err;
+      if (validateHeaderValues)
+        assertValidHeaderValue(key, value, httpValidation);
       pseudoHeaders += `${key}\0${value}\0${flags}`;
       count++;
       return;
@@ -837,11 +864,15 @@ function buildNgHeaderString(arrayOrMap,
     if (isArray) {
       for (let j = 0; j < value.length; ++j) {
         const val = String(value[j]);
+        if (validateHeaderValues)
+          assertValidHeaderValue(key, val, httpValidation);
         headers += `${key}\0${val}\0${flags}`;
       }
       count += value.length;
       return;
     }
+    if (validateHeaderValues)
+      assertValidHeaderValue(key, value, httpValidation);
     headers += `${key}\0${value}\0${flags}`;
     count++;
   }
@@ -1002,6 +1033,7 @@ module.exports = {
   kAutoEmptyTrailers,
   kDisableAutoTrailers,
   kSensitiveHeaders,
+  kHttpValidation,
   kStrictSingleValueFields,
   kSocket,
   kProtocol,

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

test/parallel/test-http2-httpvalidation.js

@@ -0,0 +1,105 @@
+'use strict';
+const common = require('../common');
+if (!common.hasCrypto) { common.skip('missing crypto'); }
+const assert = require('assert');
+const http2 = require('http2');
+
+// Integration tests for the http2.connect() `httpValidation` option.
+// See also test-http2-util-headers-list.js for the per-value validation logic.
+
+const server = http2.createServer();
+// Consume any stream that is actually created. Control characters and
+// duplicate pseudo-headers may be rejected by nghttp2 asynchronously, so
+// swallow any stream errors rather than letting them become uncaught.
+server.on('stream', (stream) => {
+  stream.on('error', () => {});
+});
+
+server.listen(0, common.mustCall(() => {
+  const origin = `http://localhost:${server.address().port}`;
+
+  function swallow(stream) {
+    stream.on('error', () => {});
+    stream.destroy();
+    return stream;
+  }
+
+  // -- strict (default) --
+  {
+    const session = http2.connect(origin);
+    session.on('error', () => {});
+    // Non-latin1 characters are rejected by default.
+    assert.throws(() => session.request({ 'x-bad-char': 'oʊmɪɡə' }), {
+      code: 'ERR_HTTP2_INVALID_HEADER_VALUE',
+    });
+    // Control characters are rejected by default per the HTTP RFCs.
+    assert.throws(() => session.request({ 'x-control': 'value\x01here' }), {
+      code: 'ERR_HTTP2_INVALID_HEADER_VALUE',
+    });
+    // strictSingleValueFields is enabled by default.
+    assert.throws(() => session.request([
+      [':method', 'GET'],
+      [':method', 'POST'],
+    ]), {
+      code: 'ERR_HTTP2_HEADER_SINGLE_VALUE',
+    });
+    // The :path pseudo-header must not contain unescaped characters.
+    assert.throws(() => session.request({ ':path': 'bad\u0100path' }), {
+      code: 'ERR_UNESCAPED_CHARACTERS',
+    });
+    session.close();
+  }
+
+  // -- relaxed --
+  {
+    const session = http2.connect(origin, { httpValidation: 'relaxed' });
+    session.on('error', () => {});
+    // Control characters permitted by the Fetch specification are allowed.
+    swallow(session.request({ 'x-control': 'value\x01here' }));
+    // relaxed/insecure mode subsumes and disables strictSingleValueFields.
+    swallow(session.request([
+      [':method', 'GET'],
+      [':method', 'POST'],
+    ]));
+    // Non-latin1 characters are still rejected in relaxed mode.
+    assert.throws(() => session.request({ 'x-bad-char': 'oʊmɪɡə' }), {
+      code: 'ERR_HTTP2_INVALID_HEADER_VALUE',
+    });
+    // NUL is always rejected.
+    assert.throws(() => session.request({ 'x-nul': 'value\x00here' }), {
+      code: 'ERR_HTTP2_INVALID_HEADER_VALUE',
+    });
+    // The :path pseudo-header check is always enforced.
+    assert.throws(() => session.request({ ':path': 'bad\u0100path' }), {
+      code: 'ERR_UNESCAPED_CHARACTERS',
+    });
+    session.close();
+  }
+
+  // -- insecure --
+  {
+    const session = http2.connect(origin, { httpValidation: 'insecure' });
+    session.on('error', () => {});
+    // All header value validation is skipped, including newlines.
+    swallow(session.request({ 'x-newline': 'value\nhere' }));
+    // strictSingleValueFields is disabled in insecure mode too.
+    swallow(session.request([
+      [':method', 'GET'],
+      [':method', 'POST'],
+    ]));
+    // The :path pseudo-header check is still enforced.
+    assert.throws(() => session.request({ ':path': 'bad\u0100path' }), {
+      code: 'ERR_UNESCAPED_CHARACTERS',
+    });
+    session.close();
+  }
+
+  // -- invalid option value --
+  {
+    assert.throws(() => http2.connect(origin, { httpValidation: 'bogus' }), {
+      code: 'ERR_INVALID_ARG_VALUE',
+    });
+  }
+
+  server.close();
+}));

test/parallel/test-http2-invalidheaderfields-client.js

@@ -14,6 +14,11 @@ 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.close();
   server1.close();
 }));