From 82667dba105a3d6c5cba966a7f9653a342ca5c89 Mon Sep 17 00:00:00 2001 From: Minh Le Date: Wed, 10 Jun 2026 09:46:13 +0700 Subject: [PATCH] fix(h2): do not rewind kPendingIdx past in-flight requests When two requests are multiplexed on one HTTP/2 session and one stream is closed by the server before response headers, finalizeRequest(true) unconditionally reset kPendingIdx back to kRunningIdx while the other request was still in flight. Once the surviving request finalized, kRunningIdx advanced past kPendingIdx, leaving a nulled queue slot that Client[kDestroy] later splices and calls errorRequest on, crashing the process with 'TypeError: Cannot read properties of null (reading 'onResponseError')'. Only reset kPendingIdx when it has actually fallen behind kRunningIdx, which is the invariant the reset was added to restore in #5090. Fixes: https://github.com/nodejs/undici/issues/5404 Signed-off-by: Minh Le --- lib/dispatcher/client-h2.js | 2 +- test/http2-destroy-after-failed-stream.js | 40 +++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 test/http2-destroy-after-failed-stream.js diff --git a/lib/dispatcher/client-h2.js b/lib/dispatcher/client-h2.js index e980c12d948..18082a8b80b 100644 --- a/lib/dispatcher/client-h2.js +++ b/lib/dispatcher/client-h2.js @@ -744,7 +744,7 @@ function writeH2 (client, request) { requestFinalized = true client[kQueue][client[kRunningIdx]++] = null - if (resetPendingIdx) { + if (resetPendingIdx && client[kPendingIdx] < client[kRunningIdx]) { client[kPendingIdx] = client[kRunningIdx] } diff --git a/test/http2-destroy-after-failed-stream.js b/test/http2-destroy-after-failed-stream.js new file mode 100644 index 00000000000..aaf8290eeea --- /dev/null +++ b/test/http2-destroy-after-failed-stream.js @@ -0,0 +1,40 @@ +'use strict' + +const { test } = require('node:test') +const assert = require('node:assert') +const { createServer } = require('node:http2') +const { once } = require('node:events') +const { setTimeout: sleep } = require('node:timers/promises') +const { Agent } = require('..') + +test('closing dispatcher after one multiplexed stream failed pre-response does not crash', async (t) => { + const server = createServer() + server.on('stream', async (stream, headers) => { + if (headers[':path'] === '/slow') { + await sleep(50) + stream.respond({ ':status': 200 }) + stream.end('slow') + } else { + stream.close() + } + }) + server.listen(0) + await once(server, 'listening') + t.after(() => server.close()) + + const origin = `http://localhost:${server.address().port}` + const dispatcher = new Agent({ connections: 1, useH2c: true }) + + const slow = dispatcher + .request({ origin, path: '/slow', method: 'GET' }) + .then((res) => res.body.text()) + + await assert.rejects( + dispatcher.request({ origin, path: '/bad', method: 'GET' }), + { name: 'InformationalError' } + ) + + assert.strictEqual(await slow, 'slow') + + await dispatcher.close() +})