[PR #907] [MERGED] fix(app-router): fixing cache request leaks #936

New issue

Closed

opened 2026-05-06 13:10:57 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:10:57 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/907
Author: @NathanDrake2406
Created: 4/26/2026
Status: ✅ Merged
Merged: 4/29/2026
Merged by: @james-elicx

Base: main ← Head: nathan/cache-isolation-fixes

📝 Commits (1)

f0f741a fix app router cache request leaks

📊 Changes

18 files changed (+1054 additions, -151 deletions)

View changed files

📝 packages/vinext/src/entries/app-rsc-entry.ts (+30 -14)
📝 packages/vinext/src/server/app-page-cache.ts (+36 -1)
📝 packages/vinext/src/server/app-page-render.ts (+3 -4)
📝 packages/vinext/src/server/app-route-handler-cache.ts (+6 -0)
📝 packages/vinext/src/server/app-route-handler-execution.ts (+35 -3)
📝 packages/vinext/src/server/app-route-handler-runtime.ts (+172 -1)
➕ packages/vinext/src/server/app-static-generation.ts (+54 -0)
📝 packages/vinext/src/shims/headers.ts (+9 -1)
📝 packages/vinext/src/shims/server.ts (+57 -0)
📝 tests/__snapshots__/entry-templates.test.ts.snap (+156 -84)
📝 tests/app-page-cache.test.ts (+106 -0)
📝 tests/app-page-render.test.ts (+2 -2)
📝 tests/app-route-handler-cache.test.ts (+74 -0)
📝 tests/app-route-handler-execution.test.ts (+56 -0)
📝 tests/app-route-handler-runtime.test.ts (+166 -0)
➕ tests/app-static-generation.test.ts (+47 -0)
📝 tests/e2e/app-router/rsc-fetch-errors.spec.ts (+40 -41)
➕ tests/fixtures/app-basic/app/rsc-fetch-redirect-src/page.tsx (+5 -0)

📄 Description

What this changes

Fixes two App Router cache-safety issues around request-specific data:

Enforces dynamic = "force-static" and dynamic = "error" for route-handler request APIs by stubbing or throwing for direct NextRequest fields and next/headers.
Tracks direct request.ip and request.geo access as dynamic in auto mode.
Re-checks dynamic usage after HTML stream consumption before writing ISR HTML/RSC cache entries.

Why

Route handlers could previously expose request-specific IP/geo/header/cookie data through untracked request fields, and HTML ISR could cache page output before lazy streamed Server Components had finished touching cookies() or headers(). Both cases could put per-request output into shared cache.

Approach

This mirrors Next.js' static-generation request policy instead of treating dynamic API access as observation-only state:

force-static route handlers receive empty readonly stubs for request-specific APIs.
dynamic = "error" throws when request-specific APIs are touched during static rendering.
auto-mode direct request fields are tracked as dynamic instead of silently flowing into cached output.
cache writes happen only after the relevant stream has been consumed and dynamic usage has been checked again.

Next.js references

Checked against Next.js canary commit ae61573e062e900050b8e6b24626e450accc4570:

App route modules set workStore.forceStatic for dynamic = "force-static" and proxy the request through forceStaticRequestHandlers: github.com/vercel/next.js@ae61573e06/packages/next/src/server/route-modules/app-route/module.ts (L907-L913)
That proxy returns sealed empty headers and sealed empty cookies for request.headers and request.cookies: github.com/vercel/next.js@ae61573e06/packages/next/src/server/route-modules/app-route/module.ts (L1038-L1056)
headers() under forceStatic returns HeadersAdapter.seal(new Headers({})) without tracking, while dynamic = "error" throws StaticGenBailoutError: github.com/vercel/next.js@ae61573e06/packages/next/src/server/request/headers.ts (L58-L101)
cookies() under forceStatic returns sealed empty request cookies, while dynamic = "error" throws StaticGenBailoutError: github.com/vercel/next.js@ae61573e06/packages/next/src/server/request/cookies.ts (L52-L62)
Next's readonly cookie adapter returns throwing callables for mutators, so method lookup is safe but invocation fails: github.com/vercel/next.js@ae61573e06/packages/next/src/server/web/spec-extension/adapters/request-cookies.ts (L36-L49)
Next's readonly headers adapter uses the same throwing-callable shape for append, delete, and set: github.com/vercel/next.js@ae61573e06/packages/next/src/server/web/spec-extension/adapters/headers.ts (L117-L132)
Next consumes dynamic access after prerender/static stream work before returning the result metadata: github.com/vercel/next.js@ae61573e06/packages/next/src/server/app-render/app-render.tsx (L7967-L7974)

Validation

vp check
vp test run tests/app-route-handler-runtime.test.ts tests/app-route-handler-execution.test.ts tests/app-route-handler-cache.test.ts tests/app-route-handler-policy.test.ts tests/app-page-cache.test.ts tests/app-page-render.test.ts tests/app-page-response.test.ts tests/app-page-probe.test.ts tests/shims.test.ts
vp test run tests/app-router.test.ts
vp run vinext#build
GitHub CI passed on the latest pushed branch

Risks / follow-ups

The behavior is deliberately stricter around cacheable App Router work. The main compatibility risk is code that relied on reading real request fields under dynamic = "force-static", which conflicts with Next.js' empty-stub behavior and can leak per-request data into shared cache.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/cloudflare/vinext/pull/907 **Author:** [@NathanDrake2406](https://github.com/NathanDrake2406) **Created:** 4/26/2026 **Status:** ✅ Merged **Merged:** 4/29/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `nathan/cache-isolation-fixes` --- ### 📝 Commits (1) - [`f0f741a`](https://github.com/cloudflare/vinext/commit/f0f741a5e5e3459a586f502279dd73e7975c12fe) fix app router cache request leaks ### 📊 Changes **18 files changed** (+1054 additions, -151 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/entries/app-rsc-entry.ts` (+30 -14) 📝 `packages/vinext/src/server/app-page-cache.ts` (+36 -1) 📝 `packages/vinext/src/server/app-page-render.ts` (+3 -4) 📝 `packages/vinext/src/server/app-route-handler-cache.ts` (+6 -0) 📝 `packages/vinext/src/server/app-route-handler-execution.ts` (+35 -3) 📝 `packages/vinext/src/server/app-route-handler-runtime.ts` (+172 -1) ➕ `packages/vinext/src/server/app-static-generation.ts` (+54 -0) 📝 `packages/vinext/src/shims/headers.ts` (+9 -1) 📝 `packages/vinext/src/shims/server.ts` (+57 -0) 📝 `tests/__snapshots__/entry-templates.test.ts.snap` (+156 -84) 📝 `tests/app-page-cache.test.ts` (+106 -0) 📝 `tests/app-page-render.test.ts` (+2 -2) 📝 `tests/app-route-handler-cache.test.ts` (+74 -0) 📝 `tests/app-route-handler-execution.test.ts` (+56 -0) 📝 `tests/app-route-handler-runtime.test.ts` (+166 -0) ➕ `tests/app-static-generation.test.ts` (+47 -0) 📝 `tests/e2e/app-router/rsc-fetch-errors.spec.ts` (+40 -41) ➕ `tests/fixtures/app-basic/app/rsc-fetch-redirect-src/page.tsx` (+5 -0) </details> ### 📄 Description ## What this changes Fixes two App Router cache-safety issues around request-specific data: - Enforces `dynamic = "force-static"` and `dynamic = "error"` for route-handler request APIs by stubbing or throwing for direct `NextRequest` fields and `next/headers`. - Tracks direct `request.ip` and `request.geo` access as dynamic in auto mode. - Re-checks dynamic usage after HTML stream consumption before writing ISR HTML/RSC cache entries. ## Why Route handlers could previously expose request-specific IP/geo/header/cookie data through untracked request fields, and HTML ISR could cache page output before lazy streamed Server Components had finished touching `cookies()` or `headers()`. Both cases could put per-request output into shared cache. ## Approach This mirrors Next.js' static-generation request policy instead of treating dynamic API access as observation-only state: - `force-static` route handlers receive empty readonly stubs for request-specific APIs. - `dynamic = "error"` throws when request-specific APIs are touched during static rendering. - auto-mode direct request fields are tracked as dynamic instead of silently flowing into cached output. - cache writes happen only after the relevant stream has been consumed and dynamic usage has been checked again. ## Next.js references Checked against Next.js canary commit `ae61573e062e900050b8e6b24626e450accc4570`: - App route modules set `workStore.forceStatic` for `dynamic = "force-static"` and proxy the request through `forceStaticRequestHandlers`: https://github.com/vercel/next.js/blob/ae61573e062e900050b8e6b24626e450accc4570/packages/next/src/server/route-modules/app-route/module.ts#L907-L913 - That proxy returns sealed empty headers and sealed empty cookies for `request.headers` and `request.cookies`: https://github.com/vercel/next.js/blob/ae61573e062e900050b8e6b24626e450accc4570/packages/next/src/server/route-modules/app-route/module.ts#L1038-L1056 - `headers()` under `forceStatic` returns `HeadersAdapter.seal(new Headers({}))` without tracking, while `dynamic = "error"` throws `StaticGenBailoutError`: https://github.com/vercel/next.js/blob/ae61573e062e900050b8e6b24626e450accc4570/packages/next/src/server/request/headers.ts#L58-L101 - `cookies()` under `forceStatic` returns sealed empty request cookies, while `dynamic = "error"` throws `StaticGenBailoutError`: https://github.com/vercel/next.js/blob/ae61573e062e900050b8e6b24626e450accc4570/packages/next/src/server/request/cookies.ts#L52-L62 - Next's readonly cookie adapter returns throwing callables for mutators, so method lookup is safe but invocation fails: https://github.com/vercel/next.js/blob/ae61573e062e900050b8e6b24626e450accc4570/packages/next/src/server/web/spec-extension/adapters/request-cookies.ts#L36-L49 - Next's readonly headers adapter uses the same throwing-callable shape for `append`, `delete`, and `set`: https://github.com/vercel/next.js/blob/ae61573e062e900050b8e6b24626e450accc4570/packages/next/src/server/web/spec-extension/adapters/headers.ts#L117-L132 - Next consumes dynamic access after prerender/static stream work before returning the result metadata: https://github.com/vercel/next.js/blob/ae61573e062e900050b8e6b24626e450accc4570/packages/next/src/server/app-render/app-render.tsx#L7967-L7974 ## Validation - `vp check` - `vp test run tests/app-route-handler-runtime.test.ts tests/app-route-handler-execution.test.ts tests/app-route-handler-cache.test.ts tests/app-route-handler-policy.test.ts tests/app-page-cache.test.ts tests/app-page-render.test.ts tests/app-page-response.test.ts tests/app-page-probe.test.ts tests/shims.test.ts` - `vp test run tests/app-router.test.ts` - `vp run vinext#build` - GitHub CI passed on the latest pushed branch ## Risks / follow-ups The behavior is deliberately stricter around cacheable App Router work. The main compatibility risk is code that relied on reading real request fields under `dynamic = "force-static"`, which conflicts with Next.js' empty-stub behavior and can leak per-request data into shared cache. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>