Conversation
|
@robobun adopt |
|
Updated 2:18 AM PT - Jun 2nd, 2026
❌ @robobun, your commit ce0e093 has 8 failures in
🧪 To try this PR locally: bunx bun-pr 31522That installs a local version of the PR into your bun-31522 --bun |
|
✅ Multi-member gzip (RFC 1952 §2.2): @Jarred-Sumner's review (no de-opt): the libdeflate fast path decodes every member itself — @Jarred-Sumner's 2nd ask (Node consistency): @alii's follow-ups: rebased onto main; streaming-encode tests un-skipped on debug/ASAN with a measured 180s timeout (per #31505's approach) and confirmed green on the CI ASAN lanes; bomb-cap tests annotated manually-verified-only. Also rode along a one-line Windows baseline-allowlist fix (UCRT 10.0.26100's CI: 280 jobs green — every build lane, Linux/Windows/Alpine test lanes, ASAN. The only red is the macOS darwin agents flaking on unrelated infra (git SSH-prompt timeouts in |
|
Actionable comments posted: 0 |
|
Warning Review limit reached
More reviews will be available in 51 minutes and 51 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
WalkthroughThis PR adds RFC 1952 multi-member gzip stream support across Bun's decompression stack. The libdeflate fast path, Rust zlib wrapper, and runtime JSZlib implementations now detect partial member decodes, reset stream state, and continue processing subsequent members instead of truncating. Comprehensive test coverage validates single and multi-member scenarios, cross-runtime compatibility with Node.js, and error handling edge cases. ChangesMulti-member gzip concatenation
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks — 9bb1a93 addresses both actionable points (the streaming chunk-boundary keep-alive and the .zig revert) and adds a regression test that exercises the exact failure; the fix traces correctly to me. I'm not auto-approving only because this changes inflate-state lifetime across network chunks in the fetch gzip path, which is hot/critical enough to deserve a human sign-off.
Extended reasoning...
Overview
This PR fixes RFC 1952 §2.2 multi-member gzip decoding in Bun.gunzipSync and fetch(). Touched files: src/zlib/lib.rs (the ZlibReaderArrayList::read_all / ZlibReader::read_all StreamEnd handlers now inflateReset and continue when input remains, and keep the inflate state alive across incremental feeds when !is_done), src/http/InternalState.rs (libdeflate fast path falls back to the zlib slow path when a successful gzip decode left trailing input), plus tests in test/js/node/zlib/zlib.test.js and test/js/web/fetch/fetch-gzip.test.ts. My earlier review flagged a streaming-boundary gap and a CLAUDE.md .zig-reference convention issue; both were fixed in 9bb1a93 and the .zig hunks were dropped (now 4 files changed, no zlib.zig).
Security risks
No new external input surface; this is on the decode side of data already being decompressed. The added unsafe { *self.zlib.next_in } dereference is guarded by avail_in > 0, which is zlib's invariant for next_in validity. The libdeflate fallback discards the partial output and re-decodes via zlib rather than concatenating, so no risk of mixing partial buffers. Decompression-bomb guard (max_output_size) is unaffected — total_out is preserved across inflateReset so the budget still accumulates. I don't see an injection/auth/data-exposure angle.
Level of scrutiny
High. The StreamEnd keep-alive branch changes when inflateEnd runs for every gzip-encoded fetch() response on the incremental-decode path (Connection: close framing, streamed bodies). The reasoning — "inflate on a finished stream returns StreamEnd without consuming, then the avail_in > 0 && *next_in != 0 check resets or stops" — is correct per zlib semantics and mirrors NativeZlib::do_work_inflate, but it's a subtle cross-chunk state machine in production-critical code with raw FFI pointers. That warrants a human reviewer rather than bot approval.
Other factors
- Both of my actionable prior comments are resolved with code + a deterministic regression test ("decodes all members split across network reads").
- The remaining 🟣 note (explicit
{ library: "libdeflate" }opt-in still single-member) was filed as pre-existing/non-blocking follow-up and doesn't gate this PR. - The robobun build-status comment still references the pre-fix commit (d4cf751) with an ❌; CI for 9bb1a93 isn't reflected in the thread yet.
- No new findings from the current bug-hunting pass.
Jarred-Sumner
left a comment
Jarred-Sumner
left a comment
There was a problem hiding this comment.
This de-opts the libdeflate code path. We need to not do that. We should instead detect if there're more entries and resume from the unread portion.
Jarred-Sumner
left a comment
Jarred-Sumner
left a comment
There was a problem hiding this comment.
Can we add a test that runs in Node.js and Bun and verifies the behavior is consistent?
ec5eeae to
34e75de
Compare
There was a problem hiding this comment.
Nothing further from me on ebc4684 — all prior threads are resolved. Given this rewrites the HTTP-thread gzip decompression path for attacker-controlled input and took several rounds to get the bomb-cap / grow-loop semantics right, it's worth a human pass before merge.
Extended reasoning...
Overview
This PR makes Bun.gunzipSync and fetch() (Content-Encoding: gzip) decode all members of a concatenated RFC 1952 §2.2 gzip stream instead of silently truncating to the first. It touches four Rust source files — src/zlib/lib.rs (multi-member inflateReset loop in ZlibReaderArrayList::read_all, new window_bits field, end() gate fix), src/http/InternalState.rs (new ~85-line decompress_gzip_members libdeflate loop and a rewrite of the libdeflate fast-path branch in decompress_bytes), src/runtime/api/BunObject.rs (new ~75-line libdeflate_gzip_all_members for the {library:'libdeflate'} opt-in), and a one-line visibility bump in src/http/Decompressor.rs — plus ~300 lines of new tests across two test files.
Security risks
This is squarely security-sensitive: it rewrites how the HTTP thread decompresses attacker-controlled Content-Encoding: gzip response bodies. Over the course of review I flagged (and the author fixed) an unbounded ISIZE-driven ~4 GiB pre-allocation, two separate Vec::reserve no-op infinite loops that hung the HTTP thread / JS main thread, an error→silent-success regression on leading-zero/empty input, and a decompression-bomb-cap bypass via the spare-empty grow path. All of those are resolved on ebc4684 and the bug hunter found nothing new, but the density of subtle issues in the buffer-growth and bomb-cap logic argues for a human sanity check.
Level of scrutiny
High. This is not a mechanical change: it introduces two new hand-rolled libdeflate retry loops with their own capacity-doubling and 1 GiB cap semantics, changes the StreamEnd state machine in the incremental zlib reader (including a new keep-alive-across-network-reads branch), and reroutes the fetch fast path for all gzip bodies. The fact that it took ~15 review iterations across 7 rounds to converge — with multiple 🔴 findings along the way — is itself a signal that the invariants here are tricky.
Other factors
Test coverage is good (multi-member, zero padding, large bodies, chunked/streaming slow path, member-boundary-on-network-read, leading-zero error, the two grow-loop regression guards, and a Node-vs-Bun cross-runtime diff), though the two >1 GiB cap regression tests are skipIf(isCI). CI on the latest commit shows one unrelated vendor/elysia failure. No CODEOWNERS apply to these paths. All inline threads are marked resolved.
|
@robobun — Jarred's review is still open and the core ask isn't done yet. He wrote: "This de-opts the libdeflate code path. We need to not do that. We should instead detect if there're more entries and resume from the unread portion." The current multi-member handling falls back to the zlib slow path whenever input remains after the first gzip member (the libdeflate fast path in Please make the libdeflate fast path decode all members itself:
His second ask — a Node-and-Bun consistency test: in Verify: a multi-member body decodes fully on the fast path (no fallback — confirm both fast and |
|
Thanks @alii — the libdeflate fast path already decodes every member itself; no fallback to zlib for trailing members. That rewrite landed earlier in the PR (55316c7 + the bomb-cap fixes), so Jarred's de-opt is already addressed. Walking through it so it's easy to confirm: The fast-path loop — let mut offset = 0usize;
while offset < input.len() {
if offset > 0 && input[offset] == 0 { break; } // trailing zero padding, like node's *next_in != 0
// grow-and-retry on InsufficientSpace, under the 1 GiB bomb cap …
let result = decompressor.decompress_to_vec(&input[offset..], out, Encoding::Gzip);
if result.status != Success { out.truncate(start_len); return Err(()); } // bad data only
if result.read == 0 { break; }
offset += result.read; // advance by bytes consumed
}
The consistency tests you asked for (c7bef0a):
Verified: all pass on |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/http/InternalState.rs`:
- Around line 170-193: The decompression-bomb cap is only checked with
`out.capacity() > MAX_DECOMPRESSED_BODY_SIZE` and before calling
`grow_to_double_capacity`, allowing a buffer at exactly the cap to still grow;
change the initial check to `>= MAX_DECOMPRESSED_BODY_SIZE` and also add a guard
in the `InsufficientSpace` branch before calling `grow_to_double_capacity` that
truncates to `start_len` and returns `Err(())` if `out.capacity() >=
crate::decompressor::MAX_DECOMPRESSED_BODY_SIZE`, ensuring no further
reserve/doubling occurs (refer to the `grow_to_double_capacity` closure, the
loop around `decompressor.decompress_to_vec`, and the existing
`out.truncate(start_len); return Err(());` behavior).
In `@src/runtime/api/BunObject.rs`:
- Around line 2407-2427: The guard allows growing when out.capacity() == 1 GiB,
so change grow_to_double_capacity (used by the decompression loop and referenced
when handling bun_libdeflate::Status::InsufficientSpace and the spare_capacity
check) to prevent any reserve that would make capacity exceed the 1 GiB ceiling
— e.g. compute the intended target = out.capacity().max(4096).saturating_mul(2)
and return false if target > MAX_BYTES (1 << 30) (or use >= on capacity) before
calling out.reserve; ensure both the initial spare_capacity_mut() branch and the
InsufficientSpace retry path use this updated logic.
In `@test/js/node/zlib/zlib.test.js`:
- Around line 917-928: The current run function awaits proc.exited (exitCode) in
parallel with proc.stdout.text() so the test asserts exit code before verifying
stdout; change the ordering to first await and parse stdout and stderr from
proc.stdout.text() and proc.stderr.text(), then parse JSON from stdout
(capture/propagate the raw stdout on parse errors), and only after successful
parsing assert that exitCode (from await proc.exited) is 0; update references to
proc, stdout, stderr, exitCode, and the JSON.parse step in run accordingly.
In `@test/js/web/fetch/fetch-gzip.test.ts`:
- Line 285: The test currently calls expect(fetch(server.url).then(r =>
r.text())).rejects.toThrow() without awaiting or returning the promise, so the
assertion may not run; update the assertion in the fetch-gzip test to await the
rejects matcher (e.g., await expect(fetch(server.url).then(r =>
r.text())).rejects.toThrow()) or return the expectation so the test runner waits
for the rejection from fetch(...).then(r => r.text()) to be asserted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 56734fac-8d06-44dd-b0a7-4d50a250f628
📒 Files selected for processing (6)
src/http/Decompressor.rssrc/http/InternalState.rssrc/runtime/api/BunObject.rssrc/zlib/lib.rstest/js/node/zlib/zlib.test.jstest/js/web/fetch/fetch-gzip.test.ts
|
@robobun this was green earlier but CI went red after main moved (a single sub-job failing — looks flaky/infra). Please rebase onto main and re-run CI; if a test genuinely fails, investigate and fix it. |
c2ac860 to
36581dc
Compare
|
Rebased onto main (conflict was just the import block in fetch-gzip.test.ts — main's new node:zlib import absorbed my gunzipSync addition) and force-pushed as 36581dc. Full local run of both test files against the rebased branch: 414 pass / 0 fail, including main's new Content-Encoding tests alongside the multi-member block. CI re-running now — if anything genuinely fails I'll dig in. |
Concatenated multi-member gzip decoding was extended to Bun.gunzipSync and fetch in the previous commit, but fetch decompresses the body incrementally (one network read at a time on the Connection: close / streamed path). When a member's trailer landed exactly on a read boundary, inflate returned StreamEnd with avail_in == 0, so read_all ran end() -> inflateEnd and marked the reader End. The next chunk's update_buffers re-seated the input but never reset the state, so read_all returned immediately and the remaining members were dropped (and, because a prior member reset could leave avail_in != 0, update_buffers' avail_in == 0 assertion could panic). On StreamEnd in a gzip-capable mode with no input left and the stream not done, keep the inflate state alive (do not end()) and stay Inflating so the next read re-enters: inflate on a finished stream returns StreamEnd without consuming, then the existing avail_in > 0 && *next_in != 0 check resets for the next member or stops on zero padding -- matching node:zlib's GUNZIP loop, including tolerating a trailing zero-padding chunk without erroring. Also drop the non-compiled zlib.zig reference hunks from the previous commit; new behavior lives in lib.rs.
The multi-member gzip tests share zlib.test.js and fetch-gzip.test.ts with a few pre-existing cases that are flaky under the sanitizer-instrumented debug build: - zlib.brotli / zlib.zstd "streaming encode doesn't wait for entire input" push 50 MB of random data through the compressor and can't finish in the 15s budget under ASAN (they pass on the release lanes). Skip them there via an isDebug/isASAN-gated `it`. - fetch-gzip's "multiple chunks, TCP server" bound its listener to "localhost"; on a dual-stack host that can resolve to ::1 while the socket binds 127.0.0.1, so fetch hit ConnectionRefused. Bind 127.0.0.1 directly. No behavior change to the gzip fix or its tests.
Re-run CI — the only red lanes are unrelated flakes on Windows: node-tls-root-certs-concurrent-init.test.ts crashed (illegal instruction) on windows-11-aarch64 and the windows-2019-x64-baseline shard was SIGKILLed mid-run. This PR touches only gzip inflate (src/zlib, src/http) and its tests; root-cert init parses plaintext PEM (no zlib), and the ASAN test lane that runs these gzip tests passed.
setImmediate between each member's write lets the socket flush it as its own network read before the next is queued, so the test deterministically exercises the cross-chunk !is_done keep-alive path (a bare microtask could leave all members in one TCP segment on fast loopback, exercising only the in-buffer inflateReset path). Still panics on base and passes with the fix.
The previous commit accidentally reverted src/zlib/lib.rs to the base version (a stray staged revert from a local verification step). Restore the StreamEnd keep-alive / multi-member inflateReset logic from 9bb1a93.
ZlibReader::end() was gated on state == Inflating, so the StreamEnd arm's `state = End; end()` sequence (and the Error-state Drop paths) skipped inflateEnd and would leak zlib's internal_state. Match ZlibReaderArrayList::end's `!= End` gate and drop the premature `state = End` so end() frees while still Inflating. ZlibReader (the streaming-writer variant) has no callers, so this is a correctness cleanup with no runtime effect today.
Addressing review: the previous commit made fetch's libdeflate fast path fall back to the zlib slow path whenever a gzip body had trailing input (a concatenated multi-member stream, RFC 1952 §2.2), de-opting the common case. Instead, loop libdeflate over the members: libdeflate_gzip_decompress_ex reports bytes consumed per member (result.read), so resume decoding from the unread portion, appending each member's output. Trailing zero bytes stop the loop (matching node:zlib's *next_in != 0); a member that fails to decode (genuine trailing garbage) falls back to the zlib path, which produces the matching error. Single-member gzip and deflate are unchanged. The zlib-path multi-member fix stays: it serves Bun.gunzipSync and fetch's streaming/chunked slow path, where the whole stream isn't present in one call.
Per review, add a test that spawns the same fixture under both node and bun and diffs the output: concatenated multi-member streams, trailing zero padding, trailing gzip-header-shaped garbage, single member, and a larger multi-member body. node:zlib.gunzipSync must match Node exactly for every case (including the thrown-error shape on garbage); Bun.gunzipSync must match Node's decoded bytes for the successful cases. Skips when node isn't on PATH.
Review follow-ups on the libdeflate multi-member loop: - Leading 0x00: the 'trailing zero is padding' stop fired at offset 0, so a Content-Encoding: gzip body starting with 0x00 returned Ok with nothing decoded -> fetch delivered a silent empty 200 (was: errors). Gate the stop on offset > 0 so a leading zero is a bad header that falls through to the erroring zlib path. Added a regression test. - Unbounded reserve: the normal-path gzip branch reserved the attacker- controlled ISIZE trailer with no cap (a 20-byte body with ISIZE=0xFFFFFFFF -> ~4 GiB alloc). Cap the hint at 32 MiB like the big-body path; the loop still grows under the 1 GiB bomb cap. - Share Decompressor::MAX_DECOMPRESSED_BODY_SIZE (pub(crate)) instead of an inline 1 GiB literal.
…e' }) The explicit libdeflate opt-in used libdeflate's single-shot decoder, so it returned only the first member of a concatenated stream. Loop over the members using the per-member consumed count (result.read) to resume from the unread portion, matching the default zlib path and node:zlib: trailing zero padding stops the loop, a leading zero / bad data errors, and output is capped at 1 GiB. deflate stays single-member. Added a test.
The InsufficientSpace retry grew the output with reserve(capacity), but Vec::reserve is relative to len, not capacity: while a member decodes, len stays 0 (it only advances on Success), so once capacity >= 4096 the reserve(capacity) call is a no-op -> spare capacity never grows -> the decode re-fails -> infinite loop hanging the thread. Triggers whenever the first member's plaintext exceeds the last member's ISIZE (the reserve hint), e.g. gzip(1MB) ++ gzip(600KB). Double the capacity explicitly instead (reserve(target - len)), mirroring decompress_to_vec_grow. Applies to both the fetch path (decompress_gzip_members) and Bun.gunzipSync's libdeflate opt-in. Added regression tests that hang on the old code.
An empty buffer has no gzip header, so the multi-member loop exited immediately and returned Success with nothing decoded — a silent empty success, diverging from the default zlib path and node:zlib, which both throw. Reject empty input up front. (The fetch path's empty-body case is short- circuited upstream and correctly returns an empty body, matching Node's fetch, so it is left as-is.) Added tests for the empty-input error and a single-member highly-compressible payload that exercised the grow loop.
The MAX_DECOMPRESSED_BODY_SIZE check sat only in the InsufficientSpace branch, so the pre-decode spare-empty doubling could chain past 1 GiB on a stream whose members keep filling the spare exactly (Success with len == cap, never InsufficientSpace). Hoist the check to the top of the inner loop so it covers both grow paths, matching libdeflate_gzip_all_members. Also correct a comment: the reserve hint is the last member's ISIZE trailer, not the first/largest.
The default bun:test timeout already bounds the infinite-loop regression guards; an actual hang still fails the test.
Bun.gunzipSync(buf, { library: 'libdeflate' }) sizes the output Vec from
the input when the gzip ISIZE hint is >= 256 MB or 0 (it falls through to
Vec::with_capacity(compressed.len())). A gzip stream whose compressed size
exceeds 1 GiB therefore entered libdeflate_gzip_all_members with a capacity
already past the 1 GiB decompression-bomb cap, and the cap check at the top
of the grow loop returned InsufficientSpace before any decode ran — the
caller mapped that to a premature OutOfMemory throw, even though the whole
output fit in the space the caller had already reserved.
Pre-PR this path used decompress_to_vec_grow, which decodes first and only
consults the cap after an InsufficientSpace doubling, so the same input
succeeded. Restore that: grow_to_double_capacity now returns false once a
doubling would exceed 1 GiB, and the loop only checks it after growing the
buffer. A large initial reservation justified by the input size decodes;
genuine amplification (a small input growing past 1 GiB) is still rejected.
Test allocates a >1 GiB stored-block gzip and asserts it decodes; gated off
CI for memory and given a long timeout for the payload build.
a6d1562 moved the cap check into grow_to_double_capacity but consulted it *after* the doubling, making the grow path one doubling stricter than the pre-PR decompress_to_vec_grow: that helper checks capacity > max_capacity *before* doubling, so it permits one doubling past 1 GiB and a decode at that size, whereas the after-check rejected a doubling that landed past 1 GiB without attempting the decode into the just-allocated buffer. A single-member ~700 MB output (tiny compressed, so the output Vec starts sub-MB and doublings top out at ~696 MB < 700 MB, requiring a final double to ~1.39 GiB) therefore threw OutOfMemory though it succeeded before the PR. Check the cap before doubling to restore the one-overshoot semantics. The >1 GiB entry-capacity fix from a6d1562 still holds: on entry the spare is non-empty, so the first decode runs before any grow. Split the regression test into the two distinct paths (compressed > 1 GiB; grow loop doubling past 1 GiB on a ~700 MB output) and drop the ad-hoc 120s timeout for the file's existing 15s large-payload convention.
…zlib
The libdeflate fast path already decodes every gzip member itself (via
decompress_gzip_members, which advances by libdeflate_gzip_decompress_ex's
actual_in_nbytes_ret and never falls back to zlib for trailing members).
These tests pin that behavior to node:zlib:
- fetch() of a Content-Encoding: gzip multi-member body now byte-equals
require('zlib').gunzipSync(body) for both the small (fast-path) and large
cases.
- A subprocess test fetches the same multi-member body with libdeflate
enabled (default) and with BUN_FEATURE_FLAG_NO_LIBDEFLATE=1 (zlib slow
path) and asserts both decode to identical bytes, and that both match
node:zlib — proving there is no de-opt and the two paths agree.
- Bun.gunzipSync({ library: 'libdeflate' }) multi-member output is asserted
equal to both the default zlib path and node:zlib.
All fail on the released bun (first-member truncation) and pass on a build
with the fix.
- fetch-gzip: the 'errors on an invalid gzip body' test asserted rejects.toThrow() on a floating promise, so it could finish before the assertion resolved; await it. - zlib cross-runtime helper: surface stdout (and require it non-empty) before asserting the exit code, so a malformed fixture gives a useful message instead of a bare JSON.parse throw.
…lper Mirror the fix applied to the zlib.test.js run() helper in 52ffa8a: the sibling subprocess helper here had the same shape and Buffer.from("", "hex") silently returns an empty buffer, so an empty-body regression would fail the downstream comparison with no diagnostic. Require stdout non-empty first.
Both callers read buffer[len-4..] — the trailing member's ISIZE. The call-site comments already say so; align the decompress_gzip_members docstring.
c0c5f4a to
aaac946
Compare
|
@robobun two test-coverage nits before this is ready for re-review:
Everything else looks right: the libdeflate fast path now loops over all members via |
Instead of skipping the brotli/zstd 'streaming encode doesn't wait for entire input' tests under the sanitizer-instrumented build, give them a longer timeout like #31505 (with extra headroom: 50 MB through ASAN brotli measures ~64s on a slow runner, so 180s instead of 60s). Also note on the two bomb-cap edge tests that the cap-ordering is manually-verified-only: they allocate multiple GiB so they skip on CI, and the 1 GiB cap is hardcoded with no injection point.
|
Done in 9f92fd2:
|
The builders' updated Windows SDK (UCRT 10.0.26100.8249) ships a strpbrk whose vectorized path now emits CLDEMOTE cache-line-demote hints, failing the static baseline scan. strpbrk was already allowlisted for AVX/AVX2 as runtime-gated on __isa_available; CLDEMOTE is additionally encoded in the hint-NOP space, so it executes as a NOP on CPUs without the feature and cannot fault on the Nehalem baseline.
|
Heads-up on an unrelated CI fix that rode along (ce0e093): the Windows x64-baseline static scan started failing after the builders picked up a newer Windows SDK — UCRT 10.0.26100's |
This branch has not been deployed
3 participants
A concatenated (multi-member) gzip stream — multiple gzip members back-to-back per RFC 1952 §2.2 (what
cat a.gz b.gzor many log pipelines produce) — was silently truncated to the first member byBun.gunzipSyncand byfetch()withContent-Encoding: gzip(HTTP 200, no error, just missing data).node:zlib.gunzipSyncand Node's fetch decode all members.read_allreturned onZ_STREAM_ENDwithout checking for remaining input, so it neverinflateReset'd to decode the next member; and fetch's libdeflate fast path is single-shot. Fix: onStreamEndwith input remaining (and the next byte non-zero — trailing zero padding stops the loop),inflateResetand continue, mirroringnode:zlib's GUNZIP loop (NativeZlib). The loop is gated to gzip-capable inits, soinflateSync/ zlib-wrapped streams are unchanged; the fetch libdeflate fast path falls back to the now-correct slow path when trailing input remains.Verified against Node — including Node's own
test-zlib-from-concatenated-gzipandtest-zlib-from-gzip-with-trailing-garbageparallel tests — acrossBun.gunzipSyncand fetch's fast + slow paths; single-member and deflate/br/zstd unaffected. Not a port regression: 1.3.14 truncates the same way.