fix: handle invalid HTTP/2 headers gracefully (#4356)

When session.request() throws synchronously due to invalid HTTP/2 headers
(e.g., duplicate headers like 'content-type' and 'Content-Type'), the error
was not being caught, causing an uncaught exception.

The fix wraps all session.request() calls with try-catch blocks. When an
error is caught, only the request is errored (via util.errorRequest) and
the function returns false. The session is not destroyed because the error
is caused by the client sending invalid headers, not the server.

This allows the client to continue using the same session for subsequent
requests, and the error is properly caught by user code.

Closes #4356

Author: mcollina

lib/dispatcher/client-h2.js

@@ -531,14 +531,11 @@ function writeH2 (client, request) {
       try {
         stream = session.request(headers, { endStream: false, signal })
       } catch (err) {
-        // An error here means the server sent invalid HTTP/2 headers
-        // (e.g., HTTP/1 headers like "http2-settings"). This is a server error.
-        // We destroy the socket/session and error the request so the client
-        // can retry with a new connection.
+        // An error here means the client sent invalid HTTP/2 headers
+        // (e.g., duplicate headers like "content-type" and "Content-Type").
+        // This is a client-side error, not a server-side error.
+        // We only error the request and do not destroy the session.
         util.errorRequest(client, request, err)
-        const socket = session[kSocket]
-        session.destroy(err)
-        util.destroy(socket, err)
         return false
       }
 
@@ -579,11 +576,11 @@ function writeH2 (client, request) {
     try {
       stream = session.request(headers, { endStream: false, signal })
     } catch (err) {
-      // An error here means the server sent invalid HTTP/2 headers
+      // An error here means the client sent invalid HTTP/2 headers
+      // (e.g., duplicate headers like "content-type" and "Content-Type").
+      // This is a client-side error, not a server-side error.
+      // We only error the request and do not destroy the session.
       util.errorRequest(client, request, err)
-      const socket = session[kSocket]
-      session.destroy(err)
-      util.destroy(socket, err)
       return false
     }
     stream[kHTTP2Stream] = true
@@ -686,11 +683,11 @@ function writeH2 (client, request) {
     try {
       stream = session.request(headers, { endStream: shouldEndStream, signal })
     } catch (err) {
-      // An error here means the server sent invalid HTTP/2 headers
+      // An error here means the client sent invalid HTTP/2 headers
+      // (e.g., duplicate headers like "content-type" and "Content-Type").
+      // This is a client-side error, not a server-side error.
+      // We only error the request and do not destroy the session.
       util.errorRequest(client, request, err)
-      const socket = session[kSocket]
-      session.destroy(err)
-      util.destroy(socket, err)
       return false
     }
     stream[kHTTP2Stream] = true
@@ -703,11 +700,11 @@ function writeH2 (client, request) {
         signal
       })
     } catch (err) {
-      // An error here means the server sent invalid HTTP/2 headers
+      // An error here means the client sent invalid HTTP/2 headers
+      // (e.g., duplicate headers like "content-type" and "Content-Type").
+      // This is a client-side error, not a server-side error.
+      // We only error the request and do not destroy the session.
       util.errorRequest(client, request, err)
-      const socket = session[kSocket]
-      session.destroy(err)
-      util.destroy(socket, err)
       return false
     }
     stream[kHTTP2Stream] = true

test/http2-invalid-headers.js

@@ -55,3 +55,62 @@ test('HTTP/2 invalid headers should be recoverable (#4356)', async t => {
   t.ok(client instanceof Client, 'client should still be a valid Client instance')
   t.ok(client.destroyed !== undefined, 'client state should be consistent')
 })
+
+test('HTTP/2 duplicate headers should be catchable (#4356)', async t => {
+  t = tspl(t, { plan: 3 })
+
+  const server = createSecureServer(await pem.generate({ opts: { keySize: 2048 } }))
+
+  let requestCount = 0
+  server.on('stream', (stream, headers) => {
+    requestCount++
+    stream.respond({
+      ':status': 200,
+      'content-type': 'text/plain'
+    })
+    stream.end('response')
+  })
+
+  after(() => server.close())
+  await once(server.listen(0), 'listening')
+
+  const client = new Client(`https://localhost:${server.address().port}`, {
+    connect: {
+      rejectUnauthorized: false
+    },
+    allowH2: true
+  })
+  after(() => client.close())
+
+  // Request with duplicate headers (content-type and Content-Type)
+  // should throw a catchable error, not an uncaughtException
+  try {
+    await client.request({
+      path: '/',
+      method: 'POST',
+      headers: {
+        'content-type': 'foo/bar',
+        'Content-Type': 'foo/bar'
+      },
+      body: 'foo-bar'
+    })
+    t.fail('should have thrown')
+  } catch (err) {
+    t.ok(err.code === 'ERR_HTTP2_INVALID_CONNECTION_HEADERS' ||
+         err.code === 'ERR_HTTP2_HEADER_SINGLE_VALUE' ||
+         err.code === 'UND_ERR_INFO',
+         `expected ERR_HTTP2_INVALID_CONNECTION_HEADERS/ERR_HTTP2_HEADER_SINGLE_VALUE or UND_ERR_INFO, got ${err.code}`)
+  }
+
+  // Verify the client is still functional (not crashed)
+  t.ok(client instanceof Client, 'client should still be a valid Client instance')
+
+  // After the error, the client should be able to make another request
+  // (on a new connection if the session was destroyed)
+  const response = await client.request({
+    path: '/',
+    method: 'GET'
+  })
+  await response.body.text()
+  t.strictEqual(requestCount, 2) // Two successful requests
+})