fix: make HTTP2 invalid headers recoverable (#4356)

lib/core/errors.js

@@ -217,6 +217,16 @@ class SecureProxyConnectionError extends UndiciError {
   }
 }
 
+class H2InvalidConnectionHeadersError extends UndiciError {
+  constructor (cause, message, options = {}) {
+    super(message, { cause, ...options })
+    this.name = 'H2InvalidConnectionHeadersError'
+    this.message = message || 'Invalid HTTP/2 connection-specific headers'
+    this.code = 'UND_ERR_H2_INVALID_CONNECTION_HEADERS'
+    this.cause = cause
+  }
+}
+
 module.exports = {
   AbortError,
   HTTPParserError,
@@ -240,5 +250,6 @@ module.exports = {
   ResponseExceededMaxSizeError,
   RequestRetryError,
   ResponseError,
-  SecureProxyConnectionError
+  SecureProxyConnectionError,
+  H2InvalidConnectionHeadersError
 }

lib/dispatcher/client-h2.js

@@ -139,14 +139,15 @@ async function connectH2 (client, socket) {
 
 function resumeH2 (client) {
   const socket = client[kSocket]
+  const session = client[kHTTP2Session]
 
   if (socket?.destroyed === false) {
     if (client[kSize] === 0 || client[kMaxConcurrentStreams] === 0) {
       socket.unref()
-      client[kHTTP2Session].unref()
+      if (session) session.unref()
     } else {
       socket.ref()
-      client[kHTTP2Session].ref()
+      if (session) session.ref()
     }
   }
 }
@@ -360,7 +361,22 @@ function writeH2 (client, request) {
     // will create a new stream. We trigger a request to create the stream and wait until
     // `ready` event is triggered
     // We disabled endStream to allow the user to write to the stream
-    stream = session.request(headers, { endStream: false, signal })
+    try {
+      stream = session.request(headers, { endStream: false, signal })
+    } catch (err) {
+      if (err && err.code === 'ERR_HTTP2_INVALID_CONNECTION_HEADERS') {
+        const { H2InvalidConnectionHeadersError } = require('../core/errors.js')
+        const wrapped = new H2InvalidConnectionHeadersError(err)
+        if (client[kHTTP2Session]) {
+          client[kHTTP2Session].destroy()
+          client[kHTTP2Session] = null
+        }
+        util.errorRequest(client, request, wrapped)
+        client[kResume]()
+        return false
+      }
+      throw err
+    }
 
     if (!stream.pending) {
       request.onUpgrade(null, null, stream)
@@ -464,16 +480,44 @@ function writeH2 (client, request) {
   const shouldEndStream = method === 'GET' || method === 'HEAD' || body === null
   if (expectContinue) {
     headers[HTTP2_HEADER_EXPECT] = '100-continue'
-    stream = session.request(headers, { endStream: shouldEndStream, signal })
-
-    stream.once('continue', writeBodyH2)
+    try {
+      stream = session.request(headers, { endStream: shouldEndStream, signal })
+      stream.once('continue', writeBodyH2)
+    } catch (err) {
+      if (err && err.code === 'ERR_HTTP2_INVALID_CONNECTION_HEADERS') {
+        const { H2InvalidConnectionHeadersError } = require('../core/errors.js')
+        const wrapped = new H2InvalidConnectionHeadersError(err)
+        if (client[kHTTP2Session]) {
+          client[kHTTP2Session].destroy()
+          client[kHTTP2Session] = null
+        }
+        util.errorRequest(client, request, wrapped)
+        client[kResume]()
+        return false
+      }
+      throw err
+    }
   } else {
-    stream = session.request(headers, {
-      endStream: shouldEndStream,
-      signal
-    })
-
-    writeBodyH2()
+    try {
+      stream = session.request(headers, {
+        endStream: shouldEndStream,
+        signal
+      })
+      writeBodyH2()
+    } catch (err) {
+      if (err && err.code === 'ERR_HTTP2_INVALID_CONNECTION_HEADERS') {
+        const { H2InvalidConnectionHeadersError } = require('../core/errors.js')
+        const wrapped = new H2InvalidConnectionHeadersError(err)
+        if (client[kHTTP2Session]) {
+          client[kHTTP2Session].destroy()
+          client[kHTTP2Session] = null
+        }
+        util.errorRequest(client, request, wrapped)
+        client[kResume]()
+        return false
+      }
+      throw err
+    }
   }
 
   // Increment counter as we have new streams open

test/http2-error-handling-test.js

@@ -0,0 +1,43 @@
+const { test } = require('node:test')
+const assert = require('node:assert')
+
+test('ERR_HTTP2_INVALID_CONNECTION_HEADERS should be catchable', async (t) => {
+  const error = new TypeError('HTTP/1 Connection specific headers are forbidden: "http2-settings"')
+  error.code = 'ERR_HTTP2_INVALID_CONNECTION_HEADERS'
+
+  let errorCaught = false
+  let errorCode = null
+
+  try {
+    throw error
+  } catch (err) {
+    errorCaught = true
+    errorCode = err.code
+  }
+
+  assert.ok(errorCaught, 'Error should be catchable')
+  assert.strictEqual(errorCode, 'ERR_HTTP2_INVALID_CONNECTION_HEADERS', 'Error code should match')
+})
+
+test('writeH2 function has try-catch protection', async (t) => {
+  const fs = require('node:fs')
+  const path = require('node:path')
+
+  const clientH2Path = path.join(__dirname, '../lib/dispatcher/client-h2.js')
+  const clientH2Content = fs.readFileSync(clientH2Path, 'utf8')
+
+  assert.ok(
+    clientH2Content.includes('ERR_HTTP2_INVALID_CONNECTION_HEADERS'),
+    'client-h2.js should handle ERR_HTTP2_INVALID_CONNECTION_HEADERS'
+  )
+
+  assert.ok(
+    clientH2Content.includes('H2InvalidConnectionHeadersError'),
+    'client-h2.js should wrap invalid h2 header errors in an Undici error'
+  )
+
+  assert.ok(
+    clientH2Content.includes('session.request(headers'),
+    'client-h2.js should contain session.request calls'
+  )
+})

test/http2-invalid-header-recovery.js

@@ -0,0 +1,70 @@
+// Test: HTTP2 invalid header handling (fail-fast)
+// This test spins up an HTTP/2 TLS server and patches the client's HTTP/2 session
+// to throw ERR_HTTP2_INVALID_CONNECTION_HEADERS from session.request(). Undici
+// should fail-fast and wrap the error as an Undici error without internal retry.
+
+const { test } = require('node:test')
+const assert = require('node:assert')
+const http2 = require('node:http2')
+const { Client } = require('..')
+const pem = require('https-pem')
+
+function createServer (cb) {
+  const server = http2.createSecureServer(pem)
+  server.on('stream', (stream) => {
+    // Normal valid response; client error should occur before this if invalid headers are sent
+    stream.respond({ ':status': 200 })
+    stream.end('ok')
+  })
+  server.listen(0, cb)
+  return server
+}
+
+test('undici should fail-fast and wrap invalid HTTP/2 connection header errors', async (t) => {
+  const server = createServer(() => {})
+
+  const address = server.address()
+  const port = typeof address === 'string' ? 0 : address.port
+
+  // Monkey-patch http2.connect to throw on request creation
+  const originalConnect = http2.connect
+  let thrown = false
+  http2.connect = function (...args) {
+    const session = originalConnect.apply(this, args)
+    const originalRequest = session.request
+    session.request = function (...rargs) {
+      if (!thrown) {
+        thrown = true
+        const e = new TypeError('HTTP/1 Connection specific headers are forbidden: "http2-settings"')
+        e.code = 'ERR_HTTP2_INVALID_CONNECTION_HEADERS'
+        throw e
+      }
+      return originalRequest.apply(this, rargs)
+    }
+    return session
+  }
+
+  const client = new Client(`https://localhost:${port}`, {
+    connect: {
+      rejectUnauthorized: false
+    },
+    allowH2: true
+  })
+  let errorCaught = null
+
+  try {
+    await client.request({
+      path: '/',
+      method: 'GET'
+    })
+  } catch (err) {
+    errorCaught = err
+  } finally {
+    await client.close()
+    await new Promise((resolve) => server.close(resolve))
+    http2.connect = originalConnect
+  }
+
+  assert.ok(errorCaught, 'Request should surface an error')
+  assert.strictEqual(errorCaught.code, 'UND_ERR_H2_INVALID_CONNECTION_HEADERS', 'Error code should indicate invalid HTTP/2 connection headers')
+})

test/http2-invalid-header-unit-test.js

@@ -0,0 +1,64 @@
+const { test } = require('node:test')
+const assert = require('node:assert')
+const { Client } = require('..')
+
+test('undici should fail-fast on ERR_HTTP2_INVALID_CONNECTION_HEADERS', async (t) => {
+  let retryCount = 0
+  const http2 = require('node:http2')
+  const pem = require('https-pem')
+  const server = http2.createSecureServer(pem)
+  server.on('stream', (stream) => {
+    stream.respond({ ':status': 200 })
+    stream.end('success')
+  })
+  await new Promise((resolve) => server.listen(0, resolve))
+  const port = server.address().port
+
+  const originalConnect = http2.connect
+  let patchedOnce = false
+  http2.connect = function (...args) {
+    const session = originalConnect.apply(this, args)
+    const originalRequest = session.request
+    session.request = function (...reqArgs) {
+      retryCount++
+      if (!patchedOnce) {
+        patchedOnce = true
+        const error = new TypeError('HTTP/1 Connection specific headers are forbidden: "http2-settings"')
+        error.code = 'ERR_HTTP2_INVALID_CONNECTION_HEADERS'
+        throw error
+      }
+      return originalRequest.apply(this, reqArgs)
+    }
+    // Do not wrap destroy
+    return session
+  }
+
+  const client = new Client(`https://localhost:${port}`, {
+    allowH2: true,
+    connect: { rejectUnauthorized: false }
+  })
+
+  let errorCaught = null
+  let responseReceived = false
+
+  try {
+    await client.request({
+      path: '/',
+      method: 'GET'
+    })
+
+    responseReceived = true
+  } catch (err) {
+    errorCaught = err
+  } finally {
+    await new Promise((resolve) => setImmediate(resolve))
+    await client.close()
+    await new Promise((resolve) => server.close(resolve))
+    http2.connect = originalConnect
+  }
+
+  assert.strictEqual(retryCount, 1, 'Should attempt exactly once (no internal retry)')
+  assert.ok(!responseReceived, 'No response should be received on fail-fast')
+  assert.ok(errorCaught, 'Error should be surfaced to the caller')
+  assert.strictEqual(errorCaught.code, 'UND_ERR_H2_INVALID_CONNECTION_HEADERS', 'Error should be wrapped as Undici error')
+})