prefetch #938

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/906
Author: @NathanDrake2406
Created: 4/26/2026
Status: ✅ Merged
Merged: 4/26/2026
Merged by: @james-elicx

Base: main ← Head: nathan/router-block-javascript-uri

📝 Commits (4)

fb819fa fix(app-router): block javascript: URLs in router.push/replace/prefetch
1ed1b48 test(router): split ported vs vinext-only dangerous-scheme cases
5cb5af3 fix app router dangerous redirect gaps
520e8be test address dangerous URL review nits

📊 Changes

12 files changed (+295 additions, -5 deletions)

View changed files

📝 packages/vinext/src/server/app-browser-entry.ts (+6 -0)
📝 packages/vinext/src/shims/navigation.ts (+4 -0)
📝 packages/vinext/src/shims/server.ts (+17 -4)
📝 packages/vinext/src/shims/url-safety.ts (+9 -0)
➕ tests/e2e/app-router/nextjs-compat/javascript-urls.spec.ts (+98 -0)
➕ tests/fixtures/app-basic/app/nextjs-compat/javascript-urls/action-redirect-form/page.tsx (+22 -0)
➕ tests/fixtures/app-basic/app/nextjs-compat/javascript-urls/action-redirect-onclick/page.tsx (+20 -0)
➕ tests/fixtures/app-basic/app/nextjs-compat/javascript-urls/bad-url.ts (+2 -0)
➕ tests/fixtures/app-basic/app/nextjs-compat/javascript-urls/boom/page.tsx (+3 -0)
➕ tests/fixtures/app-basic/app/nextjs-compat/javascript-urls/safe/page.tsx (+3 -0)
➕ tests/router-javascript-urls.test.ts (+93 -0)
📝 tests/shims.test.ts (+18 -1)

📄 Description

What this changes

This PR blocks dangerous navigation URLs across the App Router surfaces that can reach browser navigation sinks:

useRouter().push, .replace, and .prefetch reject dangerous URI schemes.
Server Action redirect("javascript:...") responses are blocked before the browser entry assigns to window.location.*.
NextResponse.redirect() and NextResponse.rewrite() now validate URLs before writing Location / x-middleware-rewrite headers.

The block message intentionally matches Next.js exactly:

Next.js has blocked a javascript: URL as a security precaution.

Why

router.push("javascript:alert(1)") previously flowed into navigateClientSide, was classified as external, and reached window.location.assign, which browsers treat as script execution.

After the initial router fix, two related paths were still open:

Server Action redirects are returned through x-action-redirect; the browser entry consumed that header and sent it directly to window.location.href, .assign(), or .replace().
NextResponse.redirect() / rewrite() wrote raw userland input into framework routing headers. This differed from Next.js, which validates these helper URLs and rejects relative or malformed values.

Next.js references

Router dangerous URL blocking:

push: packages/next/src/client/components/app-router-instance.ts#L343
inner push helper: packages/next/src/client/components/app-router-instance.ts#L400
replace: packages/next/src/client/components/app-router-instance.ts#L440
prefetch: packages/next/src/client/components/app-router-instance.ts#L458
hard navigation / Server Action redirect block: packages/next/src/client/components/segment-cache/navigation.ts#L536
shared matcher: packages/next/src/client/lib/javascript-url.ts

NextResponse URL validation:

helper implementation: packages/next/src/server/web/spec-extension/response.ts#L117-L140
validateURL: packages/next/src/server/web/utils.ts#L141-L152
relative URL tests: test/e2e/middleware-general/test/index.test.ts#L721-L735
server-action javascript URL tests: test/e2e/app-dir/javascript-urls/javascript-urls.test.ts#L200-L250

Approach

The dangerous-scheme error message and assertion now live in shims/url-safety.ts, so next/navigation, next/server, and the browser Server Action redirect sink share one policy.

NextResponse.redirect() / rewrite() now mirror Next.js by passing URLs through new URL(String(url)). That rejects relative and malformed URLs and normalizes valid absolute URLs before header write.

The Server Action redirect consumer checks x-action-redirect before any window.location.* call. If blocked, it logs the Next.js message and returns without navigating, matching the upstream hard-navigation behavior.

Deliberate divergence from Next.js

Vinext already treats data: and vbscript: as dangerous in shims/url-safety.ts, and <Link> / <Form> use that same helper. This PR keeps that existing defense-in-depth policy for the added surfaces too.

Next.js's validateURL() rejects relative/malformed URLs but does not reject javascript: by itself, because new URL("javascript:...") is syntactically valid. Vinext applies the shared dangerous-scheme guard before URL validation so all navigation sinks stay aligned.

Validation

vp test run tests/shims.test.ts -t "relative URL"
vp test run tests/router-javascript-urls.test.ts tests/url-safety.test.ts
PLAYWRIGHT_PROJECT=app-router vp run test:e2e tests/e2e/app-router/nextjs-compat/javascript-urls.spec.ts
vp check <changed files>
vp run vinext#build

Full vp check currently fails on an unrelated local-only ignored note file under nathan/, not on this PR's changed files.

_{🔄 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/906 **Author:** [@NathanDrake2406](https://github.com/NathanDrake2406) **Created:** 4/26/2026 **Status:** ✅ Merged **Merged:** 4/26/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `nathan/router-block-javascript-uri` --- ### 📝 Commits (4) - [`fb819fa`](https://github.com/cloudflare/vinext/commit/fb819faf2c3bc597cc2574c3113612d591e6a86f) fix(app-router): block javascript: URLs in router.push/replace/prefetch - [`1ed1b48`](https://github.com/cloudflare/vinext/commit/1ed1b48f1b1a2b93fbe43ed7297338c7b00fbaae) test(router): split ported vs vinext-only dangerous-scheme cases - [`5cb5af3`](https://github.com/cloudflare/vinext/commit/5cb5af37af1aa7da851f7a74a8eb6e70630c81fc) fix app router dangerous redirect gaps - [`520e8be`](https://github.com/cloudflare/vinext/commit/520e8be15593afe47ff04ee7a62e5b9d2540e538) test address dangerous URL review nits ### 📊 Changes **12 files changed** (+295 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/server/app-browser-entry.ts` (+6 -0) 📝 `packages/vinext/src/shims/navigation.ts` (+4 -0) 📝 `packages/vinext/src/shims/server.ts` (+17 -4) 📝 `packages/vinext/src/shims/url-safety.ts` (+9 -0) ➕ `tests/e2e/app-router/nextjs-compat/javascript-urls.spec.ts` (+98 -0) ➕ `tests/fixtures/app-basic/app/nextjs-compat/javascript-urls/action-redirect-form/page.tsx` (+22 -0) ➕ `tests/fixtures/app-basic/app/nextjs-compat/javascript-urls/action-redirect-onclick/page.tsx` (+20 -0) ➕ `tests/fixtures/app-basic/app/nextjs-compat/javascript-urls/bad-url.ts` (+2 -0) ➕ `tests/fixtures/app-basic/app/nextjs-compat/javascript-urls/boom/page.tsx` (+3 -0) ➕ `tests/fixtures/app-basic/app/nextjs-compat/javascript-urls/safe/page.tsx` (+3 -0) ➕ `tests/router-javascript-urls.test.ts` (+93 -0) 📝 `tests/shims.test.ts` (+18 -1) </details> ### 📄 Description ## What this changes This PR blocks dangerous navigation URLs across the App Router surfaces that can reach browser navigation sinks: - `useRouter().push`, `.replace`, and `.prefetch` reject dangerous URI schemes. - Server Action `redirect("javascript:...")` responses are blocked before the browser entry assigns to `window.location.*`. - `NextResponse.redirect()` and `NextResponse.rewrite()` now validate URLs before writing `Location` / `x-middleware-rewrite` headers. The block message intentionally matches Next.js exactly: `Next.js has blocked a javascript: URL as a security precaution.` ## Why `router.push("javascript:alert(1)")` previously flowed into `navigateClientSide`, was classified as external, and reached `window.location.assign`, which browsers treat as script execution. After the initial router fix, two related paths were still open: - Server Action redirects are returned through `x-action-redirect`; the browser entry consumed that header and sent it directly to `window.location.href`, `.assign()`, or `.replace()`. - `NextResponse.redirect()` / `rewrite()` wrote raw userland input into framework routing headers. This differed from Next.js, which validates these helper URLs and rejects relative or malformed values. ## Next.js references Router dangerous URL blocking: - `push`: [packages/next/src/client/components/app-router-instance.ts#L343](https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/app-router-instance.ts#L343) - inner push helper: [packages/next/src/client/components/app-router-instance.ts#L400](https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/app-router-instance.ts#L400) - `replace`: [packages/next/src/client/components/app-router-instance.ts#L440](https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/app-router-instance.ts#L440) - `prefetch`: [packages/next/src/client/components/app-router-instance.ts#L458](https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/app-router-instance.ts#L458) - hard navigation / Server Action redirect block: [packages/next/src/client/components/segment-cache/navigation.ts#L536](https://github.com/vercel/next.js/blob/canary/packages/next/src/client/components/segment-cache/navigation.ts#L536) - shared matcher: [packages/next/src/client/lib/javascript-url.ts](https://github.com/vercel/next.js/blob/canary/packages/next/src/client/lib/javascript-url.ts) `NextResponse` URL validation: - helper implementation: [packages/next/src/server/web/spec-extension/response.ts#L117-L140](https://github.com/vercel/next.js/blob/canary/packages/next/src/server/web/spec-extension/response.ts#L117-L140) - `validateURL`: [packages/next/src/server/web/utils.ts#L141-L152](https://github.com/vercel/next.js/blob/canary/packages/next/src/server/web/utils.ts#L141-L152) - relative URL tests: [test/e2e/middleware-general/test/index.test.ts#L721-L735](https://github.com/vercel/next.js/blob/canary/test/e2e/middleware-general/test/index.test.ts#L721-L735) - server-action javascript URL tests: [test/e2e/app-dir/javascript-urls/javascript-urls.test.ts#L200-L250](https://github.com/vercel/next.js/blob/canary/test/e2e/app-dir/javascript-urls/javascript-urls.test.ts#L200-L250) ## Approach The dangerous-scheme error message and assertion now live in `shims/url-safety.ts`, so `next/navigation`, `next/server`, and the browser Server Action redirect sink share one policy. `NextResponse.redirect()` / `rewrite()` now mirror Next.js by passing URLs through `new URL(String(url))`. That rejects relative and malformed URLs and normalizes valid absolute URLs before header write. The Server Action redirect consumer checks `x-action-redirect` before any `window.location.*` call. If blocked, it logs the Next.js message and returns without navigating, matching the upstream hard-navigation behavior. ## Deliberate divergence from Next.js Vinext already treats `data:` and `vbscript:` as dangerous in `shims/url-safety.ts`, and `<Link>` / `<Form>` use that same helper. This PR keeps that existing defense-in-depth policy for the added surfaces too. Next.js's `validateURL()` rejects relative/malformed URLs but does not reject `javascript:` by itself, because `new URL("javascript:...")` is syntactically valid. Vinext applies the shared dangerous-scheme guard before URL validation so all navigation sinks stay aligned. ## Validation - `vp test run tests/shims.test.ts -t "relative URL"` - `vp test run tests/router-javascript-urls.test.ts tests/url-safety.test.ts` - `PLAYWRIGHT_PROJECT=app-router vp run test:e2e tests/e2e/app-router/nextjs-compat/javascript-urls.spec.ts` - `vp check <changed files>` - `vp run vinext#build` Full `vp check` currently fails on an unrelated local-only ignored note file under `nathan/`, not on this PR's changed files. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>