diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index 10c2ad3343f4..e12b0a0fd8cd 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -2064,13 +2064,27 @@ impl BlobExt for Blob { // index the full fixed-3 array (args[2] is written below regardless of len). let args = &mut arguments_.ptr[..]; - if self.size.get() == 0 { - let ptr = Blob::new(Blob::init_empty(global_this)); - // SAFETY: `ptr` just came from `heap::alloc` in `Blob::new`; force - // the inherent `Blob::to_js(&mut self)` over `JsClass::to_js`. - return Ok(unsafe { BlobExt::to_js(&*ptr, global_this) }); + // The W3C relative-start/end clamp below needs the real size. For a + // lazy `Bun.file()` the size is still the `MAX_SIZE` sentinel, so + // negative `end` (and `start`) would be computed against that and + // over/under-read. Resolve now, same as `.size` does. A non-seekable + // file (pipe, FIFO) has no meaningful size; on macOS fstat reports the + // currently-buffered byte count there, which must not cap the slice. + if self.size.get() == MAX_SIZE && self.needs_to_read_file() { + self.resolve_size(); + if let Some(store) = self.store.get() { + if let store::Data::File(file) = store.data_mut() { + if file.seekable == Some(false) { + self.size.set(MAX_SIZE); + } + } + } } + // No `size == 0` early return: the clamp below already yields + // `(0, 0)` and `get_slice_from` keeps the store (so a missing file + // still surfaces ENOENT on read) and honours the `contentType` arg. + // If the optional start parameter is not used as a parameter, let relativeStart be 0. let mut relative_start: i64 = 0; // If the optional end parameter is not used, let relativeEnd be size. diff --git a/src/runtime/webcore/FileReader.rs b/src/runtime/webcore/FileReader.rs index 2cb0fba6f932..41cedeb66e7d 100644 --- a/src/runtime/webcore/FileReader.rs +++ b/src/runtime/webcore/FileReader.rs @@ -604,19 +604,27 @@ impl FileReader { } let mut has_more = state != ReadState::Eof; - if !buf.is_empty() { - if let Some(max_size) = self.max_size { - let total_readed = self.total_readed.get(); - if total_readed >= max_size { - return false; - } + if let Some(max_size) = self.max_size { + let total_readed = self.total_readed.get(); + if total_readed >= max_size { + // Cap already reached (includes `max_size == 0`). Windows + // delivers chunks async with `_buffer` still populated, and a + // later empty-buf/`on_reader_done` call would hand that + // over-read out. Clear it and close so nothing past the cap + // is ever delivered. + self.done.set(true); + self.reader().buffer().clear(); + self.reader().close(); + return false; + } + if !buf.is_empty() { let len = (max_size - total_readed).min(buf.len()); if buf.len() > len { buf = &buf[0..len]; } self.total_readed.set(total_readed + len); - if buf.is_empty() { + if total_readed + len >= max_size { close = true; has_more = false; } diff --git a/test/js/bun/http/serve.test.ts b/test/js/bun/http/serve.test.ts index 8f8d3de72e5b..7b0b35d1cf06 100644 --- a/test/js/bun/http/serve.test.ts +++ b/test/js/bun/http/serve.test.ts @@ -1673,7 +1673,6 @@ describe("should support Content-Range with Bun.file()", () => { const badRanges = [ [10, NaN], [10, -Infinity], - [-(full.byteLength / 2) | 0, Infinity], [-(full.byteLength / 2) | 0, -Infinity], [full.byteLength + 100, full.byteLength], [full.byteLength + 100, full.byteLength + 100], @@ -1691,6 +1690,17 @@ describe("should support Content-Range with Bun.file()", () => { }); }); } + + // A negative start is a valid W3C relative index, not a bad range: slice(-n) + // addresses the last n bytes, same as ArrayBuffer.prototype.slice. + it(`negative start: ${-(full.byteLength / 2) | 0} - Infinity`, async () => { + const start = -(full.byteLength / 2) | 0; + await getServer(async server => { + const response = await fetch(`${server.url.origin}/?start=${start}&end=Infinity`); + expect(await response.arrayBuffer()).toEqual(full.buffer.slice(start, Infinity)); + expect(response.status).toBe(206); + }); + }); }); it("formats error responses correctly", async () => { diff --git a/test/js/web/fetch/blob.test.ts b/test/js/web/fetch/blob.test.ts index dc5ccbd1adf9..34db1ed9eb7b 100644 --- a/test/js/web/fetch/blob.test.ts +++ b/test/js/web/fetch/blob.test.ts @@ -27,12 +27,10 @@ for (const info of [ { blob: new Blob(["Bun", "Foo"]), name: "Blob.slice", - is_file: false, }, { blob: Bun.file(path.join(import.meta.dir, "fixtures", "slice.txt")), name: "Bun.file().slice", - is_file: true, }, ]) { test(info.name, async () => { @@ -45,11 +43,8 @@ for (const info of [ expect(b2.size).toBe(0); const b3 = blob.slice(100, 3); expect(b3.size).toBe(0); - // file will lazy read until EOF if the size is wrong - if (!info.is_file) { - const b4 = blob.slice(0, 10); - expect(b4.size).toBe(blob.size); - } + const b4 = blob.slice(0, 10); + expect(b4.size).toBe(blob.size); expect(blob.slice().size).toBe(blob.size); expect(blob.slice(0).size).toBe(blob.size); expect(blob.slice(NaN).size).toBe(blob.size); @@ -93,6 +88,81 @@ for (const info of [ }); } +describe.concurrent("Bun.file().slice with relative start/end", () => { + // Each case slices a FRESH `Bun.file()` so the size has not been resolved by + // a prior `.size` read. The slice clamp must stat the file itself rather than + // compute against the lazy MAX_SIZE sentinel. + const cases: Array<[[number] | [number, number], string]> = [ + [[1, -1], "12345678"], + [[0, -6], "0123"], + [[3, -3], "3456"], + [[0, -10], ""], + [[5, -5], ""], + [[0, -100], ""], + [[-3], "789"], + [[-3, -1], "78"], + [[-100], "0123456789"], + [[-100, -1], "012345678"], + [[0, 100], "0123456789"], + [[1, 3], "12"], + ]; + const memBlob = new Blob(["0123456789"]); + + test.each(cases)("slice(%p) size/text/bytes/arrayBuffer/stream", async (args, want) => { + using dir = tempDir("bun-file-slice-rel", { "f.bin": "0123456789" }); + const p = path.join(String(dir), "f.bin"); + + // Each consumer reads a fresh slice of a fresh Bun.file(). + const file = () => Bun.file(p).slice(...(args as [number, number])); + + // In-memory Blob is the reference implementation. + expect({ size: want.length, text: want }).toEqual({ + size: memBlob.slice(...(args as [number, number])).size, + text: await memBlob.slice(...(args as [number, number])).text(), + }); + + expect(file().size).toBe(want.length); + expect(await file().text()).toBe(want); + expect(Buffer.from(await file().bytes()).toString()).toBe(want); + expect(Buffer.from(await file().arrayBuffer()).toString()).toBe(want); + expect(await new Response(file().stream()).text()).toBe(want); + }); + + test("slice-of-slice with negative end", async () => { + using dir = tempDir("bun-file-slice-rel", { "f.bin": "0123456789" }); + const p = path.join(String(dir), "f.bin"); + const inner = Bun.file(p).slice(1, -1).slice(1, -1); + expect({ size: inner.size, text: await inner.text() }).toEqual({ size: 6, text: "234567" }); + }); + + test(".size agrees with the bytes actually read", async () => { + using dir = tempDir("bun-file-slice-rel", { "f.bin": "0123456789" }); + const p = path.join(String(dir), "f.bin"); + const b = Bun.file(p).slice(1, -1); + const size = b.size; + const text = await b.text(); + expect({ size, textLength: text.length }).toEqual({ size: 8, textLength: 8 }); + }); + + test("slice of a nonexistent file still rejects with ENOENT on read", async () => { + using dir = tempDir("bun-file-slice-rel", {}); + const bad = path.join(String(dir), "does-not-exist"); + // Resolving the size for the clamp must not drop the File store; the + // read still has to surface the open() error, same as the unsliced case. + expect(async () => await Bun.file(bad).slice(0, 5).text()).toThrow(expect.objectContaining({ code: "ENOENT" })); + expect(async () => await Bun.file(bad).slice(1, -1).text()).toThrow(expect.objectContaining({ code: "ENOENT" })); + }); + + test("slice preserves the contentType arg when the source is empty", async () => { + using dir = tempDir("bun-file-slice-rel", { "empty.bin": "" }); + const p = path.join(String(dir), "empty.bin"); + // Same registry-normalised type a non-empty source produces. + const want = new Blob(["x"]).slice(0, 1, "text/html").type; + expect(Bun.file(p).slice(0, 5, "text/html").type).toBe(want); + expect(new Blob([]).slice(0, 0, "text/html").type).toBe(want); + }); +}); + test("new Blob", () => { var blob = new Blob(["Bun", "Foo"], { type: "text/foo" }); expect(blob.size).toBe(6);