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
4 changes: 4 additions & 0 deletions src/jsc/webcore_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ pub struct Blob {
pub charset: Cell<AsciiStatus>,
/// Was it created via the `File` constructor?
pub is_jsdom_file: Cell<bool>,
/// Set by `.slice()`; file/S3 `delete`/`writer`/`write` reject it.
pub is_sliced_view: 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 @@ -162,6 +164,7 @@ impl Default for Blob {
content_type_was_set: Cell::new(false),
charset: Cell::new(AsciiStatus::Unknown),
is_jsdom_file: Cell::new(false),
is_sliced_view: 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 @@ -374,6 +377,7 @@ 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()),
is_sliced_view: Cell::new(self.is_sliced_view.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()),
Expand Down
36 changes: 31 additions & 5 deletions src/runtime/webcore/Blob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ pub use bun_jsc::webcore_types::{
/// 3: Added File name serialization for File objects (when is_jsdom_file is true)
/// 4: Added the blob's `size` to file-backed stores so a sliced Bun.file()
/// keeps its window's end across structuredClone/postMessage
const SERIALIZATION_VERSION: u8 = 4;
/// 5: Added `is_sliced_view` so a cloned slice stays read-only for writes
const SERIALIZATION_VERSION: u8 = 5;

pub use bun_jsc::generated::JSBlob as js;

Expand Down Expand Up @@ -778,6 +779,8 @@ impl BlobExt for Blob {
writer.write_int_le::<u32>(0)?;
}
}

writer.write_int_le::<u8>(self.is_sliced_view.get() as u8)?;
Ok(())
}

Expand Down Expand Up @@ -1266,7 +1269,8 @@ impl BlobExt for Blob {
}

fn get_exists_sync(&self) -> JSValue {
if self.size.get() == MAX_SIZE {
let size_was_unresolved = self.size.get() == MAX_SIZE;
if size_was_unresolved {
self.resolve_size();
}

Expand All @@ -1281,9 +1285,14 @@ impl BlobExt for Blob {
}

// We say regular files and pipes exist.
let store::Data::File(file) = &store.data else {
if !matches!(store.data, store::Data::File(_)) {
return JSValue::FALSE;
};
}
// Slices have concrete `size`, so `resolve_size()` above was skipped and `file.mode` is still 0.
if !size_was_unresolved && store.data_mut().as_file().seekable.is_none() {
resolve_file_stat(store);
}
let file = store.data_mut().as_file();
JSValue::from(bun_sys::S::ISREG(file.mode) || bun_sys::S::ISFIFO(file.mode))
}
fn do_write(&self, global_this: &JSGlobalObject, callframe: &CallFrame) -> JsResult<JSValue> {
Expand Down Expand Up @@ -1970,6 +1979,7 @@ impl BlobExt for Blob {
let blob = self.dupe();
blob.offset.set(offset);
blob.size.set(len);
blob.is_sliced_view.set(true);

let content_type_was_allocated = content_type.is_owned() && !content_type.is_empty();
// infer the content type if it was not specified
Expand Down Expand Up @@ -3339,6 +3349,7 @@ impl BlobExt for Blob {
),
charset: Cell::new(blob.charset.get()),
is_jsdom_file: Cell::new(blob.is_jsdom_file.get()),
is_sliced_view: Cell::new(blob.is_sliced_view.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 @@ -4254,9 +4265,11 @@ fn on_structured_clone_deserialize<B: AsRef<[u8]>>(
blob.name.set(BunString::clone_utf8(&name_bytes));
}

if version == 3 {
if version <= 4 {
break 'versions;
}

blob.is_sliced_view.set(reader.read_int_le::<u8>()? != 0);
}

debug_assert!(
Expand Down Expand Up @@ -5012,6 +5025,12 @@ pub fn write_file_internal(
return Err(global_this.throw_invalid_arguments(format_args!("Blob is detached")));
};
debug_assert!(!matches!(blob_store.data, store::Data::Bytes(_)));
// Some callers bypass `validate_writable_blob`; reject sliced here too.
if blob.is_sliced_view.get() {
return Err(
global_this.throw_invalid_arguments(format_args!("{}", SLICED_VIEW_READONLY_MSG))
);
}
Comment thread
robobun marked this conversation as resolved.
// TODO only reset last_modified on success paths instead of resetting
// last_modified at the beginning for better performance.
if let store::Data::File(ref mut file) = *blob_store.data_mut() {
Expand Down Expand Up @@ -5298,6 +5317,8 @@ pub fn write_file_internal(
)
}

const SLICED_VIEW_READONLY_MSG: &str = "A sliced Bun.file() is a read-only byte-range view; delete() and write() would affect the whole file. Use the un-sliced Bun.file() instead.";

fn validate_writable_blob(global_this: &JSGlobalObject, blob: &Blob) -> JsResult<()> {
let Some(store) = blob.store.get() else {
return Err(global_this.throw(format_args!("Cannot write to a detached Blob")));
Expand All @@ -5307,6 +5328,11 @@ fn validate_writable_blob(global_this: &JSGlobalObject, blob: &Blob) -> JsResult
"Cannot write to a Blob backed by bytes, which are always read-only"
)));
}
if blob.is_sliced_view.get() {
return Err(
global_this.throw_invalid_arguments(format_args!("{}", SLICED_VIEW_READONLY_MSG))
);
}
Comment thread
robobun marked this conversation as resolved.
Ok(())
}

Expand Down
1 change: 1 addition & 0 deletions src/runtime/webcore/blob/read_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,7 @@ impl ReadFile {
let mut read_amount: usize = 0;
let mut retry = false;
let continue_reading = self.do_read(buf, &mut read_amount, &mut retry);
self.read_off += read_amount as SizeType;

// We might read into the stack buffer, so we need to copy it into the heap.
if use_stack {
Expand Down
17 changes: 10 additions & 7 deletions test/js/bun/io/bun-write.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -248,13 +248,16 @@ const IS_UV_FS_COPYFILE_DISABLED =
}

{
await Bun.write(
Bun.file(tmpbase + "fetch.js.in").slice(0, (exampleHtml.length / 2) | 0),
Bun.file(tmpbase + "fetch.js.out"),
);
expect(await Bun.file(tmpbase + "fetch.js.in").text()).toBe(
exampleHtml.substring(0, (exampleHtml.length / 2) | 0),
);
// A sliced Bun.file() destination is a read-only byte-range view; the
// previous behavior truncated the whole destination and copied the
// first N bytes, which silently destroyed the rest of the file.
expect(() =>
Bun.write(
Bun.file(tmpbase + "fetch.js.in").slice(0, (exampleHtml.length / 2) | 0),
Bun.file(tmpbase + "fetch.js.out"),
),
).toThrow(TypeError);
expect(await Bun.file(tmpbase + "fetch.js.in").text()).toBe(exampleHtml);
}

{
Expand Down
164 changes: 162 additions & 2 deletions test/js/bun/util/bun-file.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { expect, test } from "bun:test";
import { describe, expect, test } from "bun:test";
import fs from "fs";
import fsPromises from "fs/promises";
import { bunEnv, bunExe, tempDirWithFiles } from "harness";
import { bunEnv, bunExe, isPosix, tempDir, tempDirWithFiles } from "harness";
import { join } from "path";

test("delete() and stat() should work with unicode paths", async () => {
Expand Down Expand Up @@ -155,3 +156,162 @@ test("Bun.file().json() with UTF-8 BOM does not free an interior pointer", async
});
expect(exitCode).toBe(0);
});

// A `Bun.file(p).slice(a, b)` keeps the parent's pathlike store, so
// `.delete()` unlinked the whole file and `.writer()`/`.write()` truncated
// it and wrote at offset 0, ignoring the [a, b) window. A byte-range view
// has no sensible whole-file mutation semantics, so these now throw.
describe("Bun.file().slice() is a read-only view", () => {
async function tryOp(fn: () => unknown) {
try {
return { err: null, result: await fn() };
} catch (err) {
return { err, result: null };
}
}

test.concurrent.each([
["slice(2, 5)", (f: ReturnType<typeof Bun.file>) => f.slice(2, 5)],
["slice(5)", (f: ReturnType<typeof Bun.file>) => f.slice(5)],
["slice(0, 3)", (f: ReturnType<typeof Bun.file>) => f.slice(0, 3)],
["slice(-3)", (f: ReturnType<typeof Bun.file>) => f.slice(-3)],
["slice().slice(2, 5)", (f: ReturnType<typeof Bun.file>) => f.slice().slice(2, 5)],
] as const)("%s: delete()/writer()/write()/Bun.write() throw and leave the file intact", async (_label, slicer) => {
using dir = tempDir("bun-file-slice-mutate", { "f.txt": "0123456789" });
const p = join(String(dir), "f.txt");

// .delete() / .unlink()
{
const { err } = await tryOp(() => slicer(Bun.file(p)).delete());
expect({ err, after: fs.readFileSync(p, "utf8") }).toEqual({
err: expect.any(TypeError),
after: "0123456789",
});
expect((err as Error).message).toContain("sliced Bun.file()");
}

// .writer()
{
const { err } = await tryOp(() => slicer(Bun.file(p)).writer());
expect({ err, after: fs.readFileSync(p, "utf8") }).toEqual({
err: expect.any(TypeError),
after: "0123456789",
});
expect((err as Error).message).toContain("sliced Bun.file()");
}

// .write()
{
const { err } = await tryOp(() => slicer(Bun.file(p)).write("XY"));
expect({ err, after: fs.readFileSync(p, "utf8") }).toEqual({
err: expect.any(TypeError),
after: "0123456789",
});
expect((err as Error).message).toContain("sliced Bun.file()");
}

// Bun.write(dest=slice, ...)
{
const { err } = await tryOp(() => Bun.write(slicer(Bun.file(p)), "XY"));
expect({ err, after: fs.readFileSync(p, "utf8") }).toEqual({
err: expect.any(TypeError),
after: "0123456789",
});
expect((err as Error).message).toContain("sliced Bun.file()");
}
});

test.concurrent("un-sliced Bun.file() can still delete()/writer()/write() after reading .size", async () => {
using dir = tempDir("bun-file-unsliced-ops", { "f.txt": "0123456789" });
const p = join(String(dir), "f.txt");

// Reading .size resolves the stat size into blob.size (so it is no
// longer MAX_SIZE); this must not be mistaken for a slice.
{
const f = Bun.file(p);
expect(f.size).toBe(10);
await f.write("abc");
expect(fs.readFileSync(p, "utf8")).toBe("abc");
}
{
fs.writeFileSync(p, "0123456789");
const f = Bun.file(p);
expect(f.size).toBe(10);
await Bun.write(f, "xyz");
expect(fs.readFileSync(p, "utf8")).toBe("xyz");
}
{
fs.writeFileSync(p, "0123456789");
const f = Bun.file(p);
expect(f.size).toBe(10);
const w = f.writer();
w.write("hello world");
await w.end();
expect(fs.readFileSync(p, "utf8")).toBe("hello world");
}
{
fs.writeFileSync(p, "0123456789");
const f = Bun.file(p);
expect(f.size).toBe(10);
await f.delete();
expect(fs.existsSync(p)).toBe(false);
}
});

test.concurrent("structuredClone of a slice is also a read-only view", async () => {
using dir = tempDir("bun-file-slice-clone", { "f.txt": "0123456789" });
const p = join(String(dir), "f.txt");

const clone = structuredClone(Bun.file(p).slice(2, 5));
expect(await clone.text()).toBe("234");

const { err } = await tryOp(() => clone.delete());
expect({ err, after: fs.readFileSync(p, "utf8") }).toEqual({
err: expect.any(TypeError),
after: "0123456789",
});
expect(() => Bun.write(clone, "XY")).toThrow(TypeError);
expect(fs.readFileSync(p, "utf8")).toBe("0123456789");
});

test.concurrent("structuredClone of an un-sliced Bun.file() can still write after .size was read", async () => {
using dir = tempDir("bun-file-unsliced-clone", { "f.txt": "0123456789" });
const p = join(String(dir), "f.txt");
const f = Bun.file(p);
expect(f.size).toBe(10);
const clone = structuredClone(f);
await Bun.write(clone, "abc");
expect(fs.readFileSync(p, "utf8")).toBe("abc");
});

// `exists()` reads `file.mode` from the store, which is only populated by
// `resolve_size()`. A slice's `size` is concrete so `resolve_size()` was
// skipped and `exists()` returned false while `stat()` succeeded.
test.concurrent("slice().exists() agrees with slice().stat()", async () => {
using dir = tempDir("bun-file-slice-exists", { "f.txt": "0123456789" });
const p = join(String(dir), "f.txt");
const sl = Bun.file(p).slice(2, 5);
expect({
exists: await sl.exists(),
statSize: (await sl.stat()).size,
text: await sl.text(),
}).toEqual({
exists: true,
statSize: 10,
text: "234",
});
});
});

// The threadpool ReadFile loop caps each read by `max_length - read_off`, but
// `read_off` was never advanced on POSIX, so a character-device slice kept
// reading whole 64 KiB stack-buffer chunks and returned the next multiple of
// 64 KiB above the requested length (1_000_000 -> 1_048_576).
describe.skipIf(!isPosix)("Bun.file(chardev).slice().bytes() returns exactly the requested length", () => {
test.concurrent.each([1, 4095, 4096, 4097, 65535, 65536, 65537, 1_000_000])("%d bytes", async n => {
const bytes = await Bun.file("/dev/zero").slice(0, n).bytes();
expect(bytes.length).toBe(n);
expect(bytes[0]).toBe(0);
expect(bytes[n - 1]).toBe(0);
});
});
Loading