Files
bun.sh/test/js/web/fetch/response.test.ts
robobun 344b2c1dfe fix: Response.clone() no longer locks body when body was accessed before clone (#25484)
## Summary
- Fix bug where `Response.clone()` would lock the original response's
body when `response.body` was accessed before cloning
- Apply the same fix to `Request.clone()`

## Root Cause
When `response.body` was accessed before calling `response.clone()`, the
original response's body would become locked after cloning. This
happened because:

1. When the cloned response was wrapped with `toJS()`,
`checkBodyStreamRef()` was called which moved the stream from
`Locked.readable` to `js.gc.stream` and cleared `Locked.readable`
2. The subsequent code tried to get the stream from `Locked.readable`,
which was now empty, so the body cache update was skipped
3. The JavaScript-level body property cache still held the old locked
stream

## Fix
Updated the cache update logic to:
1. For the cloned response: use `js.gc.stream.get()` instead of
`Locked.readable.get()` since `toJS()` already moved the stream
2. For the original response: use `Locked.readable.get()` which still
holds the teed stream since `checkBodyStreamRef` hasn't been called yet

## Reproduction
```javascript
const readableStream = new ReadableStream({
  start(controller) {
    controller.enqueue(new TextEncoder().encode("Hello, world!"));
    controller.close();
  },
});

const response = new Response(readableStream);
console.log(response.body?.locked); // Accessing body before clone
const cloned = response.clone();
console.log(response.body?.locked); // Expected: false, Actual: true 
console.log(cloned.body?.locked);   // Expected: false, Actual: false 
```

## Test plan
- [x] Added regression tests for `Response.clone()` in
`test/js/web/fetch/response.test.ts`
- [x] Added regression test for `Request.clone()` in
`test/js/web/request/request.test.ts`
- [x] Verified tests fail with system bun (before fix) and pass with
debug build (after fix)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Bot <claude-bot@bun.sh>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
2025-12-15 18:46:02 -08:00

179 lines
5.7 KiB
TypeScript

import { describe, expect, test } from "bun:test";
import { normalizeBunSnapshot } from "harness";
test("zero args returns an otherwise empty 200 response", () => {
const response = new Response();
expect(response.status).toBe(200);
expect(response.statusText).toBe("");
});
test("calling cancel() on response body doesn't throw", () => {
expect(() => new Response("").body?.cancel()).not.toThrow();
});
test("undefined args don't throw", () => {
const response = new Response("", {
status: undefined,
statusText: undefined,
headers: undefined,
});
expect(response.status).toBe(200);
expect(response.statusText).toBe("");
});
test("1-arg form returns a 200 response", () => {
const response = new Response("body text");
expect(response.status).toBe(200);
expect(response.statusText).toBe("");
});
describe("2-arg form", () => {
test("can fill in status/statusText, and it works", () => {
const response = new Response("body text", {
status: 202,
statusText: "Accepted.",
});
expect(response.status).toBe(202);
expect(response.statusText).toBe("Accepted.");
});
test('empty object continues to return 200/""', () => {
const response = new Response("body text", {});
expect(response.status).toBe(200);
expect(response.statusText).toBe("");
});
});
test("print size", () => {
expect(normalizeBunSnapshot(Bun.inspect(new Response(Bun.file(import.meta.filename)))), import.meta.dir)
.toMatchInlineSnapshot(`
"Response (5.83 KB) {
ok: true,
url: "",
status: 200,
statusText: "",
headers: Headers {
"content-type": "text/javascript;charset=utf-8",
},
redirected: false,
bodyUsed: false,
FileRef ("<cwd>/test/js/web/fetch/response.test.ts") {
type: "text/javascript;charset=utf-8"
}
}"
`);
});
test("Response.redirect with invalid arguments should not crash", () => {
// This should not crash - issue #18414
// Passing a number as URL and string as init should handle gracefully
expect(() => Response.redirect(400, "a")).not.toThrow();
// Test various invalid argument combinations - should not crash
expect(() => Response.redirect(42, "test")).not.toThrow();
expect(() => Response.redirect(true, "string")).not.toThrow();
expect(() => Response.redirect(null, "init")).not.toThrow();
expect(() => Response.redirect(undefined, "value")).not.toThrow();
});
test("Response.redirect status code validation", () => {
// Valid redirect status codes should work
expect(() => Response.redirect("url", 301)).not.toThrow();
expect(() => Response.redirect("url", 302)).not.toThrow();
expect(() => Response.redirect("url", 303)).not.toThrow();
expect(() => Response.redirect("url", 307)).not.toThrow();
expect(() => Response.redirect("url", 308)).not.toThrow();
// Invalid status codes should throw RangeError
expect(() => Response.redirect("url", 200)).toThrow(RangeError);
expect(() => Response.redirect("url", 400)).toThrow(RangeError);
expect(() => Response.redirect("url", 500)).toThrow(RangeError);
// Status in object should also be validated
expect(() => Response.redirect("url", { status: 307 })).not.toThrow();
expect(() => Response.redirect("url", { status: 400 })).toThrow(RangeError);
// Check that the correct status is set
expect(Response.redirect("url", 301).status).toBe(301);
expect(Response.redirect("url", { status: 308 }).status).toBe(308);
});
test("new Response(123, { statusText: 123 }) does not throw", () => {
// @ts-expect-error
expect(new Response("123", { statusText: 123 }).statusText).toBe("123");
});
test("new Response(123, { method: 456 }) does not throw", () => {
// @ts-expect-error
expect(() => new Response("123", { method: 456 })).not.toThrow();
});
test("handle stack overflow", () => {
function f0(a1, a2) {
const v4 = new Response();
// @ts-ignore
const v5 = v4.text(a2, a2, v4, f0, f0);
a1(a1); // Recursive call causes stack overflow
return v5;
}
expect(() => {
// @ts-ignore
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!");
});
});