fetch: add automatic request body compression via compress option - #32416
Merged
Conversation
Accepts `boolean | "gzip" | "deflate" | "br" | "zstd" | { encoding, level? }`.
Compresses buffered bodies (string, ArrayBuffer/TypedArray, Blob) on the JS
thread using a thread-local libdeflate compressor + 512 KiB scratch buffer
(mirroring the response-decompression fast path), and injects the
`Content-Encoding` request header. ReadableStream and sendfile bodies are
left untouched, as are requests that already set `Content-Encoding`.
Collaborator
|
Updated 6:22 PM PT - Jun 16th, 2026
❌ @Jarred-Sumner, your commit 6aec4e2 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 32416That installs a local version of the PR into your bun-32416 --bun |
Jarred-Sumner
commented
Jun 16, 2026
Jarred-Sumner
left a comment
Jarred-Sumner
left a comment
Collaborator
Author
There was a problem hiding this comment.
Use the existing pattern for parsing enums into a ComptimeStringMap - avoid calling .to_utf8 for this it's completely unnecessary.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a Changesfetch() request-body compression
Suggested reviewers
🚥 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}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/runtime/webcore/fetch/compress_body.rs`:
- Around line 161-167: The compressor_mut function signature allows returning a
mutable reference with an unconstrained lifetime 'a from an immutable reference
&self, which violates Rust's borrow rules. Fix this by changing the function
signature to properly constrain the lifetime of the returned mutable reference
to be tied to self's lifetime. Remove the explicit generic lifetime parameter 'a
from the function signature and instead let the Rust compiler infer the
lifetime, or change the parameter from &self to &mut self to properly express
the exclusive borrow semantics that the code actually relies on. The returned
reference to the compressor should not outlive the CompressorState instance it
belongs to.
In `@test/js/web/fetch/fetch-compress.test.ts`:
- Around line 186-187: The fetch calls in the negative validation tests are
using the external network host `http://example.com`, which can cause tests to
become non-hermetic and flaky if validation behavior shifts from sync to async.
Replace all occurrences of `http://example.com` with a local URL like
`http://127.0.0.1` or `http://localhost` to ensure tests remain hermetic and do
not contact external network hosts. The comment indicates this issue appears in
multiple test cases within the file.
🪄 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: 4e679ecf-d001-4318-9469-008513caf4c5
📒 Files selected for processing (4)
packages/bun-types/globals.d.tssrc/runtime/webcore/fetch.rssrc/runtime/webcore/fetch/compress_body.rstest/js/web/fetch/fetch-compress.test.ts
…sor_mut - Add fragmented-response leak test: drips gzip/br/zstd over chunked TE to force the per-request boxed Decompressor path (bypasses libdeflate one-shot fast path), paired with a 700 KiB compress: request body for multi-write send. - Drop CompressorState::compressor_mut<'a>(&self) -> &'a mut and inline its single call site with a SAFETY comment; the unbounded-lifetime signature was correct (C heap handle disjoint from shared_buffer) but read as unsound.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runtime/webcore/fetch/compress_body.rs (1)
166-173:⚠️ Potential issue | 🟠 MajorRelease the cached libdeflate compressor on thread exit.
The
CompressorStateis stored in a thread-local static (line 155-158) and thecompressorraw pointer is allocated at line 167 viaCompressor::alloc(). However, noDropimplementation exists to free this allocation when the thread exits. The comment at line 225-226 even acknowledges it is "never freed for the thread's lifetime." Only the temporary compressor (lines 231-237) is properly destroyed via scopeguard.Suggested fix
struct CompressorState { compressor: *mut bun_libdeflate_sys::libdeflate::Compressor, shared_buffer: [u8; SHARED_BUFFER_SIZE], } + +impl Drop for CompressorState { + fn drop(&mut self) { + if !self.compressor.is_null() { + // SAFETY: allocated via `Compressor::alloc`, owned by this state. + unsafe { bun_libdeflate_sys::libdeflate::Compressor::destroy(self.compressor) }; + self.compressor = core::ptr::null_mut(); + } + } +}🤖 Prompt for 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. In `@src/runtime/webcore/fetch/compress_body.rs` around lines 166 - 173, The `CompressorState` struct, which holds a raw pointer to a libdeflate compressor allocated via `Compressor::alloc()`, lacks a `Drop` implementation to properly free this allocation when the thread-local static is destroyed at thread exit. Implement the `Drop` trait for `CompressorState` to deallocate the compressor pointer using the appropriate libdeflate deallocation method (similar to how the temporary compressor is properly destroyed via scopeguard in the surrounding code at lines 231-237). This ensures the cached compressor allocation is freed instead of leaking for the thread's lifetime.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@src/runtime/webcore/fetch/compress_body.rs`:
- Around line 166-173: The `CompressorState` struct, which holds a raw pointer
to a libdeflate compressor allocated via `Compressor::alloc()`, lacks a `Drop`
implementation to properly free this allocation when the thread-local static is
destroyed at thread exit. Implement the `Drop` trait for `CompressorState` to
deallocate the compressor pointer using the appropriate libdeflate deallocation
method (similar to how the temporary compressor is properly destroyed via
scopeguard in the surrounding code at lines 231-237). This ensures the cached
compressor allocation is freed instead of leaking for the thread's lifetime.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ded26e03-3afd-4da2-81ce-62bad1723836
📒 Files selected for processing (3)
src/runtime/webcore/fetch/compress_body.rstest/js/web/fetch/fetch-compress.test.tstest/js/web/fetch/fetch-leak.test.ts
…n-integer level - Bun.file() bodies that qualified for the sendfile fast path (plain http, no proxy, >=32 KiB, non-Windows) silently skipped explicit compression while the same call over https/proxy/small-file/Windows compressed. Gate sendfile on compress.is_none() so an explicit request always wins. - compress.level: NaN/5.5/Infinity passed is_number() and to_int32() mapped them to in-range values (NaN->0). Validate via as_number() + is_nan()/fract() like maxRedirects does.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/runtime/webcore/fetch/compress_body.rs (1)
172-179:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRelease the cached libdeflate compressor when the TLS state drops.
Line 172 allocates a C compressor and Line 178 stores it in
CompressorState, but the state has noDropimpl, so dropping the TLSBox<CompressorState>only frees the Rust struct and leaks the libdeflate handle. Add aDropowner for the cached handle, mirroring the temporary compressor guard below.Proposed fix
struct CompressorState { compressor: *mut bun_libdeflate_sys::libdeflate::Compressor, shared_buffer: [u8; SHARED_BUFFER_SIZE], } +impl Drop for CompressorState { + fn drop(&mut self) { + if !self.compressor.is_null() { + // SAFETY: `compressor` was returned by `Compressor::alloc` and is + // owned by this TLS state. + unsafe { + bun_libdeflate_sys::libdeflate::Compressor::destroy(self.compressor); + } + self.compressor = core::ptr::null_mut(); + } + } +} + // SAFETY: `*mut T` (null) and `[u8; N]` are both valid at the all-zero bit pattern. unsafe impl bun_core::Zeroable for CompressorState {}As per coding guidelines, “Pair every acquisition with its release at the acquisition site using Drop/RAII guards.”
🤖 Prompt for 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. In `@src/runtime/webcore/fetch/compress_body.rs` around lines 172 - 179, The CompressorState struct holds a libdeflate compressor handle allocated at line 172 but has no Drop implementation, causing a memory leak when the TLS-stored state is dropped. Add a Drop implementation for the CompressorState struct that properly releases the libdeflate compressor handle by calling the appropriate deallocation function from bun_libdeflate_sys when the struct is dropped, following the RAII pattern of pairing acquisition with release at the acquisition site.Source: Coding guidelines
🤖 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/runtime/webcore/fetch.rs`:
- Around line 1659-1662: The sendfile eligibility check around line 1662
disables sendfile whenever compress is set, but the compression logic at lines
1777-1781 skips actual compression if Content-Encoding is already present. Use
the same predicate in both locations: modify the condition checking
`compress.is_none()` at line 1662 to also verify that Content-Encoding is not
already set in the response headers, ensuring sendfile is only disabled when
compression will actually be applied to the body.
---
Outside diff comments:
In `@src/runtime/webcore/fetch/compress_body.rs`:
- Around line 172-179: The CompressorState struct holds a libdeflate compressor
handle allocated at line 172 but has no Drop implementation, causing a memory
leak when the TLS-stored state is dropped. Add a Drop implementation for the
CompressorState struct that properly releases the libdeflate compressor handle
by calling the appropriate deallocation function from bun_libdeflate_sys when
the struct is dropped, following the RAII pattern of pairing acquisition with
release at the acquisition site.
🪄 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: 8aceeb66-ba2e-43a0-b151-6e15baac33b0
📒 Files selected for processing (3)
src/runtime/webcore/fetch.rssrc/runtime/webcore/fetch/compress_body.rstest/js/web/fetch/fetch-compress.test.ts
Compression now runs in HTTPClient::start() on the HTTP thread, reusing HttpThread.lazy_libdeflater (LibdeflateState gains a lazy compressor handle and the existing 512 KiB shared_buffer is reused for the fast path). - bun_http::compress_body: CompressEncoding/CompressOption + compress_into() taking &mut LibdeflateState; returns bun_core::Error on encoder failure. - HTTPClient: new compress + compressed_request_body fields. start() compresses Bytes bodies before InternalState::init; compress is .take()n so retry/h2-retry re-entries don't double-compress. The compressed Vec is freed in on_async_http_callback_raw alongside redirect/prev_redirect (the threadlocal clone is dealloc'd without Drop, so clone-owned state is torn down explicitly). - async_http::Options.compress threaded through AsyncHTTP::init. - runtime/webcore/fetch/compress_body.rs is now just from_js() re-exporting the bun_http types; the JS-thread thread_local! CompressorState is gone. - fetch_impl no longer compresses; it appends Content-Encoding and forwards the option via FetchOptions. Content-Length was already computed on the HTTP thread in build_request from original_request_body.len().
… send Common case (h1, compressed bound <= 512 KiB, body fits in one socket write) now allocates no per-request Vec: - Compression moves from HTTPClient::start() to write time so the output can borrow LibdeflateState::shared_buffer for the synchronous send. - state.original_request_body stays as the original uncompressed slice; only state.request_body (the cursor) is re-seated to compressed bytes. Redirects (307/308) and h2/idempotent retries re-read the original and re-compress on the next hop. New state.flags.body_compressed gates per-attempt re-entry. - Content-Length comes from HTTPClient::body_len_for_send() (compressed_body_len when set, else original_request_body.len()); h1/h2/h3/proxy build_request call sites updated. - h1: send_initial_request_payload compresses into shared_buffer, writes, then spill_compressed_body() copies any unsent tail into compressed_request_body before yielding to the event loop. Covers both the amount==0 early return and the normal return. - h2/h3/proxy-tunnel: compress_body_for_send(false) writes straight into the Vec (their body sends span event-loop ticks). - gzip/deflate slow path (bound > 512 KiB) now uses streaming zlib deflate(Z_FINISH) into a Vec growing in 64 KiB steps, instead of libdeflate one-shot which would prealloc the worst-case bound. zlib level clamped to 9. - Tests: incompressible 600 KiB body (forces zlib streaming + spill); 307 redirect with compressed body (re-compress from original on second hop).
…ped test - h2: withH2Server echo that gunzips the received body; small (32 KiB, shared-buffer fast path) and large (600 KiB, zlib-streaming spill) variants; asserts content-encoding, content-length == compressed length, round-trip. - h3: new /raw-echo route; same small/large variants. - h1: 600 KiB compress:"deflate" to verify the zlib-streaming slow path emits a zlib-wrapped (RFC 1950) stream that inflateSync decodes.
…r_exit
Mirrors the clone-only teardown in on_async_http_callback_raw so an in-flight
fetch({compress}) at shutdown_for_exit() doesn't leak its compressed Vec under
LSan.
…AFETY - compress_zlib_streaming: avail_in is c_uint; feed input in <=u32::MAX chunks with NoFlush until the tail, then Finish, so a >=4 GiB body isn't silently truncated to its low 32 bits. avail_out also clamped via try_from. - Move the SAFETY comment inside the deflateEnd scopeguard closure (-D clippy::undocumented-unsafe-blocks).
Collaborator
|
The two leak tests added here have been flaking on the macOS arm64 Tart runners (deltaMB 25-43 vs the 32 threshold) and went red in build 71937; loosened in #33988. |
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
2 participants
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.
Adds a
compressoption tofetch()that compresses the request body before sending and sets theContent-Encodingrequest header.true→"gzip"ArrayBuffer/TypedArray,Blob) are compressed;ReadableStreamand sendfile bodies are sent as-is.Content-Encodingheader, the body is empty, or the URL iss3://.Content-Lengthautomatically reflects the compressed size.How did you verify your code works?
test/js/web/fetch/fetch-compress.test.ts— 24 tests covering all four encodings × {string, Uint8Array, Blob},compress: true/false,{encoding, level}, explicitContent-Encodingskip,ReadableStreamskip, empty body, >512 KiB body (slow path), and validation errors. Verified failing onUSE_SYSTEM_BUN=1.Implementation
Mirrors the response-decompression fast path in
bun_http::InternalState: a thread-local libdeflateCompressor+ 512 KiB shared scratch buffer (bun_core::boxed_zeroed, same shape asLibdeflateStateinHTTPThread.rs) reused across calls. gzip/deflate go through libdeflate; brotli/zstd use their one-shot encoders into the same buffer. HTTP"deflate"is encoded as zlib-wrapped DEFLATE per RFC 9110 §8.4.1.2.