Skip to content
Closed
24 changes: 19 additions & 5 deletions src/runtime/webcore/Blob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
}
Comment thread
robobun marked this conversation as resolved.

// 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.
Expand Down
22 changes: 15 additions & 7 deletions src/runtime/webcore/FileReader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
12 changes: 11 additions & 1 deletion test/js/bun/http/serve.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand All @@ -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 () => {
Expand Down
84 changes: 77 additions & 7 deletions test/js/web/fetch/blob.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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);
Expand Down Expand Up @@ -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" }));
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});

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);
Expand Down
Loading