Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 25 additions & 15 deletions src/runtime/server/RequestContext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1746,6 +1746,7 @@ where
let crate::webcore::blob::store::Data::File(file) = &self.blob.store().unwrap().data else {
unreachable!("do_sendfile called with non-file blob");
};
let store_max_size = file.max_size;
let mut file_buf = PathBuffer::uninit();
let auto_close = !matches!(
file.pathlike,
Expand Down Expand Up @@ -1826,7 +1827,22 @@ where
AnyBlob::Blob(b) => b.size.get(),
_ => unreachable!(),
};
let blob_offset = match &self.blob {
AnyBlob::Blob(b) => b.offset.get(),
_ => unreachable!(),
};
let stat_size: BlobSizeType = BlobSizeType::try_from(stat.st_size.max(0)).unwrap();
// An unsliced blob has offset 0 and either the unset-size sentinel or,
// if JS touched `.size`, the value `resolve_size` cached on the store
// (`file.max_size`). Comparing against the fresh `stat_size` here is
// wrong: when the file grew after `.size` was read, a stale cached
// size would be mistaken for a slice bound and the response truncated
// with an unsolicited 206. `.slice(0, store_max_size)` is
// indistinguishable from an unsliced blob (same store, same size) and
// is served as the whole file; any other `.slice()` differs in offset
// or size.
let is_whole_file = blob_offset == 0
&& (original_size == crate::webcore::blob::MAX_SIZE || original_size == store_max_size);
Comment thread
robobun marked this conversation as resolved.
if let AnyBlob::Blob(b) = &mut self.blob {
b.size.set(if is_regular {
stat_size
Expand All @@ -1836,16 +1852,16 @@ where
}

self.flags.set_needs_content_length(true);
let blob_offset = match &self.blob {
AnyBlob::Blob(b) => b.offset.get(),
_ => unreachable!(),
};
self.sendfile = SendfileContext {
remain: blob_offset + original_size,
remain: if is_regular && is_whole_file {
stat_size
} else {
blob_offset + original_size
},
offset: blob_offset,
total: 0,
};
if is_regular && auto_close {
if is_regular && auto_close && !is_whole_file {
self.flags.set_needs_content_range(
self.sendfile.remain.saturating_sub(self.sendfile.offset) != stat_size,
);
Expand All @@ -1863,13 +1879,9 @@ where
// Honor an incoming Range: header for whole-file responses. We
// don't compose Range with a user-supplied .slice() because the
// Content-Range arithmetic gets ambiguous; the slice path keeps
// its existing slice-as-range behavior. `offset == 0` alone is
// insufficient — `Bun.file(p).slice(0, n)` has offset 0 — so we
// also check the size: an unsliced blob has either the unset-size
// sentinel or, if JS already read `.size`, the stat'd size; a
// `.slice(0, n)` blob has `n < stat_size`. Skip if the user
// already set Content-Range or a non-200 status — they're
// managing partial responses themselves.
// its existing slice-as-range behavior. Skip if the user already
// set Content-Range or a non-200 status — they're managing
// partial responses themselves.
let user_handles_range = if let Some(r) = self.response_weakref.get() {
r.status_code() != 200
|| r.get_init_headers_mut()
Expand All @@ -1878,8 +1890,6 @@ where
} else {
false
};
let is_whole_file = blob_offset == 0
&& (original_size == crate::webcore::blob::MAX_SIZE || original_size == stat_size);
// RFC 9110 §14.2: Range is only defined for GET (HEAD mirrors GET's headers).
let method_allows_range = self.method == Method::GET || self.method == Method::HEAD;
if is_regular
Expand Down
84 changes: 83 additions & 1 deletion test/js/bun/http/bun-serve-file.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { Server } from "bun";
import { afterAll, beforeAll, describe, expect, it, mock, test } from "bun:test";
import { bunEnv, bunExe, isASAN, isWindows, rmScope, tempDir, tempDirWithFiles } from "harness";
import { mkfifo } from "mkfifo";
import { unlinkSync } from "node:fs";
import { appendFileSync, unlinkSync, writeFileSync } from "node:fs";
import { join } from "node:path";

const LARGE_SIZE = 1024 * 1024 * 8;
Expand Down Expand Up @@ -1138,3 +1138,85 @@ console.log("OK");
},
60_000,
);

describe.concurrent("Bun.file response after the file grew on disk", () => {
// Touching `.size`/`.exists()` caches the stat result on the BunFile. If the
// file then grows before the Response is sent, the stale cached size must not
// be mistaken for a `.slice()` bound: the fresh stat taken at send time wins,
// and the whole file is served as a plain 200.
async function run(mode: string, init?: ResponseInit, reqInit?: RequestInit) {
using dir = tempDir("serve-file-grow", {});
const p = join(String(dir), "asset.bin");
await using server = Bun.serve({
port: 0,
async fetch() {
writeFileSync(p, Buffer.alloc(1000, 65));
const f = Bun.file(p);
if (mode === "size") void f.size;
if (mode === "exists") await f.exists();
appendFileSync(p, Buffer.alloc(4000, 66));
return new Response(f, init);
},
});
const res = await fetch(server.url, reqInit);
const body = new Uint8Array(await res.arrayBuffer());
return {
status: res.status,
contentRange: res.headers.get("content-range"),
contentLength: res.headers.get("content-length"),
bodyLength: body.byteLength,
body,
};
}

it.each(["none", "size", "exists"])(
"serves the fresh file as 200 after %s was touched (no init headers)",
async mode => {
const r = await run(mode);
expect({
status: r.status,
contentRange: r.contentRange,
contentLength: r.contentLength,
bodyLength: r.bodyLength,
}).toEqual({ status: 200, contentRange: null, contentLength: "5000", bodyLength: 5000 });
expect(r.body.subarray(0, 1000)).toEqual(Buffer.alloc(1000, 65));
expect(r.body.subarray(1000)).toEqual(Buffer.alloc(4000, 66));
},
);

it.each(["none", "size", "exists"])(
"serves the fresh file as 200 after %s was touched (with init headers)",
async mode => {
const r = await run(mode, { headers: { "x-a": "b" } });
expect({
status: r.status,
contentRange: r.contentRange,
contentLength: r.contentLength,
bodyLength: r.bodyLength,
}).toEqual({ status: 200, contentRange: null, contentLength: "5000", bodyLength: 5000 });
},
);

it("still resolves an incoming Range header against the fresh size", async () => {
const r = await run("size", undefined, { headers: { Range: "bytes=0-1999" } });
expect({
status: r.status,
contentRange: r.contentRange,
contentLength: r.contentLength,
bodyLength: r.bodyLength,
}).toEqual({ status: 206, contentRange: "bytes 0-1999/5000", contentLength: "2000", bodyLength: 2000 });
expect(r.body.subarray(0, 1000)).toEqual(Buffer.alloc(1000, 65));
expect(r.body.subarray(1000)).toEqual(Buffer.alloc(1000, 66));
});

it("a Range past the stale cached size is satisfiable against the fresh size", async () => {
const r = await run("size", undefined, { headers: { Range: "bytes=2000-2999" } });
expect({
status: r.status,
contentRange: r.contentRange,
contentLength: r.contentLength,
bodyLength: r.bodyLength,
}).toEqual({ status: 206, contentRange: "bytes 2000-2999/5000", contentLength: "1000", bodyLength: 1000 });
expect(r.body).toEqual(Buffer.alloc(1000, 66));
});
});
Loading