fix(readable): keep body bytes that arrive after setEncoding() - #5620
Merged
Conversation
A response body was silently truncated when setEncoding() was used and the body arrived in more than one chunk: .text(), .json(), .bytes(), .arrayBuffer() and .blob() all returned short data and raised nothing. setEncoding() installs a StringDecoder, so state.buffer holds decoded strings from then on and the trailing bytes of a multi-byte sequence split across a chunk boundary sit inside the decoder. #5003 worked around that by snapshotting the raw chunks buffered at the time of the call into kPreservedBuffer, but chunks arriving afterwards were never added, and consumeStart() treated the two sources as mutually exclusive, so everything but the snapshot was dropped. Drop kPreservedBuffer and read the single source of truth instead: state.buffer, re-encoded back to bytes by consumePush(), plus the bytes the decoder is still holding. Nothing is retained beside the stream's own buffer, and push() stays untouched on the hot path. Fixes: #5611
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5620 +/- ##
==========================================
- Coverage 93.49% 93.49% -0.01%
==========================================
Files 110 110
Lines 38429 38414 -15
==========================================
- Hits 35931 35916 -15
Misses 2498 2498 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The chunk that has to arrive after setEncoding() is now released by a handshake with the server rather than a 50ms timer, and the wait for the body to arrive is the client's 'drain' event rather than a 150ms sleep.
metcoder95
approved these changes
Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5611. Alternative to #5612, which fixes the same bug by retaining a second copy of the body.
The bug
setEncoding()installs aStringDecoder(since #5003), so from that pointstate.bufferholds decoded strings, and the trailing bytes of a multi-byte sequence split across a chunk boundary sit inside the decoder, in neither place. #5003 worked around this by snapshotting the raw chunks that happened to be buffered at the time of the call intokPreservedBuffer. Chunks arriving after the call were never added to it, andconsumeStart()treated the two sources as mutually exclusive:so everything outside the snapshot was dropped, silently:
The fix
Remove
kPreservedBufferand read the one source of truth the stream already keeps:consumePush()re-encodes a string chunk back to bytes, soconsume.lengthstays a byte count. Without that.arrayBuffer()/.bytes()return a zero-filled buffer of the wrong length, because a string'slengthis in UTF-16 code units andUint8Array.prototype.set(string)writes nothing.consumeStart()then appends the bytes the decoder is holding for an incomplete sequence, which are absent from every string instate.buffer.That covers chunks buffered before
setEncoding(), after it, and after the consume was requested, because all of them end up in exactly one of those two places. The "which entry ofstate.bufferis the duplicate?" problem that makes reading both sources unworkable only exists whilekPreservedBufferdoes; deleting it removes the ambiguity rather than arbitrating it.Net effect on
lib/: -15 lines,push()untouched, and nothing retained beyond the stream's own buffer. #5003 is not undone —setEncoding()+for awaitand +on('data')still decode a split multi-byte character correctly.Why not keep the raw chunks
#5612 makes
push()append every raw chunk while an encoding is set, which is a second full copy of everything buffered, allocated on the hot path. Backpressure bounds it (api-request.jspauses whenpush()returnsfalse), but not at one highWaterMark: with a decoder installedstate.lengthcounts decoded characters, so 64 KiB of buffered CJK is ~192 KiB of retained bytes on top. The release also lags apush()behind the first emitted chunk.Trade-off
Re-encoding a decoded string is byte-exact for
latin1,hex,base64,utf16leand valid UTF-8, and only applies to what was buffered before the consume started — chunks arriving later reachconsumePush()as rawBuffers. It is lossy in two corners: invalid UTF-8 bytes undersetEncoding('utf8')come back as U+FFFD, and high-bit bytes undersetEncoding('ascii')come back masked to 7 bits, if a.bytes()/.arrayBuffer()/.blob()consume reads them out of the pre-consume buffer. Both mean "decode this as text, then hand me the raw bytes back", and a plainnode:streamReadable loses the same information. Retaining raw chunks is the only way to be exact there, at the cost above.Tests
test/readable.jsgains the 7 cases from #5612 (@marko1olo) — chunks pushed aftersetEncoding(), a partial character buffered,utf8/hex/base64/latin1before any chunk arrives, and.json()— andtest/client-request.jsgains the end-to-end repro from #5611 over a real socket. All were confirmed failing onmainfirst, with the truncation,ERR_INVALID_ARG_TYPEfromBuffer.concat, and a zero-filledUint8Array.test/readable.js18/18,test/client-request.js49/49,npm run test:unit1459 pass / 0 fail,npm run test:fetch47/47,npm run test:typescriptandnpm run lintclean. Node 24, Linux.