diff --git a/src/bun.js/webcore/Request.zig b/src/bun.js/webcore/Request.zig index 2210700a11..0a04f43245 100644 --- a/src/bun.js/webcore/Request.zig +++ b/src/bun.js/webcore/Request.zig @@ -866,19 +866,23 @@ pub fn doClone( const js_wrapper = cloned.toJS(globalThis); if (js_wrapper != .zero) { - if (cloned.#body.value == .Locked) { - if (cloned.#body.value.Locked.readable.get(globalThis)) |readable| { - // If we are teed, then we need to update the cached .body - // value to point to the new readable stream - // We must do this on both the original and cloned request - // but especially the original request since it will have a stale .body value now. - js.bodySetCached(js_wrapper, globalThis, readable.value); - if (this.#body.value.Locked.readable.get(globalThis)) |other_readable| { - js.bodySetCached(this_value, globalThis, other_readable.value); - } - } + // After toJS, checkBodyStreamRef has already moved the streams from + // Locked.readable to js.gc.stream. So we need to use js.gc.stream + // to get the streams and update the body cache. + if (js.gc.stream.get(js_wrapper)) |cloned_stream| { + js.bodySetCached(js_wrapper, globalThis, cloned_stream); } } + + // Update the original request's body cache with the new teed stream. + // At this point, this.#body.value.Locked.readable still holds the teed stream + // because checkBodyStreamRef hasn't been called on the original request yet. + if (this.#body.value == .Locked) { + if (this.#body.value.Locked.readable.get(globalThis)) |readable| { + js.bodySetCached(this_value, globalThis, readable.value); + } + } + this.checkBodyStreamRef(globalThis); return js_wrapper; } diff --git a/src/bun.js/webcore/Response.zig b/src/bun.js/webcore/Response.zig index a23b258f4b..b02967ccb6 100644 --- a/src/bun.js/webcore/Response.zig +++ b/src/bun.js/webcore/Response.zig @@ -381,17 +381,20 @@ pub fn doClone( const js_wrapper = Response.makeMaybePooled(globalThis, cloned); if (js_wrapper != .zero) { - if (cloned.#body.value == .Locked) { - if (cloned.#body.value.Locked.readable.get(globalThis)) |readable| { - // If we are teed, then we need to update the cached .body - // value to point to the new readable stream - // We must do this on both the original and cloned response - // but especially the original response since it will have a stale .body value now. - js.bodySetCached(js_wrapper, globalThis, readable.value); - if (this.#body.value.Locked.readable.get(globalThis)) |other_readable| { - js.bodySetCached(this_value, globalThis, other_readable.value); - } - } + // After toJS/makeMaybePooled, checkBodyStreamRef has already moved + // the streams from Locked.readable to js.gc.stream. So we need to + // use js.gc.stream to get the streams and update the body cache. + if (js.gc.stream.get(js_wrapper)) |cloned_stream| { + js.bodySetCached(js_wrapper, globalThis, cloned_stream); + } + } + + // Update the original response's body cache with the new teed stream. + // At this point, this.#body.value.Locked.readable still holds the teed stream + // because checkBodyStreamRef hasn't been called on the original response yet. + if (this.#body.value == .Locked) { + if (this.#body.value.Locked.readable.get(globalThis)) |readable| { + js.bodySetCached(this_value, globalThis, readable.value); } } diff --git a/test/js/web/fetch/response.test.ts b/test/js/web/fetch/response.test.ts index cdbdfd42a9..ca309439cc 100644 --- a/test/js/web/fetch/response.test.ts +++ b/test/js/web/fetch/response.test.ts @@ -49,7 +49,7 @@ describe("2-arg form", () => { test("print size", () => { expect(normalizeBunSnapshot(Bun.inspect(new Response(Bun.file(import.meta.filename)))), import.meta.dir) .toMatchInlineSnapshot(` - "Response (4.15 KB) { + "Response (5.83 KB) { ok: true, url: "", status: 200, @@ -123,3 +123,56 @@ test("handle stack overflow", () => { f0(f0); }).toThrow("Maximum call stack size exceeded."); }); + +describe("clone()", () => { + test("does not lock original body when body was accessed before clone", async () => { + const readableStream = new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode("Hello, world!")); + controller.close(); + }, + }); + + const response = new Response(readableStream); + + // Access body before clone (this triggers the bug in the unfixed version) + const bodyBeforeClone = response.body; + expect(bodyBeforeClone?.locked).toBe(false); + + const cloned = response.clone(); + + // Both should be unlocked after clone + expect(response.body?.locked).toBe(false); + expect(cloned.body?.locked).toBe(false); + + // Both should be readable + const [originalText, clonedText] = await Promise.all([response.text(), cloned.text()]); + + expect(originalText).toBe("Hello, world!"); + expect(clonedText).toBe("Hello, world!"); + }); + + test("works when body is not accessed before clone", async () => { + const readableStream = new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode("Hello, world!")); + controller.close(); + }, + }); + + const response = new Response(readableStream); + + // Do NOT access body before clone + const cloned = response.clone(); + + // Both should be unlocked after clone + expect(response.body?.locked).toBe(false); + expect(cloned.body?.locked).toBe(false); + + // Both should be readable + const [originalText, clonedText] = await Promise.all([response.text(), cloned.text()]); + + expect(originalText).toBe("Hello, world!"); + expect(clonedText).toBe("Hello, world!"); + }); +}); diff --git a/test/js/web/request/request.test.ts b/test/js/web/request/request.test.ts index 88a52f6a65..317e5229cd 100644 --- a/test/js/web/request/request.test.ts +++ b/test/js/web/request/request.test.ts @@ -47,3 +47,30 @@ test("request can receive null signal", async () => { expect(await request.text()).toBe("bun"); expect(await clone.text()).toBe("bun"); }); + +test("clone() does not lock original body when body was accessed before clone", async () => { + const readableStream = new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode("Hello, world!")); + controller.close(); + }, + }); + + const request = new Request("http://example.com", { method: "POST", body: readableStream }); + + // Access body before clone (this triggers the bug in the unfixed version) + const bodyBeforeClone = request.body; + expect(bodyBeforeClone?.locked).toBe(false); + + const cloned = request.clone(); + + // Both should be unlocked after clone + expect(request.body?.locked).toBe(false); + expect(cloned.body?.locked).toBe(false); + + // Both should be readable + const [originalText, clonedText] = await Promise.all([request.text(), cloned.text()]); + + expect(originalText).toBe("Hello, world!"); + expect(clonedText).toBe("Hello, world!"); +});