diff --git a/lib/dispatcher/client-h2.js b/lib/dispatcher/client-h2.js index 68205556f37..8d36d613a2f 100644 --- a/lib/dispatcher/client-h2.js +++ b/lib/dispatcher/client-h2.js @@ -528,7 +528,17 @@ function writeH2 (client, request) { headers[HTTP2_HEADER_SCHEME] = protocol === 'http:' ? 'http' : 'https' } - stream = session.request(headers, { endStream: false, signal }) + try { + stream = session.request(headers, { endStream: false, signal }) + } catch (err) { + // 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) + return false + } + stream[kHTTP2Stream] = true stream.once('response', (headers, _flags) => { @@ -563,7 +573,16 @@ 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) { + // 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) + return false + } stream[kHTTP2Stream] = true stream.on('response', headers => { const { [HTTP2_HEADER_STATUS]: statusCode, ...realHeaders } = headers @@ -661,15 +680,33 @@ 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 }) + try { + stream = session.request(headers, { endStream: shouldEndStream, signal }) + } catch (err) { + // 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) + return false + } stream[kHTTP2Stream] = true stream.once('continue', writeBodyH2) } else { - stream = session.request(headers, { - endStream: shouldEndStream, - signal - }) + try { + stream = session.request(headers, { + endStream: shouldEndStream, + signal + }) + } catch (err) { + // 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) + return false + } stream[kHTTP2Stream] = true writeBodyH2() diff --git a/test/http2-invalid-headers.js b/test/http2-invalid-headers.js new file mode 100644 index 00000000000..780f90aa005 --- /dev/null +++ b/test/http2-invalid-headers.js @@ -0,0 +1,115 @@ +'use strict' + +const { tspl } = require('@matteo.collina/tspl') +const { test, after } = require('node:test') +const { createSecureServer } = require('node:http2') +const { once } = require('node:events') + +const pem = require('@metcoder95/https-pem') + +const { Client } = require('..') + +test('HTTP/2 invalid headers should be recoverable (#4356)', async t => { + t = tspl(t, { plan: 4 }) + + 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 ${requestCount}`) + }) + + 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()) + + // First request should succeed + const response1 = await client.request({ + path: '/', + method: 'GET' + }) + const body1 = await response1.body.text() + t.strictEqual(body1, 'response 1') + + // Second request should also work (client should recover) + const response2 = await client.request({ + path: '/', + method: 'GET' + }) + const body2 = await response2.body.text() + t.strictEqual(body2, 'response 2') + + // Verify the client is still functional (not crashed) + 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 + const response = await client.request({ + path: '/', + method: 'GET' + }) + await response.body.text() + t.strictEqual(requestCount, 1) // Only the GET request succeeded +})