Skip to content

Commit 27586ee

Browse files
authored
fix(h2): do not rewind kPendingIdx past in-flight requests (#5408)
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: #5404 Signed-off-by: Minh Le <ducminhldm@gmail.com>
1 parent ac5394b commit 27586ee

2 files changed

Lines changed: 41 additions & 1 deletion

File tree

‎lib/dispatcher/client-h2.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -744,7 +744,7 @@ function writeH2 (client, request) {
744744
requestFinalized = true
745745
client[kQueue][client[kRunningIdx]++] = null
746746

747-
if (resetPendingIdx) {
747+
if (resetPendingIdx && client[kPendingIdx] < client[kRunningIdx]) {
748748
client[kPendingIdx] = client[kRunningIdx]
749749
}
750750

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
'use strict'
2+
3+
const { test } = require('node:test')
4+
const assert = require('node:assert')
5+
const { createServer } = require('node:http2')
6+
const { once } = require('node:events')
7+
const { setTimeout: sleep } = require('node:timers/promises')
8+
const { Agent } = require('..')
9+
10+
test('closing dispatcher after one multiplexed stream failed pre-response does not crash', async (t) => {
11+
const server = createServer()
12+
server.on('stream', async (stream, headers) => {
13+
if (headers[':path'] === '/slow') {
14+
await sleep(50)
15+
stream.respond({ ':status': 200 })
16+
stream.end('slow')
17+
} else {
18+
stream.close()
19+
}
20+
})
21+
server.listen(0)
22+
await once(server, 'listening')
23+
t.after(() => server.close())
24+
25+
const origin = `http://localhost:${server.address().port}`
26+
const dispatcher = new Agent({ connections: 1, useH2c: true })
27+
28+
const slow = dispatcher
29+
.request({ origin, path: '/slow', method: 'GET' })
30+
.then((res) => res.body.text())
31+
32+
await assert.rejects(
33+
dispatcher.request({ origin, path: '/bad', method: 'GET' }),
34+
{ name: 'InformationalError' }
35+
)
36+
37+
assert.strictEqual(await slow, 'slow')
38+
39+
await dispatcher.close()
40+
})

0 commit comments

Comments
 (0)