fetch: send the slice's Content-Length when uploading Bun.file().slice() via sendfile - #36862
Conversation
The sendfile fast path (plain http, no proxy/compress, file >= 32 KiB) computed content_size as the whole file's stat size, ignoring the slice window. remain was then clamped to the slice length, so the right bytes went on the wire but Content-Length advertised the full file, and the origin waited for bytes that never came. Set content_size to the clamped remain for regular files; that is exactly what sendfile will write.
|
Status: fix verified locally; waiting on CI. Reproduced with: USE_SYSTEM_BUN=1 bun test test/js/bun/http/fetch-file-upload.test.ts -t slice # 4 timeouts
bun bd test test/js/bun/http/fetch-file-upload.test.ts # 11 pass |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughChangesSendfile slice uploads
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — the sendfile content_size now derives from the same clamped remain that governs how many bytes are actually written, so Content-Length matches the slice window.
What was reviewed
- Traced whole-file (
offset=0,size=MAX_SIZE=2^52-1): after the ISREG clampremain=stat_size, socontent_size=stat_size— unchanged from before. - Non-regular-file branch (Linux only; macOS bails earlier): initial
content_size = original_size.min(stat_size)is byte-identical to the removedelsearm. - Tests bracket the 32 KiB threshold (32767 exercises the read-file fallback as a control), assert exact Content-Length + received byte count + first/last byte, and cover the open-ended
slice(10)case.
Extended reasoning...
Overview
Two-file change: src/runtime/webcore/fetch.rs (net −2 lines in the sendfile setup block) and new tests in test/js/bun/http/fetch-file-upload.test.ts. The bug was that for regular files ≥32 KiB, content_size was set to stat.st_size (whole file) while remain was independently clamped to the slice window — so sendfile(2) sent the right bytes but the advertised Content-Length was the whole file, hanging the origin.
The fix drops the redundant blob_size computation and, inside the existing ISREG clamp block, sets sf.content_size = sf.remain after remain has been clamped to [offset, min(offset+size, stat_size)) - offset. That is by construction the exact byte count sendfile will write.
Behavior preservation
I traced three cases against the old code:
- Whole file (
offset=0,size=MAX_SIZEwhereMAX_SIZE=(1<<52)-1): newremainclamps tostat_size, socontent_size=stat_size— identical to the oldISREG → stat_sizebranch. The existing whole-file sendfile roundtrip test in the same file covers this. - Non-regular file (Linux only — macOS breaks out of
'use_sendfilefor non-regular, Windows never enters viais_eligible): the ISREG block is skipped, socontent_sizekeeps its initial valueoriginal_size.min(stat_size), which is exactly the oldelsearm. No change. - Sliced regular file:
content_sizenow equals the clamped slice length instead ofstat_size. This is the fix.
No overflow concern from (blob_offset + original_size): MAX_SIZE is 2^52-1, so adding a u64 offset stays well within u64/usize on 64-bit — and that line is unchanged by this PR anyway.
Security risks
None. This is a header-value correctness fix in the outbound HTTP client; no untrusted-input parsing, no auth/crypto/permissions surface.
Level of scrutiny
Low-to-moderate. The production change is a 3-line rearrangement inside a single well-understood block, with no new control flow or types. The sendfile path is already gated by is_eligible (POSIX + plain http + no proxy + no compress) and the ≥32 KiB check.
Other factors
Tests follow harness conventions (tempDir, port: 0, await using, test.concurrent, exact-value assertions on a combined object). The 32767-byte case is a useful control that confirms the read-file fallback path was already correct. PR description confirms USE_SYSTEM_BUN=1 reproduces 4 timeouts and the debug build passes all 11. No prior reviews or outstanding comments.
…e() via sendfile (oven-sh#36862) Found by the outbound-request-body fuzzer (ledger oven-sh#11440). ### Repro ```js // file is >= 32 KiB so the sendfile fast path is taken require("fs").writeFileSync("/tmp/f.bin", Buffer.alloc(65536)); using server = Bun.serve({ port: 0, async fetch(req) { console.log("CL", req.headers.get("content-length")); await req.arrayBuffer(); return new Response("ok"); } }); await fetch(server.url, { method: "POST", body: Bun.file("/tmp/f.bin").slice(10, 110) }); ``` ``` CL 65536 <- should be 100 (hangs: fetch() never settles) ``` The 100 slice bytes arrive correctly (right offset, right count); only the `Content-Length` header is wrong, so the origin waits for 65436 more bytes that never come. Cliff is exactly at a 32 KiB backing file; slice size is irrelevant. ### Cause `src/runtime/webcore/fetch.rs`'s sendfile setup computed `content_size` as the whole file's `stat.st_size` for regular files, discarding the slice's own size: ```rust let blob_size = if bun_sys::S::ISREG(stat.st_mode as u32) { stat_size // <- ignores the slice window } else { original_size.min(stat_size) }; ``` `remain` was then separately clamped to the slice window, so `sendfile(2)` wrote the right bytes while `HTTPRequestBody::Sendfile(sf).len()` (= `sf.content_size`) produced the wrong `Content-Length`. ### Fix After the existing `remain` clamp for regular files, set `content_size = remain`; that is exactly the byte count `sendfile` will write. The now-redundant `blob_size` branch is dropped. ### Verification `test/js/bun/http/fetch-file-upload.test.ts` gains a `describe` covering slice uploads across the 32 KiB boundary (32767 / 32768 / 64 KiB / 1 MiB files) plus an open-ended `slice(10)`. All four boundary cases and the open-ended slice time out on `main` and pass with this change. The existing whole-file sendfile roundtrip test in the same file continues to pass. Related: oven-sh#32794 fixes the same bug class on the `Bun.serve` response side; this is the `fetch()` client upload side. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/http/fetch-file-upload.test.ts <!-- robobun:evidence:end -->
2 participants
Found by the outbound-request-body fuzzer (ledger #11440).
Repro
The 100 slice bytes arrive correctly (right offset, right count); only the
Content-Lengthheader is wrong, so the origin waits for 65436 more bytes that never come. Cliff is exactly at a 32 KiB backing file; slice size is irrelevant.Cause
src/runtime/webcore/fetch.rs's sendfile setup computedcontent_sizeas the whole file'sstat.st_sizefor regular files, discarding the slice's own size:remainwas then separately clamped to the slice window, sosendfile(2)wrote the right bytes whileHTTPRequestBody::Sendfile(sf).len()(=sf.content_size) produced the wrongContent-Length.Fix
After the existing
remainclamp for regular files, setcontent_size = remain; that is exactly the byte countsendfilewill write. The now-redundantblob_sizebranch is dropped.Verification
test/js/bun/http/fetch-file-upload.test.tsgains adescribecovering slice uploads across the 32 KiB boundary (32767 / 32768 / 64 KiB / 1 MiB files) plus an open-endedslice(10). All four boundary cases and the open-ended slice time out onmainand pass with this change. The existing whole-file sendfile roundtrip test in the same file continues to pass.Related: #32794 fixes the same bug class on the
Bun.serveresponse side; this is thefetch()client upload side.no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/http/fetch-file-upload.test.ts