Bun.serve: honor If-Range on Bun.file Range requests by robobun · Pull Request #33548 · oven-sh/bun

robobun · 2026-07-06T20:12:04Z

What

Bun.serve serves Bun.file responses and supports Range requests (206 Partial Content), but neither file-serving path evaluated the If-Range precondition. Per RFC 9110 §13.1.5, If-Range exists to make download resumption safe: if the client's validator no longer matches the current representation, the server MUST ignore Range and return the full 200 body.

Because file routes automatically advertise Last-Modified on every 200/206, clients are invited into conditional resume, and the condition was then never checked. A client resuming a download with a stale validator got a 206 containing the new file's bytes, so the assembled file was a silent mix of two versions.

Two paths were affected:

static routes (routes: { "/f": Bun.file(p) }, FileRoute), which advertise Last-Modified
the fetch handler returning new Response(Bun.file(p), { headers }), which serves via the sendfile path in RequestContext

Reproduction

import * as fs from "node:fs";
const P = "/tmp/if-range-asset.bin";
const wr = (b, t) => { fs.writeFileSync(P, Buffer.alloc(400, b)); fs.utimesSync(P, new Date(t), new Date(t)); };
wr("A", "2020-01-02T03:04:05Z");
const srv = Bun.serve({
  port: 0, hostname: "127.0.0.1",
  routes: { "/f": new Response(Bun.file(P)) },
  fetch: () => new Response(Bun.file(P), { headers: { etag: '"etag-A"' } }),
});
const U = srv.url.href;
const lmA = (await fetch(U + "f")).headers.get("last-modified"); // validator Bun served for A
wr("B", "2021-06-07T08:09:10Z");                                 // file changes on disk
const r = await fetch(U + "f", { headers: { Range: "bytes=200-", "If-Range": lmA } });
console.log(r.status, r.headers.get("content-range"));           // before: 206 "bytes 200-399/400" (RFC: 200)
srv.stop(true);

Fix

Evaluate If-Range against the response's own current validator before honoring the Range:

strong entity-tag comparison (octet-by-octet), or
exact Last-Modified match (second precision, matching the existing If-Modified-Since comparison)

A weak If-Range tag (W/"..."), a mismatch, a missing validator of the requested type, or an unparsable value all fall back to the full 200 response. The shared decision lives in RangeRequest::if_range_allows_range; FileRoute evaluates it synchronously, and RequestContext captures the raw header at construction (the uWS request is gone by the time the response validator is known) and evaluates it in do_sendfile.

Verification

test/js/bun/http/bun-serve-file.test.ts gains an If-Range block covering both serving paths: stale Last-Modified on a FileRoute, and matching/mismatched/weak ETag on the fetch handler. The mismatch cases fail on current main (return 206) and pass with this change; the full file (74 tests) passes.

no test proof · iteration 3 · 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

coderabbitai · 2026-07-06T20:12:12Z

Warning

Review limit reached

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

Next review available in: 1 minute

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: ecdc5ff5-2276-47f3-8057-c5296971480b

📥 Commits

Reviewing files that changed from the base of the PR and between 2e2230a and 43a911f.

📒 Files selected for processing (4)

src/runtime/server/FileRoute.rs
src/runtime/server/RangeRequest.rs
src/runtime/server/RequestContext.rs
test/js/bun/http/bun-serve-file.test.ts

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

robobun · 2026-07-06T20:14:29Z

^{Updated 6:09 AM PT - Jul 12th, 2026}

✅ @robobun, your commit 43a911f1589d88350c03085c79e722e0ca545f43 passed in Build #72188! 🎉

🧪 To try this PR locally:

bunx bun-pr 33548

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

bun-33548 --bun

robobun · 2026-07-06T20:32:20Z

Good catch on the contract inconsistency. Flipped the empty-after-trim case to return false in 14cb916, so it now fails closed to the full 200 body like every other unparsable branch. As you noted, this is only reachable via a whitespace-only If-Range (uWS maps a zero-length header to absent), so no real-client behavior changes.

claude

Thanks for addressing the empty-after-trim case. I didn't find any further issues, but this adds a field to RequestContext and changes hot-path Range handling across both file-serving paths, so it's worth a maintainer's eyes.

Extended reasoning...

Overview

This PR implements RFC 9110 §13.1.5 If-Range precondition handling for Bun.serve file responses. It adds a shared if_range_allows_range() helper in RangeRequest.rs, wires it into FileRoute::on (static routes), and into RequestContext::do_sendfile (fetch-handler path) via a new if_range: Option<Box<[u8]>> field captured at request construction. A new describe("If-Range") block in bun-serve-file.test.ts covers stale Last-Modified on a FileRoute and matching/mismatched/weak ETag on the fetch handler.

My earlier inline comment (whitespace-only If-Range failing open) was addressed in 14cb916 — that branch now returns false, consistent with the doc comment and every other unparsable case.

Security risks

The new code parses a client-controlled header, but all operations are bounds-safe byte-slice primitives (trim, first(), starts_with, ==, parse_http_date) and every malformed/weak/missing case fails closed to a full 200 body. No allocation is sized from untrusted input beyond copying the header itself. I don't see injection, OOB, or DoS surface here.

Level of scrutiny

Medium-high. This is production HTTP server hot-path code: it adds a field to the per-request RequestContext struct (with matching init and reset), introduces a per-request header lookup + optional heap allocation, and alters Range/206 semantics on both file-serving paths. The logic follows the existing If-Modified-Since pattern closely (same second-precision comparison, same last_modified_date() error handling via scopeguard early-return in FileRoute), which is reassuring, but changes to RequestContext lifecycle and Range gating are exactly the kind of thing a maintainer familiar with the uWS integration should sign off on.

Other factors

Single RequestContext construction site sets if_range alongside the existing range field, and the reset path nulls it — I didn't find other init paths that would leave it stale.
if_range_permits_partial clones the Option<Box<[u8]>> (an extra small allocation on the rare If-Range path); minor, not a blocker.
Tests look solid and follow harness conventions (tempDir, await using, port 0, exact-value assertions on status/content-range/body bytes).
No CODEOWNERS entry for these paths.

claude

Thanks for addressing the whitespace-only case — the helper now fails closed as documented. I didn't find further issues, but since this adds a new owned field and per-request allocation to RequestContext and wires new logic into do_sendfile, it's worth a human pass on the hot-path/lifecycle side.

Extended reasoning...

Overview

This PR implements RFC 9110 §13.1.5 If-Range precondition handling for Bun.serve file responses, across both serving paths: static FileRoute and the RequestContext::do_sendfile path used by fetch handlers returning Bun.file. It adds a shared RangeRequest::if_range_allows_range helper, a new if_range: Option<Box<[u8]>> field on RequestContext (captured at construction because the uWS request buffer is gone by response time), and a guard in do_sendfile that suppresses the Range → 206 path when the client's validator no longer matches. Tests cover matching/stale Last-Modified on a FileRoute and matching/mismatched/weak ETag on the fetch-handler path.

Since my previous inline comment, the author flipped the whitespace-only-header branch in if_range_allows_range from true to false (commit 14cb916), so it now fails closed consistently with the doc comment and every other malformed branch. That thread is resolved.

Security risks

Low. The change only narrows when a 206 partial is served (falling back to a full 200), which is the safe direction — a mismatch means more bytes sent, never fewer or wrong ones. The header value is copied into an owned Box<[u8]> and only compared byte-for-byte or fed to the existing parse_http_date helper; there's no new parsing of untrusted structure beyond what If-Modified-Since already exercises. No auth, crypto, or filesystem-path handling is touched.

Level of scrutiny

Moderate. The RFC logic itself is small and well-contained, and the fail-safe direction is correct throughout. What warrants human eyes is the RequestContext change: this is the core per-request struct for Bun.serve, and the PR adds a new field, a per-request heap allocation (only when the header is present), a reset in the deinit/reuse path, and a new method (if_range_permits_partial) that reaches into the response's FetchHeaders via get_init_headers_mut / to_slice_clone during do_sendfile. A maintainer should confirm the field is reset on every recycle path, that the extra allocation is acceptable on the request hot path, and that the &mut self header access in if_range_permits_partial is sound at that call site.

Other factors

The FileRoute side mirrors the existing If-Modified-Since handling closely (same last_modified_date() call, same second-precision comparison, same early-return on JS exception under the existing fd_guard), which gives good confidence there. Test coverage is reasonable for the happy paths but doesn't exercise the RequestContext Last-Modified branch or the reset/recycle path — not blocking, but another reason a human glance at the RequestContext lifecycle is worthwhile.

robobun · 2026-07-06T23:05:15Z

Closed the coverage gap you flagged: abba999 adds fetch-handler If-Range cases with a user-set Last-Modified (matching → 206 exercises the parse_http_date branch in RequestContext::if_range_permits_partial, stale → 200 the mismatch path). The field reset lives in RequestContext::deinit, which is the single recycle path back into the pool.

robobun · 2026-07-07T00:40:17Z

Status: the diff is ready. Local verification is green (bun bd test test/js/bun/http/bun-serve-file.test.ts passes all If-Range cases plus the full file's 74 tests; fail-before on the baked bun returns 206 for the mismatch cases, confirming the tests exercise the fix).

The two most recent CI reds are unrelated flake on lanes the change doesn't touch, and they differ each run:

build 69319: darwin aarch64 test-bun failed on a buildkite artifact-download timeout (infra), plus Windows solc/spawn-pipe-leak (RSS threshold) flakes.
build 69430: Windows x64-baseline napi > napi_wrap > has the right lifetime ("Condition was not met after 100 GC attempts", a GC-timing flake), plus a flaky-tagged annotation for fetch-tls-abortsignal-timeout, update_interactive_install, and bun-server.

No failure references FileRoute.rs, RangeRequest.rs, RequestContext.rs, or bun-serve-file.test.ts. Ready for a maintainer to re-run the flaky lanes or merge.

Both Bun.file serving paths (static FileRoute and the fetch-handler sendfile path) applied an incoming Range header unconditionally, ignoring any If-Range precondition. A resuming client that sent a now-stale validator received a 206 carrying the new file's bytes, so the assembled download was a silent mix of two versions (RFC 9110 13.1.5). Evaluate If-Range against the response's own current validator before honoring the Range: strong entity-tag comparison, or an exact Last-Modified match. A weak tag, a mismatch, a missing validator, or an unparsable value falls back to a full 200 response.

…body)

robobun · 2026-09-13T19:08:24Z

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-12 and it conflicts with main. 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.

…es (#43571) ### Problem - `Bun.serve` ignores the `If-Range` request header. A `Range` request whose validator does not match the response still gets `206 Partial Content`. A client that resumes a download after the file changed joins bytes of two versions. - All three Range paths are affected: `FileRoute::serve` (`FileRoute.rs:371`), `DirectoryRoute::on` (`DirectoryRoute.rs:161`) and `RequestContext::do_sendfile` (`RequestContext.rs:1909`). ### Fix - `RangeRequest::from_request` takes the `ETag` and `Last-Modified` of the response. If `If-Range` does not match them, it returns no range, and the route sends the full `200` body. File routes and `{ dir }` routes use it. - The handler path keeps a copy of `If-Range`. `do_sendfile` compares it with the `ETag` and `Last-Modified` the handler set on its Response. Without them, the response is the full body. - Correct per RFC 9110 §13.1.5: an entity-tag uses the strong comparison, and an HTTP-date must name GMT and equal `Last-Modified`. - Verified: `test/js/bun/http/bun-serve-file.test.ts` (45 new cases, 33 fail on 1.4.3) and `test/js/bun/http/serve-directory-routes.test.ts`. Also `bun-serve-static`, `serve-if-none-match`, `serve-http2`, `serve-http3`. Self-reviewed: 2 concerns raised, 2 addressed. ### Background - `If-Range` carries the validator (an ETag or a Last-Modified date) of the partial copy that the client has. It means: send the range only if the file is still that version, otherwise send all of it. - The strong comparison needs two byte-equal tags that are not weak (no `W/` prefix). The ETag of a `{ dir }` route is weak, so only the date form can match there. - `do_sendfile` serves a handler's `new Response(Bun.file(p))`. It can run after the uWS request object is gone, so the context copies the header. <details><summary>Notes</summary> Repro on 1.4.3. `/dir/*` is a `{ dir }` route, `/file` is a file route, and the `fetch` handler returns `new Response(Bun.file(p))`. Each request sends `Range: bytes=0-3` and `If-Range: "does-not-match"`: ``` /dir/big.bin 206 bytes 0-3/100000 (with this PR: 200, full body) /file 206 bytes 0-3/100000 (with this PR: 200, full body) /fetch-handler 206 bytes 0-3/100000 (with this PR: 200, full body) ``` - #33548 made the same fix for the file route and the handler path. It was closed as stale because it conflicted with `main`, not because of the fix. This PR also covers `{ dir }` routes. - #41502 (open) replaces `jsc_hooks::parse_http_date` with a strict HTTP-date parser for `If-Modified-Since` and `If-Unmodified-Since`. The `If-Range` date uses `parse_http_date`, like the other date preconditions on `main`. The PR that lands second should switch it. - A failed `If-Range` also turns off `416`, because the server ignores the `Range` header as a whole. - `If-Range` without a usable `Range` (absent, multi-range, malformed) has no effect, and the request context does not copy it. - A weak `If-Range` tag never matches, as in Go `net/http`. A client that sends the weak ETag of a `{ dir }` route gets the full file. That is correct, but it is not a resume. nginx compares the bytes of the two tags, and npm `send` does a substring match. The docs now say that the date is the validator to resume with on a `{ dir }` route. - The date must end in `GMT`. `parse_http_date` is `Date.parse`, which reads a date with no zone (the obsolete asctime form) in the server's local time. Such a date never matches, so the result does not depend on the server's time zone. The IMF-fixdate and rfc850 forms match. The strict parser of #41502 can accept asctime as GMT. - The date comparison is exact to the second, as in nginx and Go `net/http`. A file that is rewritten twice in one second keeps the same `Last-Modified`, and the server cannot see that. RFC 9110 §8.8.2.2 gives that check to the client: it may use a `Last-Modified` as a strong validator only when the response's `Date` is later than it. - HEAD on the handler path does not evaluate `Range` on `main` (it does not reach `do_sendfile`). The HEAD cases cover the file route and the `{ dir }` route. - Cost for a request without `Range`: one branch in `RequestContext::create`, and one more header lookup in `FileRoute::serve`. The copy happens only when `Range` parses and `If-Range` is present. - The self-review raised two points, and the second commit addresses both. No test responded after an async hop, which is the case the header copy exists for. The `it.each` names did not say what the `If-Range` value was. - A manual check over HTTP/2 (`node:http2` client, `http2: true` server) gives the same results as HTTP/1.1 on all three paths. - Suites run with the debug build: `bun-serve-file`, `serve-directory-routes`, `bun-serve-static`, `serve-if-none-match`, `serve-http2`, `serve-http3`, `regression/issue/29181`, the `Content-Range` block of `serve.test.ts`, and `test/internal/source-lints/`. - The three `pollable Bun.file(...)` tests in `bun-serve-file.test.ts` take 2.6 to 6.5 s under the debug ASAN build in my container, so they can exceed the default 5 s timeout when the host is busy. Five interleaved runs of the base binary and the fixed binary show the same times for both. </details>  --- **no test proof** · iteration 3 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/http/serve-directory-routes.test.ts, test/js/bun/http/bun-serve-file.test.ts

github-actions Bot added the claude label Jul 6, 2026

claude Bot reviewed Jul 6, 2026

View reviewed changes

Comment thread src/runtime/server/RangeRequest.rs Outdated

claude Bot reviewed Jul 6, 2026

View reviewed changes

robobun and others added 5 commits July 12, 2026 10:25

[autofix.ci] apply automated fixes

62b972d

If-Range: treat a whitespace-only header as unparsable (fail to full …

bf694ae

…body)

ci: retrigger

656c2a5

test: cover the fetch-handler If-Range Last-Modified branch

43a911f

robobun force-pushed the farm/1a77fc01/if-range-file-serve branch from abba999 to 43a911f Compare July 12, 2026 10:28

This was referenced Sep 6, 2026

Parse If-Modified-Since and If-Unmodified-Since as RFC 9110 HTTP-date #41502

Open

Bun.serve: read Content-Type and Range through req.headers; give stream-body blob() its type #41922

Closed

robobun closed this Sep 13, 2026

robobun mentioned this pull request Sep 19, 2026

Bun.serve: honor If-Range on file, directory and handler file responses #43571

Merged

Navigation Menu

Bun.serve: honor If-Range on Bun.file Range requests - #33548

Bun.serve: honor If-Range on Bun.file Range requests#33548
robobun wants to merge 5 commits into
mainfrom
farm/1a77fc01/if-range-file-serve

robobun commented Jul 6, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 6, 2026 •

edited

Loading

Review limit reached

Uh oh!

robobun commented Jul 6, 2026 •

edited

Loading

Uh oh!

Uh oh!

robobun commented Jul 6, 2026

Uh oh!

claude Bot left a comment

Uh oh!

claude Bot left a comment

Uh oh!

robobun commented Jul 6, 2026

Uh oh!

robobun commented Jul 7, 2026

Uh oh!

robobun commented Sep 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

robobun commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Reproduction

Fix

Verification

Uh oh!

coderabbitai Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

robobun commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

robobun commented Jul 6, 2026

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

robobun commented Jul 6, 2026

Uh oh!

robobun commented Jul 7, 2026

Uh oh!

robobun commented Sep 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

robobun commented Jul 6, 2026 •

edited

Loading

coderabbitai Bot commented Jul 6, 2026 •

edited

Loading

robobun commented Jul 6, 2026 •

edited

Loading