diff --git a/lib/core/errors.js b/lib/core/errors.js index b2b3f326bc4..45c04940678 100644 --- a/lib/core/errors.js +++ b/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 } diff --git a/lib/dispatcher/client-h2.js b/lib/dispatcher/client-h2.js index 661d857bee1..41904c6d365 100644 --- a/lib/dispatcher/client-h2.js +++ b/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 diff --git a/test/http2-error-handling-test.js b/test/http2-error-handling-test.js new file mode 100644 index 00000000000..01a036c18da --- /dev/null +++ b/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' + ) +}) diff --git a/test/http2-invalid-header-recovery.js b/test/http2-invalid-header-recovery.js new file mode 100644 index 00000000000..668cd3e69dd --- /dev/null +++ b/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') +}) diff --git a/test/http2-invalid-header-unit-test.js b/test/http2-invalid-header-unit-test.js new file mode 100644 index 00000000000..4c1e6205ff8 --- /dev/null +++ b/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') +})