Skip to content
Open
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
18 changes: 18 additions & 0 deletions src/jsc/webcore_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ pub struct Blob {
pub charset: Cell<AsciiStatus>,
/// Was it created via the `File` constructor?
pub is_jsdom_file: Cell<bool>,
/// True when `size` is a bound the caller asked for (`Blob.slice`). For a
/// file store `size` is otherwise a `stat` hint, and a hint must never cap
/// a read: procfs/sysfs regular files report `st_size == 0` yet read more.
pub size_is_explicit: Cell<bool>,
/// `bun.ptr.RawRefCount(u32, .single_threaded)` — counts in-flight `*Blob`
/// borrows handed to async readers; not the JS GC retain count. Zero while
/// the JS cell is the sole owner.
Expand Down Expand Up @@ -105,6 +109,7 @@ impl Default for Blob {
content_type_was_set: Cell::new(false),
charset: Cell::new(AsciiStatus::Unknown),
is_jsdom_file: Cell::new(false),
size_is_explicit: Cell::new(false),
ref_count: bun_ptr::RawRefCount::init(0),
global_this: Cell::new(core::ptr::null()),
last_modified: Cell::new(0.0),
Expand Down Expand Up @@ -351,13 +356,26 @@ impl Blob {
content_type_was_set: Cell::new(self.content_type_was_set.get()),
charset: Cell::new(self.charset.get()),
is_jsdom_file: Cell::new(self.is_jsdom_file.get()),
size_is_explicit: Cell::new(self.size_is_explicit.get()),
ref_count: bun_ptr::RawRefCount::init(0), // setNotHeapAllocated
global_this: Cell::new(self.global_this.get()),
last_modified: Cell::new(self.last_modified.get()),
name: self.name.clone(),
}
}

/// Upper bound on the bytes a read of this view may return. `MAX_SIZE`
/// means "until EOF" — the only bound a file-backed blob has is one the
/// caller set with `slice()`.
#[inline]
pub fn read_limit(&self) -> SizeType {
if self.size_is_explicit.get() {
self.size.get()
} else {
MAX_SIZE
}
}

// ────────────────────────────────────────────────────────────────────
// Data-only predicates. LAYERING: hoisted from
// `bun_runtime::webcore::blob::BlobExt` — these read only the `Store`
Expand Down
27 changes: 22 additions & 5 deletions src/runtime/webcore/Blob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ impl BlobExt for Blob {
global.bun_vm().event_loop(),
self.store().expect("infallible: store present").clone(),
self.offset.get(),
self.size.get(),
self.read_limit(),
handler.cast(),
);
return promise_value;
Expand All @@ -489,7 +489,7 @@ impl BlobExt for Blob {
let file_read = read_file::ReadFile::create(
self.store().expect("infallible: store present").clone(),
self.offset.get(),
self.size.get(),
self.read_limit(),
handler,
)
.unwrap_or_else(|e| bun_core::handle_oom(Err(e)));
Expand Down Expand Up @@ -694,7 +694,7 @@ impl BlobExt for Blob {
global.bun_vm().event_loop(),
self.store().expect("infallible: store present").clone(),
self.offset.get(),
self.size.get(),
self.read_limit(),
NewInternalReadFileHandler::<C, F>::run,
ctx.cast::<c_void>(),
);
Expand All @@ -706,7 +706,7 @@ impl BlobExt for Blob {
ctx.cast::<c_void>(),
NewInternalReadFileHandler::<C, F>::run,
self.offset.get(),
self.size.get(),
self.read_limit(),
)
.unwrap_or_else(|e| bun_core::handle_oom(Err(e)));
let read_file_task = read_file::ReadFileTask::create_on_js_thread(
Expand Down Expand Up @@ -2031,6 +2031,9 @@ impl BlobExt for Blob {
let blob = self.dupe();
blob.offset.set(offset);
blob.size.set(len);
// `MAX_SIZE` is the "unbounded" sentinel, so an unbounded slice of an
// unresolved file blob is still unbounded.
blob.size_is_explicit.set(len != MAX_SIZE);

// dupe() deep-copies an allocated content_type; we're about to replace it,
// so release that copy first to avoid leaking it.
Expand Down Expand Up @@ -2403,6 +2406,12 @@ impl BlobExt for Blob {
if store.data_mut().as_file().seekable.is_none() {
resolve_file_stat(store);
}
// A sliced view already carries the bounds the caller asked for,
// and `stat` cannot clamp them: its size is a hint (see
// `Blob::read_limit`), so a 0 from procfs would zero the slice.
if self.size_is_explicit.get() {
return;
}
// Fresh borrow after possible mutation by `resolve_file_stat`.
let file = store.data_mut().as_file();

Expand Down Expand Up @@ -2461,6 +2470,10 @@ impl BlobExt for Blob {
if store.data_mut().as_file().seekable.is_none() {
resolve_file_stat(store);
}
// see `resolve_size` — a caller-supplied bound is authoritative.
if self.size_is_explicit.get() {
return (self.offset.get(), self.size.get());
}
// Fresh borrow after possible mutation by `resolve_file_stat`.
let file = store.data_mut().as_file();
if file.seekable.is_some() && file.max_size != MAX_SIZE {
Expand Down Expand Up @@ -3469,6 +3482,7 @@ impl BlobExt for Blob {
),
charset: Cell::new(blob.charset.get()),
is_jsdom_file: Cell::new(blob.is_jsdom_file.get()),
size_is_explicit: Cell::new(blob.size_is_explicit.get()),
ref_count: bun_ptr::RawRefCount::init(0), // setNotHeapAllocated
global_this: Cell::new(blob.global_this.get()),
last_modified: Cell::new(blob.last_modified.get()),
Expand Down Expand Up @@ -4160,7 +4174,10 @@ impl FormDataContext<'_> {
rf_args.encoding = crate::node::types::Encoding::Buffer;
rf_args.path = file.pathlike.clone();
rf_args.offset = blob.offset.get();
rf_args.max_size = Some(blob.size.get());
// `None` lets `read_file` read past a `stat` size it
// cannot trust; only a sliced view caps the read.
let limit = blob.read_limit();
rf_args.max_size = (limit != MAX_SIZE).then_some(limit);
let res = node_fs.read_file(&rf_args, crate::node::fs::Flavor::Sync);
match res {
Err(err) => {
Expand Down
4 changes: 2 additions & 2 deletions src/runtime/webcore/ReadableStream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,8 +349,8 @@ impl ReadableStream {
global_this.bun_vm().as_mut().event_loop().cast(),
)),
start_offset: Some(blob.offset.get() as usize),
max_size: if blob.size.get() != webcore::blob::MAX_SIZE {
Some(blob.size.get() as usize)
max_size: if blob.read_limit() != webcore::blob::MAX_SIZE {
Some(blob.read_limit() as usize)
} else {
None
},
Expand Down
14 changes: 2 additions & 12 deletions src/runtime/webcore/blob/read_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -732,18 +732,8 @@ impl ReadFile {
return self.on_finish();
}

// Special files might report a size of > 0, and be wrong.
// so we should check specifically that its a regular file before trusting the size.
if self.size == 0 && bun_sys::is_regular_file(self.file_store.mode) {
self.buffer = Vec::new();
// `Bytes` owns its allocation, so leave `byte_store`
// default — `then()` reads `self.buffer` directly.
self.byte_store = ByteStore::default();

self.on_finish();
return;
}

// `size` only sizes the buffer: procfs/sysfs regular files report
// `st_size == 0` and still read more, so the loop below runs to EOF.
// add an extra 16 bytes to the buffer to avoid having to resize it for trailing extra data
if !self.could_block || (self.size > 0 && self.size != MAX_SIZE) {
let want = (self.size as usize).saturating_add(16);
Expand Down
6 changes: 4 additions & 2 deletions src/runtime/webcore/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1735,7 +1735,7 @@ fn fetch_impl<const ALLOW_GET_BODY: bool>(

// TODO: make this async + lazy
let blob_offset = body.any_blob().blob().offset.get();
let blob_size = body.any_blob().blob().size.get();
let read_limit = body.any_blob().blob().read_limit();
// The `vm.node_fs()` accessor is a jsc↔runtime cycle. `read_file`
// with an `Fd` path only touches `self.sync_error_buf` for
// path-variant inputs, so a fresh `NodeFS` is sufficient here.
Expand All @@ -1745,7 +1745,9 @@ fn fetch_impl<const ALLOW_GET_BODY: bool>(
rf_args.encoding = Encoding::Buffer;
rf_args.path = PathOrFileDescriptor::Fd(*opened_fd);
rf_args.offset = blob_offset;
rf_args.max_size = Some(blob_size);
// `None` lets `read_file` read past a `stat` size it cannot trust;
// only a sliced view caps the read.
rf_args.max_size = (read_limit != blob::MAX_SIZE).then_some(read_limit);
let res = node_fs.read_file(&rf_args, node::fs::Flavor::Sync);

// Eagerly close before constructing the (potentially large) JS
Expand Down
111 changes: 111 additions & 0 deletions test/js/bun/util/bun-file-read.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { expect, it } from "bun:test";
import { isLinux, tempDir } from "harness";
import { tmpdir } from "node:os";
import { join } from "node:path";

it("offset should work in Bun.file() #4963", async () => {
const filename = tmpdir() + "/bun.test.offset.txt";
Expand All @@ -9,3 +11,112 @@ it("offset should work in Bun.file() #4963", async () => {
const contents = await slice.text();
expect(contents).toBe("ntents");
});

async function drain(stream: ReadableStream<Uint8Array>): Promise<string> {
let out = "";
const decoder = new TextDecoder();
for await (const chunk of stream) out += decoder.decode(chunk, { stream: true });
return out + decoder.decode();
}

// procfs/sysfs/cgroupfs files are regular files that stat as 0 bytes but read
// more, so a blob's stat size can never bound a read of the whole file.
const PROC_FILE = "/proc/version";

it.skipIf(!isLinux)("reading a procfs file is not capped by its stat size", async () => {
const expected = await Bun.file(PROC_FILE).text();
expect(expected.length).toBeGreaterThan(0);
expect(Bun.file(PROC_FILE).size).toBe(0);

// exists() and .size both resolve the lazy stat onto the blob; neither may
// turn the blob into an empty one.
const afterExists = Bun.file(PROC_FILE);
expect(await afterExists.exists()).toBe(true);
expect(await afterExists.text()).toBe(expected);

const afterSize = Bun.file(PROC_FILE);
expect(afterSize.size).toBe(0);
expect(new TextDecoder().decode(await afterSize.bytes())).toBe(expected);

const afterArrayBuffer = Bun.file(PROC_FILE);
expect(await afterArrayBuffer.exists()).toBe(true);
expect((await afterArrayBuffer.arrayBuffer()).byteLength).toBe(expected.length);
});

it.skipIf(!isLinux)("streaming a procfs file is not capped by its stat size", async () => {
const expected = await Bun.file(PROC_FILE).text();

expect(await drain(Bun.file(PROC_FILE).stream())).toBe(expected);
expect(await drain(new Response(Bun.file(PROC_FILE)).body!)).toBe(expected);

const afterExists = Bun.file(PROC_FILE);
expect(await afterExists.exists()).toBe(true);
expect(await drain(afterExists.stream())).toBe(expected);
});

it.skipIf(!isLinux)("a procfs file appended to FormData carries its contents", async () => {
const expected = await Bun.file(PROC_FILE).text();

const form = new FormData();
form.append("f", Bun.file(PROC_FILE));
expect(await new Response(form).text()).toContain(expected);
});

it.skipIf(!isLinux)("a procfs file uploaded as a fetch body is sent whole", async () => {
const expected = await Bun.file(PROC_FILE).text();

using server = Bun.serve({
port: 0,
fetch: async req => new Response(await req.text()),
});

expect(await (await fetch(server.url, { method: "POST", body: Bun.file(PROC_FILE) })).text()).toBe(expected);

const afterExists = Bun.file(PROC_FILE);
expect(await afterExists.exists()).toBe(true);
expect(await (await fetch(server.url, { method: "POST", body: afterExists })).text()).toBe(expected);
});

it("a sliced Bun.file() keeps its bounds when uploaded as a fetch body", async () => {
using dir = tempDir("bun-file-slice-fetch", { "hello.txt": "hello world" });
const file = Bun.file(join(String(dir), "hello.txt"));

using server = Bun.serve({
port: 0,
fetch: async req => new Response(await req.text()),
});

expect(await (await fetch(server.url, { method: "POST", body: file.slice(0, 5) })).text()).toBe("hello");
expect(await (await fetch(server.url, { method: "POST", body: file })).text()).toBe("hello world");
});

it("a sliced Bun.file() keeps its bounds when read as a Response body", async () => {
using dir = tempDir("bun-file-slice-body", { "hello.txt": "hello world" });
const file = Bun.file(join(String(dir), "hello.txt"));

expect(await drain(new Response(file.slice(0, 5)).body!)).toBe("hello");
expect(await drain(new Response(file.slice(6, 11)).body!)).toBe("world");
expect(await new Response(file.slice(0, 5)).text()).toBe("hello");
expect(await drain(file.slice(0, 5).stream())).toBe("hello");
expect(await file.slice(0, 0).text()).toBe("");
});

it("an empty file still reads empty after exists()", async () => {
using dir = tempDir("bun-file-empty-after-exists", { "empty.txt": "" });
const file = Bun.file(join(String(dir), "empty.txt"));

expect(await file.exists()).toBe(true);
expect(file.size).toBe(0);
expect(await file.text()).toBe("");
expect(await drain(file.stream())).toBe("");
});

it("a regular file still reads after exists()", async () => {
using dir = tempDir("bun-file-after-exists", { "hello.txt": "hello world" });
const file = Bun.file(join(String(dir), "hello.txt"));

expect(await file.exists()).toBe(true);
expect(file.size).toBe(11);
expect(await file.text()).toBe("hello world");
expect(await drain(file.stream())).toBe("hello world");
});
Loading