diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index 540a17195ec0..f85d1129ff0f 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -6032,15 +6032,22 @@ impl Any { impl Any { fn to_internal_blob_if_possible(&mut self) { - if let Any::Blob(blob) = self { - if let Some(s) = blob.store.get() { - if matches!(s.data, store::Data::Bytes(_)) && s.has_one_ref() { - let internal = Store::data_mut(s).as_bytes_mut().to_internal_blob(); - *self = Any::InternalBlob(internal); - return; - } - } + let Any::Blob(blob) = self else { + return; + }; + let Some(s) = blob.store.get() else { + return; + }; + let store::Data::Bytes(bytes) = &s.data else { + return; + }; + // A slice can hold the last reference to its parent's store. + let views_whole_store = blob.offset.get() == 0 && blob.size.get() >= bytes.len(); + if !s.has_one_ref() || !views_whole_store { + return; } + let internal = Store::data_mut(s).as_bytes_mut().to_internal_blob(); + *self = Any::InternalBlob(internal); } pub(crate) fn to_action_value( diff --git a/test/js/web/fetch/blob.test.ts b/test/js/web/fetch/blob.test.ts index 0c41948f887e..ec1a2a16e98f 100644 --- a/test/js/web/fetch/blob.test.ts +++ b/test/js/web/fetch/blob.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, isASAN, tempDir } from "harness"; +import { bunEnv, bunExe, gcTick, isASAN, tempDir } from "harness"; import type { BlobOptions } from "node:buffer"; import type { BinaryLike } from "node:crypto"; import path from "node:path"; @@ -610,6 +610,137 @@ describe("slice bounds are respected when streaming and serving", () => { }); }); +// A slice shares its parent's backing store. Once the parent and the slice +// objects have been collected, a stream made from the slice holds the only +// reference to that store, and the buffered consumers (Bun.readableStreamTo*, +// stream.text(), text() of a Response or Request whose body is that stream) +// take a fast path that hands the store itself to JS. That path used to ignore +// the slice's offset and size and return every byte in the store. +describe("consuming a slice's stream after the Blobs were collected", () => { + // Bytes on both sides of the slice, non-ASCII after it. The slice is valid + // JSON so that every consumer can be checked against the same fixture. + const parts = ["xx", '"abc"', "héllo wörld ✓"]; + const start = 2; + const end = 7; + const sliced = '"abc"'; + + // Resolves once every Blob passed to `track` has been finalized, which is + // when it releases its reference on the store. + async function afterBlobsAreCollected(make: (track: (blob: Blob) => Blob) => T): Promise { + let tracked = 0; + let collected = 0; + const registry = new FinalizationRegistry(() => collected++); + const result = make(blob => { + tracked++; + registry.register(blob, undefined); + return blob; + }); + while (collected < tracked) await gcTick(); + return result; + } + + type Track = (blob: Blob) => Blob; + + function fromCollectedSlice(make: (slice: Blob, track: Track) => T) { + return afterBlobsAreCollected(track => { + const parent = track(new Blob(parts)); + const slice = track(parent.slice(start, end)); + return make(slice, track); + }); + } + + const sources: [string, (slice: Blob, track: Track) => ReadableStream][] = [ + ["slice.stream()", slice => slice.stream()], + ["new Response(slice).body", slice => new Response(slice).body!], + ["new Blob([slice]).stream()", (slice, track) => track(new Blob([slice])).stream()], + ]; + + const consumers: [string, (stream: ReadableStream) => Promise, unknown][] = [ + ["Bun.readableStreamToText", stream => Bun.readableStreamToText(stream), sliced], + [ + "Bun.readableStreamToBytes", + async stream => Buffer.from(await Bun.readableStreamToBytes(stream)).toString(), + sliced, + ], + [ + "Bun.readableStreamToArrayBuffer", + async stream => Buffer.from(await Bun.readableStreamToArrayBuffer(stream)).toString(), + sliced, + ], + ["Bun.readableStreamToJSON", stream => Bun.readableStreamToJSON(stream), "abc"], + [ + "Bun.readableStreamToBlob", + async stream => { + const blob = await Bun.readableStreamToBlob(stream); + return [blob.size, await blob.text()]; + }, + [sliced.length, sliced], + ], + ["stream.text()", stream => stream.text(), sliced], + ["stream.json()", stream => stream.json(), "abc"], + ["stream.bytes()", async stream => Buffer.from(await stream.bytes()).toString(), sliced], + ]; + + describe.each(sources)("%s", (_, makeStream) => { + test.each(consumers)("%s", async (_, consume, expected) => { + const stream = await fromCollectedSlice(makeStream); + expect(await consume(stream)).toEqual(expected); + }); + }); + + // A body keeps a Blob-backed stream as its stream, and text() reads it + // through the same fast path as Bun.readableStreamToText. + const bodies: [string, (slice: Blob) => Response | Request][] = [ + [ + "new Response(slice) after .body was accessed", + slice => { + const response = new Response(slice); + expect(response.body).toBeInstanceOf(ReadableStream); + return response; + }, + ], + ["new Response(slice.stream())", slice => new Response(slice.stream())], + ["new Response(new Response(slice).body)", slice => new Response(new Response(slice).body)], + [ + "new Request(url, { body: new Response(slice).body })", + slice => new Request("http://localhost/", { method: "POST", body: new Response(slice).body, duplex: "half" }), + ], + ]; + + test.each(bodies)("%s, read by text()", async (_, makeBody) => { + const body = await fromCollectedSlice(makeBody); + expect(await body.text()).toBe(sliced); + }); + + // A prefix starts at the store's first byte and a suffix ends at its last: + // the offset alone, or the end alone, does not tell them from the whole store. + const windows: [string, (parent: Blob, track: Track) => Blob, string][] = [ + ["slice(0, 7), a prefix", parent => parent.slice(0, 7), 'xx"abc"'], + ["slice(0, 7).slice(0, 2), a prefix of a prefix", (parent, track) => track(parent.slice(0, 7)).slice(0, 2), "xx"], + ["slice(7), a suffix", parent => parent.slice(7), "héllo wörld ✓"], + ["slice(1).slice(1, 6), a slice of a slice", (parent, track) => track(parent.slice(1)).slice(1, 6), sliced], + ["slice(0, 0), empty at the start", parent => parent.slice(0, 0), ""], + ["slice(2, 2), empty in the middle", parent => parent.slice(2, 2), ""], + ["slice(0), the whole Blob", parent => parent.slice(0), parts.join("")], + ]; + + test.each(windows)("%s", async (_, makeWindow, expected) => { + const stream = await afterBlobsAreCollected(track => track(makeWindow(track(new Blob(parts)), track)).stream()); + expect(await Bun.readableStreamToText(stream)).toBe(expected); + }); + + // An unsliced Blob views its whole store, so handing the store over is fine. + // A type makes the stream go through the same code a slice does, with a + // view that happens to cover the store. + test.each([ + ["untyped", undefined], + ["typed", { type: "text/plain" }], + ])("%s unsliced Blob still yields everything", async (_, options) => { + const stream = await afterBlobsAreCollected(track => track(new Blob(parts, options)).stream()); + expect(await Bun.readableStreamToText(stream)).toBe(parts.join("")); + }); +}); + // Wrapping a Blob whose type is heap-owned (not in the mime table) with a // known mime type overwrote content_type with a static pointer without // clearing content_type_allocated, so GC sweep freed a static pointer.