fix: finish paused HTTP/1 parser on socket end by marko1olo · Pull Request #5364 · nodejs/undici

marko1olo · 2026-06-05T20:59:05Z

When a response body applies backpressure, the HTTP/1 parser can be left paused. If the peer then closes the socket, onHttpSocketEnd calls parser.finish(), which previously asserted that the parser was not paused and could crash the process from the socket end handler.

This resumes the paused llhttp parser before finishing it, then lets the existing finish path handle EOF and completion normally.

Tests:

node --expose-gc --test --test-name-pattern "socket end completes response when body is paused by backpressure" test/client.js
npm run lint -- --no-cache lib/dispatcher/client-h1.js test/client.js
git diff --check

Copilot

Pull request overview

This PR fixes a crash in the HTTP/1 client path where parser.finish() could be invoked from the socket 'end' handler while the llhttp parser was paused due to response-body backpressure, previously triggering an assert(!this.paused) and crashing the process (Fixes #5360).

Changes:

Update Parser.finish() to resume a paused llhttp parser before calling llhttp_finish().
Add a regression test intended to cover the “socket end while body backpressures/pauses parsing” scenario.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
lib/dispatcher/client-h1.js	Makes `Parser.finish()` resilient to being called while parsing is paused by resuming llhttp first.
test/client.js	Adds a regression test for completing a response when the socket ends while the body causes backpressure.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+      let body = ''
+      data.body.setEncoding('utf8')
+      data.body.on('data', (chunk) => {
+        body += chunk
+      })
+      data.body.on('end', () => {
+        t.strictEqual(body, payload.toString())
+      })
+
+      setImmediate(() => {
+        data.body.resume()
+      })


ronag

I don' think the test actually test for the fix. Please confirm that the test fails without the fix.

ronag · 2026-06-06T09:04:49Z

Resuming without executing leaves the response incomplete (crash → hang). llhttp_resume() + llhttp_finish() stops the assertion crash, but when the parser was paused inside on_body, llhttp_finish does not fire on_message_complete, so request.onComplete is never called and the body stream hangs forever once the consumer reads it. Driving the parser forward the way resume() does fixes it:

if (this.paused) {
llhttp.llhttp_resume(this.ptr)
this.paused = false
this.execute(this.socket.read() || EMPTY_BUF) // drive parser to on_message_complete
}

The regression test passes without the fix. As Copilot noted, attaching a 'data' listener immediately switches the stream to flowing mode, so the parser never pauses. A reliable repro keeps the body unconsumed until after FIN (attach only 'end' initially, defer 'data').

When a response body applies backpressure, the HTTP/1 llhttp parser is left paused. If the peer then closes the socket, onHttpSocketEnd calls parser.finish(), which asserted `!this.paused` and crashed the process with an uncatchable AssertionError from the socket 'end' handler. finish() now resumes a paused parser and drains the socket through it so the response completes correctly across all body framings: - Content-Length / chunked bodies reach on_message_complete during execute() (driving the parser past the body is required; calling llhttp_finish() alone leaves them hanging). - EOF-delimited bodies (no length) can't complete via execute() and re-pause, so we resume once more and let llhttp_finish() deliver the EOF completion. - Backpressure is advisory here (onData keeps buffering delivered bytes), so we resume across pauses and loop until the socket buffer is empty, rather than parsing only a single read(). Truncated responses still surface as errors, never a false completion: short Content-Length -> UND_ERR_RES_CONTENT_LENGTH_MISMATCH, unterminated chunked -> HTTPParserError. Fixes nodejs#5360. Supersedes nodejs#5364, whose fix hangs Content-Length bodies (resume without execute) and whose test passes without the fix (the 'data' listener switches the stream to flowing mode so the parser never pauses). The tests here keep the body unconsumed until after FIN and cover Content-Length/EOF completion plus Content-Length/chunked truncation; all four fail against the unpatched parser. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

marko1olo · 2026-06-07T11:49:20Z

Updated in 5132ed84.

The regression now keeps the response body unconsumed until after the socket has ended, so the HTTP/1 parser actually remains paused when onHttpSocketEnd() calls finish().

Red/green proof locally:

With the test updated but without the new this.execute(this.socket.read() || EMPTY_BUF) flush in finish(), node --test --test-name-pattern "socket end completes response" test/client.js did not complete and hit my 60s command timeout.
With the current patch, the same command passes:
1 pass, duration_ms 1922.6807.

Also ran:

npx eslint lib/dispatcher/client-h1.js test/client.js
git diff --check

ronag · 2026-06-07T13:16:02Z

+      llhttp.llhttp_resume(this.ptr)
+      this.paused = false
+      this.execute(this.socket.read() || EMPTY_BUF)
+      if (this.paused || this.ptr == null) {


Is this condition really required?

Signed-off-by: marko1olo <barsukdana@gmail.com>

marko1olo · 2026-06-07T14:13:34Z

Good point. I removed that extra early-return condition in 57847129; the important part is just resuming llhttp and driving the parser with this.execute(this.socket.read() || EMPTY_BUF) before llhttp_finish().

Re-ran locally:

node --test --test-name-pattern "socket end completes response" test/client.js
npx eslint lib/dispatcher/client-h1.js test/client.js
git diff --check

codecov-commenter · 2026-06-07T14:25:59Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.26%. Comparing base (a8ea6f2) to head (5784712).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5364      +/-   ##
==========================================
+ Coverage   93.25%   93.26%   +0.01%     
==========================================
  Files         110      110              
  Lines       36738    36757      +19     
==========================================
+ Hits        34259    34281      +22     
+ Misses       2479     2476       -3

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

AliMahmoudDev · 2026-06-07T16:00:36Z


    const { llhttp } = this

+    if (this.paused) {
+      llhttp.llhttp_resume(this.ptr)
+      this.paused = false
+      this.execute(this.socket.read() || EMPTY_BUF)
+    }


the approach here makes sense — resuming the parser before finishing handles the backpressure case cleanly without changing the existing finish path.

one thought: the this.execute(this.socket.read() || EMPTY_BUF) call after resuming will try to consume whatever is left in the socket buffer. if the socket has already ended (which it has in this scenario), socket.read() returns null so we fall through to EMPTY_BUF. should be fine since the FIN is what triggers finish() in the first place, but worth noting that this only works correctly because onHttpSocketEnd is called after the kernel has drained the read buffer.

marko1olo · 2026-06-07T16:42:30Z

Follow-up on the remaining red check here: the failing job is Test with Node.js 25 on macos-latest, and its failure is in test/issue-5137.js with late async read ECONNRESET activity after the test ended. That is unrelated to this paused-parser/socket-end patch.

I split the HTTP/2 timeout test teardown fix into #5383 so this PR can stay scoped to parser finish behavior.

ronag requested a review from Copilot June 6, 2026 07:18

Copilot started reviewing on behalf of ronag June 6, 2026 07:18 View session

Copilot AI reviewed Jun 6, 2026

View reviewed changes

Comment thread test/client.js Outdated

Comment on lines +1222 to +1233

let body = ''

data.body.setEncoding('utf8')

data.body.on('data', (chunk) => {

body += chunk

})

data.body.on('end', () => {

t.strictEqual(body, payload.toString())

})

setImmediate(() => {

data.body.resume()

})

ronag requested changes Jun 6, 2026

View reviewed changes

ronag mentioned this pull request Jun 6, 2026

fix(h1): complete paused parser on socket end instead of crashing nxtedition/undici#2

Merged

marko1olo force-pushed the fix-h1-backpressure-assertion branch from 1335ea8 to 5132ed8 Compare June 7, 2026 11:48

ronag reviewed Jun 7, 2026

View reviewed changes

fix: finish paused HTTP/1 parser on socket end

5784712

Signed-off-by: marko1olo <barsukdana@gmail.com>

marko1olo force-pushed the fix-h1-backpressure-assertion branch from 5132ed8 to 5784712 Compare June 7, 2026 14:13

AliMahmoudDev reviewed Jun 7, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: finish paused HTTP/1 parser on socket end#5364

fix: finish paused HTTP/1 parser on socket end#5364
marko1olo wants to merge 1 commit into
nodejs:mainfrom
marko1olo:fix-h1-backpressure-assertion

marko1olo commented Jun 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

ronag left a comment

Uh oh!

ronag commented Jun 6, 2026

Uh oh!

marko1olo commented Jun 7, 2026

Uh oh!

ronag Jun 7, 2026

Uh oh!

marko1olo commented Jun 7, 2026

Uh oh!

codecov-commenter commented Jun 7, 2026

Uh oh!

AliMahmoudDev Jun 7, 2026

Uh oh!

marko1olo commented Jun 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

marko1olo commented Jun 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

ronag left a comment

Choose a reason for hiding this comment

Uh oh!

ronag commented Jun 6, 2026

Uh oh!

marko1olo commented Jun 7, 2026

Uh oh!

ronag Jun 7, 2026

Choose a reason for hiding this comment

Uh oh!

marko1olo commented Jun 7, 2026

Uh oh!

codecov-commenter commented Jun 7, 2026

Codecov Report

Uh oh!

AliMahmoudDev Jun 7, 2026

Choose a reason for hiding this comment

Uh oh!

marko1olo commented Jun 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants