webcore: make Request/Response clone() throw on a disturbed or locked body

Merged
RO
4mo ago
+276-10

Problem

Request.prototype.clone() and Response.prototype.clone() never perform step 1 of the fetch spec's clone algorithms: "If this is unusable, then throw a TypeError", where unusable means the body is non-null and its stream is disturbed or locked (https://fetch.spec.whatwg.org/#body-unusable).

const q = new Request("http://x/", { method: "POST", body: "hello world" });
await q.text();       // body consumed
const c = q.clone();  // node, Deno, browsers: TypeError. Bun: succeeds
await c.text();       // "" (silent empty body)

What Bun does instead depends on the body's internal representation:

  • Consumed string, Blob, and FormData bodies: clone() succeeds and the clone resolves to an empty body.
  • Consumed or locked stream bodies: clone() succeeds and an error surfaces later, from the clone's own read or from the internal tee, never from clone() itself.

The check exists so that clone-after-read, which is always a bug in the caller, fails loudly in development. Without it, proxy and retry middleware that does clone() then forwards the request silently forwards an empty body whenever the order of operations is wrong.

Node (undici) throws a TypeError synchronously from clone() in each case:

$ node repro.mjs
1 req-consumed-string:   clone() THREW TypeError: unusable
2 req-consumed-stream:   clone() THREW TypeError: unusable
3 req-locked(unread):    clone() THREW TypeError: unusable
4 res-consumed-string:   clone() THREW TypeError: Response.clone: Body has already been consumed.
5 res-locked(unread):    clone() THREW TypeError: Response.clone: Body has already been consumed.
6 res-consumed-blob:     clone() THREW TypeError: Response.clone: Body has already been consumed.
7 req-consumed-formdata: clone() THREW TypeError: unusable

$ bun repro.mjs   # 1.4.0 and main
1 req-consumed-string:   clone() OK, clone.text() -> ""
2 req-consumed-stream:   clone() OK, clone.text() THREW TypeError: Body already used
3 req-locked(unread):    clone() OK, clone.text() THREW TypeError: Body already used
4 res-consumed-string:   clone() OK, clone.text() -> ""
5 res-locked(unread):    clone() OK, clone.text() THREW TypeError: Body already used
6 res-consumed-blob:     clone() OK, clone.text() -> ""
7 req-consumed-formdata: clone() OK, clone.text() -> ""

Fix

Add BodyMixin::throw_if_body_unusable and call it at the top of both do_clone entry points (src/runtime/webcore/Request.rs, src/runtime/webcore/Response.rs).

Request has a third JS entry point: BunRequest.prototype.clone, the subclass that Bun.serve routes: handlers receive. It dispatches through JSBunRequest::clone -> Request__clone -> Request::ffi_clone rather than do_clone, so it gets the same check at the top of ffi_clone. Response has no such subclass. Request::clone and Response::clone have no other callers, so every JS-visible clone() is covered and no internal clone path changes.

The unusable predicate is the existing bodyUsed walk with ReadableStream::is_locked OR'd in, so get_body_used is refactored to share a parameterized helper (body_stream_check) instead of duplicating the match.

The error is ERR_BODY_ALREADY_USED, an instance of TypeError like node and browsers, with the message Body is disturbed or locked (WebKit's wording, which covers both halves of the predicate).

Only the two clone() entry points are guarded. new Request(usedRequest) has the same spec check (Request constructor step 36.1) and is also missing in Bun, but it is a separate algorithm with a much wider blast radius in Bun.serve middleware, so it is intentionally not part of this PR. fetch(usedRequest) already throws.

Tests

test/js/web/fetch/body-clone.test.ts:

  • New describe("clone() throws when the body is disturbed or locked"): consumed string / user-stream / Blob / FormData bodies, locked bodies, an in-flight read, a fetch() response disturbed by a reader, a consumed Bun.serve incoming request (catch-all fetch handler), and a consumed BunRequest from a routes: handler (both a /:param route and a static one). All fail on main.
  • Negative coverage: null bodies, an unread Bun.serve request (both handler kinds), and a body materialized by the body getter still clone.
  • The existing "clone() on a locked stream body throws a catchable TypeError" subprocess test now asserts the new message. Since the usability check fires before the stream is teed, that test no longer reaches the readableStreamTee exception-propagation path it was added for in #32786, so a sibling test drives the same path through new Request(lockedRequest), which still tees, and keeps that coverage.

With the fix, bun bd test test/js/web/fetch/body-clone.test.ts passes 44/44. The surrounding body.test.ts, response.test.ts, body-stream.test.ts, blob.test.ts, and test/js/web/request/ suites are unchanged.