fix(server): transfer request bodies into NextRequest instead of teeing

[NathanDrake2406]

Overview

Goal	Stop retaining whole request bodies in memory when the runtime wraps an incoming Request
Core change	NextRequest transfers the body (super(input, init), matching Next.js) rather than input.clone()
Key boundary	Only code that genuinely needs two live branches clones, and it owns cancelling the branch it does not consume
Expected impact	Streaming route handlers stay O(1) memory; a remote unauthenticated client can no longer force full-body buffering

Why

Request.clone() is specified to tee() the body stream, and tee applies no backpressure to the slower branch: whenever one branch pulls a chunk from the source, that chunk is also enqueued into the other branch's queue. If a branch is never read, its queue grows to the full body size. Dropping the reference does not help, because the tee closure holds both branch controllers and stays reachable from the branch that is being read.

The NextRequest constructor cloned every body-bearing input. Ordinary App Router route handling wraps the incoming request through createTrackedAppRouteRequest, so the branch left behind on the caller-owned request was never read and never cancelled. A handler that streams an upload straight to storage, which should hold no more than one chunk at a time, instead retained the entire body for the life of the request. That is remotely reachable, unauthenticated on any public body-bearing route, and repeatable until the process or Worker isolate runs out of memory.

The clone also disabled an existing mitigation. executeMiddleware already cancels the middleware body branch in a finally (added in #2026). The constructor's clone inserted a second tee between that cancel and the branch actually accumulating chunks, so the cancel released a branch nothing was filling while the real one kept growing one level up. Removing the clone reconnects that cancel to the stream it was written for.

Upstream Next.js does super(input, init) in the same constructor, so transferring is also the parity behaviour. The isolation added in #1132 is preserved: the callers that need the source request to stay readable already clone explicitly before wrapping.

Area	Principle / invariant	What this PR changes
NextRequest constructor	A wrapper takes ownership of the body it wraps	Transfers the body instead of teeing it
createTrackedAppRouteRequest basePath re-prefix	Do not branch a stream whose source is discarded	Builds the prefixed request directly from the incoming one
executeMiddleware	The one place with two genuinely live branches cancels the one it does not consume	Unchanged; its existing cancel becomes effective again

What changed

Scenario	Before	After
Route handler streams a body-bearing request	Whole body retained on an unread tee branch	Streams through, nothing retained
Route handler under a configured basePath	Second dead tee added while re-adding the prefix	Body transferred into the prefixed request
Middleware matches and never reads the body	Full body buffered despite the finally cancel	Branch released, downstream streams normally
Middleware reads the body, then NextResponse.next()	Downstream still reads the body (#1132)	Unchanged
Userland new NextRequest(sourceRequest)	Source stayed readable, diverging from Next.js	Source body is transferred, matching Next.js
Userland request.clone() inside a handler	Standard Request tee semantics	Unchanged; branching stays the caller's choice

Maintainer review path

1. packages/vinext/src/shims/server.ts for the ownership decision in the constructor.
2. packages/vinext/src/server/app-route-handler-runtime.ts for the dead tee on the basePath path.
3. packages/vinext/src/server/middleware-runtime.ts (unchanged) for the clone that is genuinely needed and the finally that cancels it.
4. tests/shims.test.ts and tests/app-route-handler-runtime.test.ts for the regression proofs.

Validation

• Reproduced the leak against the built package with a 128 MiB streamed body, reading and discarding chunk by chunk, holding the source request alive across the measurement the way a real call frame does.
• Confirmed the same retention through applyAppMiddleware for middleware that does not touch the body.
• Updated tests/shims.test.ts to pin the constructor as transferring rather than teeing, and added coverage in tests/app-route-handler-runtime.test.ts for the basePath path. Both fail if a clone is reintroduced.
• The fix(app-router): isolate middleware request bodies #1132 regression test (middleware reads the body, downstream still reads it) continues to pass unchanged.
• Ran tests/shims.test.ts, tests/app-route-handler-runtime.test.ts, tests/app-router-middleware-next-request.test.ts, tests/middleware-runtime.test.ts, tests/app-server-action-execution.test.ts, tests/app-route-handler-execution.test.ts, tests/app-post-middleware-context.test.ts, tests/routing.test.ts, tests/pages-api-route.test.ts, tests/api-handler.test.ts, tests/app-route-handler-dispatch.test.ts, tests/app-route-handler-policy.test.ts, tests/app-router-production-server.test.ts, plus vp check on the changed files.

Measurements

Node 26.5.0, 128 chunks of 1 MiB, --expose-gc, delta measured around a full streaming read.

before
  baseline (no wrap)           read=128.0MiB  arrayBuffers +0.0MiB
  new NextRequest(source)      read=128.0MiB  arrayBuffers +128.0MiB

after
  baseline (no wrap)           read=128.0MiB  arrayBuffers +0.0MiB
  new NextRequest(source)      read=128.0MiB  arrayBuffers +0.0MiB

Through applyAppMiddleware, downstream draining the request body:

before
  middleware ignores body   downstream=128.0MiB  arrayBuffers +128.0MiB
  middleware reads body     downstream=128.0MiB  arrayBuffers +0.0MiB

after
  middleware ignores body   downstream=128.0MiB  arrayBuffers +0.0MiB
  middleware reads body     downstream=128.0MiB  arrayBuffers +0.0MiB

A second symptom disappears with the fix. Because a tee only releases its source once every branch has cancelled, cancelling the downstream body while the middleware branch was still live returned a promise that never settled. After the fix it settles and the source stream's cancel() runs.

Risk / compatibility

• Public API: new NextRequest(request) now disturbs the source request's body, which is what Next.js does. Code that wrapped a request and then read the original is relying on a vinext-only divergence and must clone explicitly. No in-tree caller does this.
• Runtime: no new allocation, no new async work. One tee removed from every body-bearing request, and a second from the basePath path.
• Middleware isolation from fix(app-router): isolate middleware request bodies #1132 is unchanged, and is still covered by its original test.

Non-goals

• No framework-level request body size limit is introduced here. This PR removes the amplification the framework was adding; capping upload size remains a deployment and userland concern.

References

Reference	Why it matters
Next.js NextRequest	Upstream does super(input, init), the behaviour this restores
Streams spec, ReadableStreamTee	Defines the unbounded queueing on the unread branch, and that the source is only cancelled once every branch cancels
#1132	Introduced the clone to isolate middleware body reads; that isolation is preserved
#2026	Added the middleware branch cancel that the clone had been defeating

[NathanDrake2406]

@codex review

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@2741

npm i https://pkg.pr.new/create-vinext-app@2741

npm i https://pkg.pr.new/@vinext/types@2741

npm i https://pkg.pr.new/vinext@2741

commit: de9644f

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: a87a55996b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[github-actions]

Performance benchmarks

Compared de9644f against base 25dc2f3 using alternating same-runner rounds. Next.js was unchanged and skipped.

1 improved · 0 regressed · 5 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	134.3 KB	134.3 KB	⚫ +0.0%
Client entry size (gzip)	vinext	121.9 KB	121.9 KB	⚫ +0.0%
Dev server cold start	vinext	2.90 s	2.83 s	🟢 -2.4%
Production build time	vinext	3.19 s	3.18 s	⚫ -0.6%
RSC entry closure size (gzip)	vinext	111.2 KB	111.3 KB	⚫ +0.1%
Server bundle size (gzip)	vinext	188.9 KB	189.0 KB	⚫ +0.1%

View detailed results and traces

_{🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head}

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Reviewed the full change: NextRequest transferring the body (matching Next.js super(input, init)), the removal of dead tee branches on the basePath / RSC-normalization paths, and the reworked cancellation ownership in applyAppMiddleware / executeMiddleware.

Correctness looks solid. I traced the App Router body topology end to end:

• app-rsc-handler.ts:653 creates the single genuine tee (request.clone()) only when middleware will run and a body is present. Branch A → isolatedMiddlewareRequest (middleware), branch B → request (downstream action/route handler at :983). Both branches are consumed, so no branch is left buffering. Verified via the new uses one isolated middleware branch for body-bearing RSC actions test (exactly one clone, both bodyUsed).
• URL-normalization helpers (requestWithoutRscSuffix, requestWithoutRscCacheBustingSearchParam, createTrackedAppRouteRequest basePath re-prefix) now transfer instead of teeing. Since the source alias is discarded immediately, this removes real dead-branch buffering without changing observable behavior.
• Cancellation ownership is covered on every early-return path: matcher miss / validation failure (app-middleware.ts:281 finally), middleware-returns-response (:285 + app-rsc-handler.ts:681), forwarded-context-applied (:325), and external rewrite (:233). The raw downstream branch is always cancelled by app-rsc-handler.ts:681 for any kind === "response".

The executeMiddleware finally change is the highest-risk part — it flips from NODE_ENV !== "development" to always-cancel (except when the body was transferred into the response). I checked the Pages Router prod and dev callers (pages-request-pipeline.ts:391 → middleware.ts:171): they pass requestBodyAlreadyIsolated: false, so createNextRequest still clones and downstream keeps reading the original request's tee branch. Cancelling the middleware clone branch is safe there (tee source survives while the other branch reads), and in dev it fixes a latent buffering leak. The response?.body !== nextRequest.body guard correctly preserves the new Response(request.body) passthrough case.

Ran locally, all green:

• tests/middleware-runtime.test.ts, tests/app-route-handler-runtime.test.ts (32)
• tests/app-rsc-handler.test.ts, tests/shims.test.ts (1406)
• tests/entry-templates.test.ts, tests/app-router-middleware-next-request.test.ts, tests/app-post-middleware-context.test.ts (73)
• vp check on all 5 changed source files: clean

No blocking issues. Two minor, non-blocking observations below.

[ask-bonk]

Non-blocking: for a non-RSC request that reaches this path with no suffix and no _rsc param, userlandRequest === request (no normalization Request is constructed). In that case applyAppMiddleware already calls cancelRequestBody(options.request) at app-middleware.ts:285 on the same body, and this block calls .cancel() again. It's harmless — cancel() is idempotent and the rejection is swallowed by .catch(() => {}) — but the double-cancel intent isn't obvious from either site. Consider a short comment noting the two cancel sites can target the same stream when no URL alias was created, so a future reader doesn't try to "fix" one of them.

Also a small consistency nit: this inline if (request.body && !request.body.locked) { void request.body.cancel()... } duplicates the cancelRequestBody helper defined in app-middleware.ts. If a shared helper is reasonable to import/colocate, using it here would keep the cancel semantics in one place.

[ask-bonk]

The switch from NODE_ENV !== "development" to always-cancel is the behavioral crux of this PR and is correct as far as I can trace, but the original dev exclusion was introduced in #2026 (8b53905) without a stated reason for the dev carve-out. The identity guard response?.body !== nextRequest.body handles the direct-passthrough case, and the new preserves a request body transferred into the middleware response in %s test pins both envs. One case the identity check does not cover: middleware that does new Response(request.clone().body) — response.body is then a tee branch, not identical to nextRequest.body, so this cancels nextRequest.body. With tee semantics the source survives (the response branch is still live), so it's safe, just re-introduces branch buffering for that unusual userland pattern. Worth a one-line comment acknowledging the guard is identity-based, not reachability-based, so the buffering tradeoff for cloned-in-middleware bodies is a documented choice rather than a surprise.

[ask-bonk]

Review posted successfully. Here's a summary of my findings.

Review Summary

PR #2741 changes NextRequest to transfer request bodies (matching Next.js super(input, init)) instead of input.clone(), which was teeing the stream and leaving an unread branch that buffered the entire body in memory — a remotely-reachable, unauthenticated memory-amplification vector on any body-bearing route.

Verdict: No blocking issues. The change is well-designed and correct.

What I verified

• App Router body topology (end to end): the single genuine tee is created at app-rsc-handler.ts:653 only when middleware runs and a body exists — one branch to middleware, one to the downstream action/route handler, both consumed. No branch is left buffering.
• Dead tee removal on the RSC/basePath URL-normalization paths is safe because the source alias is discarded immediately.
• Cancellation ownership is covered on every early-return path (matcher miss, middleware-returns-response, forwarded-context-applied, external rewrite).
• The executeMiddleware finally change (from NODE_ENV !== "development" to always-cancel, guarded by response?.body !== nextRequest.body) is the highest-risk part. I traced the Pages Router prod + dev callers and confirmed they still clone (requestBodyAlreadyIsolated: false), so cancelling the middleware clone branch is safe and actually fixes a latent dev-only leak.

Local validation (all green)

• middleware-runtime, app-route-handler-runtime (32 tests)
• app-rsc-handler, shims (1406 tests)
• entry-templates, app-router-middleware-next-request, app-post-middleware-context (73 tests)
• vp check on all 5 changed source files: clean

Two non-blocking notes I left inline

1. app-rsc-handler.ts:681 — for a non-RSC request with no URL alias, the same body can be .cancel()'d at both this site and app-middleware.ts:285. Harmless (idempotent), but worth a comment; also duplicates the cancelRequestBody helper.
2. middleware-runtime.ts:355 — the guard is identity-based, not reachability-based, so a new Response(request.clone().body) pattern in userland would re-introduce branch buffering. Safe, but worth documenting as a deliberate tradeoff.

github run