Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
5 changes: 5 additions & 0 deletions src/jsc/webcore_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,9 @@ pub struct Blob {
pub charset: Cell<AsciiStatus>,
/// Was it created via the `File` constructor?
pub is_jsdom_file: Cell<bool>,
/// Set by `.slice()`. A byte-range view is read-only for file/S3 stores
/// (`delete`/`writer`/`write` would act on the whole path).
Comment thread
robobun marked this conversation as resolved.
Outdated
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 +165,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 +378,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
23 changes: 21 additions & 2 deletions src/runtime/webcore/Blob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1281,9 +1281,15 @@ 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;
};
}
// A slice has concrete `size`, so the `MAX_SIZE` gate above skipped the
// stat and `file.mode` is still 0. `seekable == None` means never stat'd.
Comment thread
robobun marked this conversation as resolved.
Outdated
if 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 +1976,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 +3346,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 @@ -5012,6 +5020,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!(
"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."
)));
}
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 @@ -5307,6 +5321,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!(
"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."
)));
}
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
138 changes: 136 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,136 @@ 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);
}
});

// `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