Skip to content

[v6.x] fix(fetch): error the stream when decompression fails after onComplete - #6001

Open
mcollina wants to merge 2 commits into
nodejs:v6.xfrom
mcollina:fix/5999-broken-gzip-body-hang
Open

mcollina wants to merge 2 commits into
nodejs:v6.xfrom
mcollina:fix/5999-broken-gzip-body-hang

Conversation

@mcollina

Copy link
Copy Markdown
Member

This relates to...

Fixes #5999

Rationale

On v6.x (6.7.0 – 6.29.0, the line bundled with Node 22), a response that declares content-encoding: gzip but carries a body that is not valid gzip never settles text(), json() or a stream read when the body arrives in a later packet than the headers. The request's AbortSignal cannot end it either, so the promise hangs forever.

onComplete removes the fetch controller's terminated listener (onAborted, added in #2823) as soon as the raw bytes have arrived. Decompression happens later in the body pull loop, so when zlib fails (Z_DATA_ERROR) and the loop calls controller.terminate(err), nothing errors the response stream.

main fixed this in #3754 by no longer removing the listener in onComplete and registering it only once per fetch. That change was never backported.

Changes

  • Backport of afeb626c (fix aborting Streams #3754) onto v6.x.
  • Register the onAborted listener only once per fetch (guarded by fetchParams.controller.resume) and stop removing it in onComplete. Keeping the listener registered means a late decompression failure correctly errors the response stream; registering it once per fetch prevents the MaxListenersExceededWarning on many redirects (redirecting >11 times causes "MaxListenersExceededWarning" #1711) that the previous per-request registration caused.
  • Add a regression test reproducing Node 22 fetch hangs forever on a broken gzip body #5999 to test/fetch/encoding.js. It times out without the fix.
  • Include the "aborting a Stream throws" test from the cherry-picked commit in test/fetch/issue-1711.js.

Bug Fixes

  • fetch: the body stream is errored when decompression fails after onComplete.

Verification

  • The reproduction from Node 22 fetch hangs forever on a broken gzip body #5999 now rejects with terminated (Z_DATA_ERROR) instead of hanging.
  • test/fetch/issue-1711.js (redirects + abort-stream) passes.
  • Ran test/fetch/*.js with --expose-gc — all pass except pre-existing environment failures caused by missing dev deps (formdata-node, jsdom) and a Node version --max-http-header-size difference, which fail identically on the unpatched branch.
  • standard lint passes on the changed files.

Status

The body pull loop runs decompression after onComplete has already
fired, but onComplete removed the fetch controller's 'terminated'
listener (onAborted). When zlib fails (e.g. a response declares
content-encoding: gzip but the body is not valid gzip), controller
termination never reaches the response stream, so text(), json() and
stream reads hang forever and the AbortSignal cannot end them.

Register the onAborted listener only once per fetch (guarded by the
presence of fetchParams.controller.resume) and stop removing it in
onComplete. This keeps the listener alive until the fetch settles so a
late decompression failure correctly errors the stream, while avoiding
the MaxListenersExceededWarning on redirects that the single
registration prevents.

Fixes nodejs#5999

Backport of nodejs#3754.

Refs: nodejs#3754
@mcollina mcollina closed this Oct 13, 2026
Deferring the gzip body with a wall-clock setTimeout is flaky and adds a
fixed delay. Defer it to a later event-loop tick with setImmediate
instead, which deterministically splits the headers and body into
separate packets without any wall-clock dependency.

Refs: nodejs#5999
@mcollina mcollina reopened this Oct 14, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant