Skip to content

Bun.serve: serve the fresh file, not a 206, when Bun.file's cached size is stale - #33657

Closed
robobun wants to merge 1 commit into
mainfrom
farm/130c8de7/fix-stale-size-206
Closed

robobun wants to merge 1 commit into
mainfrom
farm/130c8de7/fix-stale-size-206

Conversation

@robobun

@robobun robobun commented Jul 7, 2026 •

Copy link
Copy Markdown
Collaborator

Repro

import * as fs from "node:fs";
const p = "/tmp/asset.bin";
using srv = Bun.serve({
  port: 0,
  async fetch() {
    fs.writeFileSync(p, Buffer.alloc(1000, 65));
    const f = Bun.file(p);
    void f.size;                                   // caches st_size=1000 on the blob and its store
    fs.appendFileSync(p, Buffer.alloc(4000, 66));  // file is now 5000 bytes
    return new Response(f);
  },
});
const res = await fetch(srv.url);                  // client sends NO Range header
console.log(res.status, res.headers.get("content-range"), (await res.arrayBuffer()).byteLength);

On 1.4.0 and main:

206 "bytes 0-999/*" 1000        // unsolicited 206, body truncated to the stale length

Adding any init header (new Response(f, { headers: { "x-a": "b" } })) flips the status to 200 but the body is still truncated to 1000.

RFC 9110 §15.3.7: a 206 is only ever the answer to a Range request. An unsolicited 206 poisons caches, and the silent truncation is data loss.

Cause

do_sendfile reopens the file and takes a fresh fstat, then decides whether the blob is a sub-range by comparing the blob's cached size against that fresh stat_size. When .size/.exists() cached the size at 1000 and the file has since grown to 5000, size != stat_size and the blob is treated as a slice: needs_content_range is set, sendfile.remain is capped at the stale 1000, and render_metadata promotes the headerless path to 206.

The is_whole_file check for native Range handling had the same defect: it compared against stat_size, so a stale-cached whole-file blob was excluded and an incoming Range header was ignored.

Fix

A whole-file blob's size is either the unset sentinel or the value resolve_size cached on the store (file.max_size); a .slice() writes size directly and does not touch the store cache. is_whole_file now compares against file.max_size (captured before the store borrow ends) instead of the fresh stat_size, and is computed once up front. A whole-file blob then sends stat_size bytes (never the stale cache), never sets needs_content_range, and is eligible for native Range handling, which resolves the client's Range against the fresh size.

Shrink (file smaller than the cached size) already behaved correctly and is unchanged.

.slice() behavior is unchanged except for one state that is bit-identical to an unsliced blob: f.slice(0, f.size) after .size was read (offset 0, size == store.max_size). That is served as the whole fresh file instead of an unsolicited 206 at the stale length; the Blob struct has no field that distinguishes it from an unsliced blob.

Not addressed here (pre-existing, same bug class): when the path did not exist at .size/.exists() time and is created afterward, resolve_size falls through to size = 0 with store.max_size still MAX_SIZE, and that state is indistinguishable from .slice(0, 0) on a never-stat'd file. Fixing it without regressing the explicit empty slice needs a dedicated sliced bit on Blob, which touches the #[repr(C)] layout shared with C++ and the structured-clone format. Follow-up.

Verification

New describe("Bun.file response after the file grew on disk") in test/js/bun/http/bun-serve-file.test.ts:

  • after .size / await .exists() / nothing, with and without init headers: 200, Content-Length: 5000, no Content-Range, full body
  • Range: bytes=0-1999 resolves against the fresh size: 206, bytes 0-1999/5000
  • Range: bytes=2000-2999 (past the stale cached size) is satisfiable against the fresh size

6 of the 8 fail without the fix, all pass with it. The rest of bun-serve-file.test.ts and the serve.test.ts Content-Range/Bun.file suites are unchanged.

Related

#32794 removes the needs_content_range setter entirely for the unsolicited-206 side of this, but still caps sendfile.remain at the stale original_size so the body remains truncated. This PR is narrower and fixes both the 206 and the truncation without changing the documented slice behavior #32794 is pending a maintainer call on.


no test proof · iteration 5 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/http/bun-serve-file.test.ts

@github-actions github-actions Bot added the claude label Jul 7, 2026
@coderabbitai

coderabbitai Bot commented Jul 7, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 9 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4391eeb9-cb2f-46b2-8b86-fedf7030ba68

📥 Commits

Reviewing files that changed from the base of the PR and between 2de114f and fb0ecfe.

📒 Files selected for processing (2)
  • src/runtime/server/RequestContext.rs
  • test/js/bun/http/bun-serve-file.test.ts

Walkthrough

Changes

File growth sendfile handling

Layer / File(s) Summary
File boundary detection
src/runtime/server/RequestContext.rs
Sendfile setup captures the store maximum size and determines whole-file status from blob offsets and original size boundaries.
Sendfile and range planning
src/runtime/server/RequestContext.rs
Remaining bytes and content-range handling now distinguish fresh whole-file responses from slices while reusing the shared whole-file decision.
Grown-file regression coverage
test/js/bun/http/bun-serve-file.test.ts
Tests cover file growth after cached metadata access, full responses, preserved headers, and ranges evaluated against the fresh file size.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly describes the main behavior change: serving fresh files instead of an unsolicited 206 when the cached size is stale.
Description check ✅ Passed The description covers what changed and how it was verified, even though it uses different headings than the template.

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Jul 7, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 12:12 AM PT - Jul 11th, 2026

❌ @robobun, your commit fb0ecfe has 2 failures in Build #71814 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 33657

That installs a local version of the PR into your bun-33657 executable, so you can run:

bun-33657 --bun

@robobun

robobun commented Jul 7, 2026 •

Copy link
Copy Markdown
Collaborator Author

CI for fb0ecfe (build 71814): bun-serve-file.test.ts including the 8 new tests passed on every lane. The two red jobs are unrelated to this change and have been handed off as main breaks:

  • test/cli/update_interactive_install.test.ts (Windows x64, bun update --interactive install exit 1)
  • test/js/node/test/parallel/test-worker-message-port-transfer-terminate.js (x64-asan, JSC assertion in JSObject::getOwnPropertyDescriptor)

Review threads resolved. Ready for review.

@robobun
robobun force-pushed the farm/130c8de7/fix-stale-size-206 branch from 090e7a6 to 0e63f34 Compare July 11, 2026 02:58
Comment thread src/runtime/server/RequestContext.rs
@robobun
robobun force-pushed the farm/130c8de7/fix-stale-size-206 branch from 0e63f34 to 2de114f Compare July 11, 2026 03:21

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 `@test/js/bun/http/bun-serve-file.test.ts`:
- Line 1142: Update the “Bun.file response after the file grew on disk” suite to
use describe.concurrent instead of describe, since each test’s run() invocation
has isolated tempDir and server state.
🪄 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: 33caa0e2-2a05-4dfe-a26c-ce86899a5a4e

📥 Commits

Reviewing files that changed from the base of the PR and between 18cfc1a and 2de114f.

📒 Files selected for processing (2)
  • src/runtime/server/RequestContext.rs
  • test/js/bun/http/bun-serve-file.test.ts

Comment thread test/js/bun/http/bun-serve-file.test.ts Outdated
…ze is stale

When a handler reads .size (or awaits .exists()) on a Bun.file and the
underlying file then grows before the Response is sent, do_sendfile
compared the blob's cached size against the fresh fstat and concluded
the blob was a sub-range. That produced an unsolicited 206 Partial
Content with Content-Range: bytes 0-(stale-1)/* and a body truncated to
the stale length, for a request that sent no Range header.

The whole-file check now compares the blob's size against the size
cached on the store by resolve_size (file.max_size) rather than the
fresh stat, so a stale cached size is recognised as a whole-file blob.
Whole-file blobs are served at the fresh stat'd length with status 200,
and incoming Range headers are resolved against that length.
@robobun
robobun force-pushed the farm/130c8de7/fix-stale-size-206 branch from 2de114f to fb0ecfe Compare July 11, 2026 03:26
@robobun

robobun commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator Author

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-11, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.

@robobun robobun closed this Sep 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant