Skip to content

Commit d03fb24

Browse files
authored
fix: handle paused parser on socket end (issue #5360) (#5389)
Remove assert(!this.paused) from Parser.finish() so that when the HTTP/1 parser is paused under backpressure (body not consumed) and the socket ends (FIN), the parser can finish gracefully instead of throwing an uncatchable AssertionError from the socket 'end' handler. llhttp_finish() already handles paused state by returning ERROR.PAUSED, which is handled in the existing code path by setting this.paused = true and returning null. Fixes #5360
1 parent ee59da3 commit d03fb24

2 files changed

Lines changed: 54 additions & 1 deletion

File tree

lib/dispatcher/client-h1.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,6 @@ class Parser {
371371
finish () {
372372
assert(currentParser === null)
373373
assert(this.ptr != null)
374-
assert(!this.paused)
375374

376375
const { llhttp } = this
377376

test/parser-issues.js

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,3 +309,57 @@ test('fetch rejects truncated chunked responses terminated by EOF', async (t) =>
309309
t.strictEqual(err.cause?.message, 'Response does not match the HTTP/1.1 protocol (Invalid EOF state)')
310310
}
311311
})
312+
313+
// https://github.com/nodejs/undici/issues/5360
314+
// When the HTTP/1 parser is paused under backpressure (body not consumed)
315+
// and the socket ends (FIN), parser.finish() must not assert(!this.paused).
316+
// That assert throws an uncatchable AssertionError from the socket 'end' handler.
317+
// The process should survive with a catchable error instead.
318+
test('no crash when parser paused and socket ends (issue 5360)', async (t) => {
319+
t = tspl(t, { plan: 2 })
320+
321+
const { spawnSync } = require('node:child_process')
322+
const { join } = require('node:path')
323+
324+
const code = `
325+
const { createServer } = require('node:net')
326+
const { fetch } = require('./index')
327+
328+
const BODY = Buffer.alloc(64 * 1024, 0x61)
329+
330+
const server = createServer((sock) => {
331+
sock.once('data', () => {
332+
sock.write(
333+
'HTTP/1.1 200 OK\\r\\n' +
334+
'Content-Length: ' + BODY.length + '\\r\\n' +
335+
'Connection: close\\r\\n\\r\\n'
336+
)
337+
sock.write(BODY)
338+
sock.end()
339+
})
340+
})
341+
342+
server.listen(0, '127.0.0.1', async () => {
343+
const { port } = server.address()
344+
const res = await fetch('http://127.0.0.1:' + port + '/')
345+
// Never read the body -> parser pauses under backpressure
346+
await new Promise((r) => setTimeout(r, 1000))
347+
console.log('no crash')
348+
server.close()
349+
})
350+
`
351+
352+
const res = spawnSync(process.execPath, ['-e', code], {
353+
cwd: join(__dirname, '..'),
354+
timeout: 5000,
355+
stdio: 'pipe'
356+
})
357+
358+
const stderr = res.stderr.toString()
359+
const stdout = res.stdout.toString()
360+
361+
// Must not contain AssertionError in stderr
362+
t.ok(!stderr.includes('AssertionError'), 'stderr must not contain AssertionError')
363+
// Must print 'no crash'
364+
t.equal(stdout.trim(), 'no crash', 'must print no crash')
365+
})

0 commit comments

Comments
 (0)