Skip to content

The 413 refusal cancels a tee branch, never the upload source #3219

Description

@frenzzy

Summary

When a body runs past bodySizeLimit, bufferBodyWithin answers 413 and calls reader.cancel(). That reader belongs to request.clone().body — a tee branch — so the cancel propagates to the branch, not to the upload. Per the Streams spec a tee cancels its source only when both branches are cancelled, and the other branch (request.body) is never cancelled anywhere in the file.

The refusal is issued; the producer is never told to stop.

Reproduction

let cancelled = false, pulls = 0;
const big = new ReadableStream({
  pull(c) { pulls++; c.enqueue(new Uint8Array(64 * 1024)); },
  cancel() { cancelled = true; }
});
const r = await handleServerFunctionRequest(new Request("http://x/_server/g", {
  method: "POST", body: big, duplex: "half",
  headers: { "Sec-Fetch-Site": "same-origin", "x-server-function-format": "8" }
}));

Observed on next @ ee73e053:

413 path: status=413 uploadSourceCancelled=false pullsAtRefusal=19

The 413 is correct and prompt. cancel() on the upload source never runs.

Where

packages/web/server-functions/src/server.ts:1149 and :1157

const reader = request.clone().body.getReader();   // a tee branch

if (total > limit) { reader.cancel().catch(() => {}); return null; }

grep "request.body" over this file returns four hits, none of which cancel. Introduced in 51392f36 feat(web): bound server-function request payloads (#3115, #3119).

Why it matters

The line reads like source teardown and is not, which is the part worth fixing regardless of impact. Practically: after a 413 the sender is not signalled to stop, so on a host that does not close the connection itself the upload keeps arriving and the socket stays occupied — the size bound refuses the request but does not stop the transfer.

Options

  1. Read request.body directly instead of a clone. The clone exists so the untouched body can still be handed on, but on the refusal path there is no "on" — the function returns null and the caller answers 413. Cancelling the real body there costs nothing.
  2. Keep the clone and cancel both branches on the refusal path.
  3. Leave the cancel to the adapter and drop the misleading reader.cancel() — the honest version of the current behaviour.

(1) is the smallest and makes the line mean what it looks like. A mutation to request.body.getReader() flips uploadSourceCancelled to true with the size-bound tests still green, so the shape is viable — though whether the clone is load-bearing elsewhere on that path is your call.

Regression test

it("cancels the upload source when refusing an oversized body", async () => {
  let cancelled = false;
  const response = await handleServerFunctionRequest(oversizedUpload(id, () => { cancelled = true; }));
  expect(response.status).toBe(413);
  expect(cancelled).toBe(true);
});

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions