diff --git a/src/jsc/webcore_types.rs b/src/jsc/webcore_types.rs index 9a9a0775bd7b..580a2919a6de 100644 --- a/src/jsc/webcore_types.rs +++ b/src/jsc/webcore_types.rs @@ -129,6 +129,8 @@ pub struct Blob { pub charset: Cell, /// Was it created via the `File` constructor? pub is_jsdom_file: Cell, + /// Set by `.slice()`; file/S3 `delete`/`writer`/`write` reject it. + pub is_sliced_view: Cell, /// `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. @@ -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), @@ -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()), diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index b3dedd3bf6c3..03b1ac7e5636 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -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; @@ -778,6 +779,8 @@ impl BlobExt for Blob { writer.write_int_le::(0)?; } } + + writer.write_int_le::(self.is_sliced_view.get() as u8)?; Ok(()) } @@ -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(); } @@ -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 { @@ -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 @@ -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()), @@ -4254,9 +4265,11 @@ fn on_structured_clone_deserialize>( 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::()? != 0); } debug_assert!( @@ -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)) + ); + } // 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() { @@ -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"))); @@ -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)) + ); + } Ok(()) } diff --git a/src/runtime/webcore/blob/read_file.rs b/src/runtime/webcore/blob/read_file.rs index c7469512233d..ff5668ea67d7 100644 --- a/src/runtime/webcore/blob/read_file.rs +++ b/src/runtime/webcore/blob/read_file.rs @@ -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 { diff --git a/test/js/bun/io/bun-write.test.js b/test/js/bun/io/bun-write.test.js index a4c3fb2f551b..ab695b8d83b7 100644 --- a/test/js/bun/io/bun-write.test.js +++ b/test/js/bun/io/bun-write.test.js @@ -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); } { diff --git a/test/js/bun/util/bun-file.test.ts b/test/js/bun/util/bun-file.test.ts index 03e5b1a61fb2..9fc67542596f 100644 --- a/test/js/bun/util/bun-file.test.ts +++ b/test/js/bun/util/bun-file.test.ts @@ -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 () => { @@ -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) => f.slice(2, 5)], + ["slice(5)", (f: ReturnType) => f.slice(5)], + ["slice(0, 3)", (f: ReturnType) => f.slice(0, 3)], + ["slice(-3)", (f: ReturnType) => f.slice(-3)], + ["slice().slice(2, 5)", (f: ReturnType) => 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); + }); +});