From 2b3c58a413b87e416d982320435e7e8729b49e5d Mon Sep 17 00:00:00 2001 From: James Date: Thu, 28 May 2026 13:58:31 +0100 Subject: [PATCH 1/2] fix(i18n): serve public files after rewrites land on them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rewrites whose destination resolves to a public/ asset (e.g. `{ source: '/:locale/rewrite-files/:path*', destination: '/:path*', locale: false }` per Next.js's i18n-ignore-rewrite-source-locale fixture) returned the 404 page because the Pages Router pipeline only served public files once — before any rewrite ran. Mirrors Next.js's `check_fs` slot in resolve-routes.ts, which runs between beforeFiles and afterFiles rewrites. - pages-server-entry: scan the project's public/ directory at build time and embed the file list in `vinextConfig.publicFiles`. - prod-server: re-run `tryServeStatic` after a beforeFiles rewrite fires so a rewrite to `/file.txt` lands on `dist/client/file.txt`. - deploy.ts (Workers entry): consume the embedded publicFiles set and short-circuit through `env.ASSETS.fetch` for any beforeFiles or afterFiles rewrite whose new pathname is a known public file. Tests: - entry-templates.test.ts: assert publicFiles is embedded (and empty when no project root is supplied). - deploy.test.ts: assert the worker template references publicFiles and emits the asset-fetch short-circuit on both rewrite stages. - pages-router.test.ts + pages-basic fixture: integration regression test for `/aliased-asset` -> `/static-asset.txt` in production. Refs #1336. --- packages/vinext/src/deploy.ts | 38 +++++++++++ .../vinext/src/entries/pages-server-entry.ts | 9 +++ packages/vinext/src/index.ts | 1 + packages/vinext/src/server/prod-server.ts | 24 +++++++ tests/deploy.test.ts | 29 +++++++++ tests/entry-templates.test.ts | 65 +++++++++++++++++++ tests/fixtures/pages-basic/next.config.mjs | 10 +++ .../pages-basic/public/static-asset.txt | 1 + tests/pages-router.test.ts | 12 ++++ 9 files changed, 189 insertions(+) create mode 100644 tests/fixtures/pages-basic/public/static-asset.txt diff --git a/packages/vinext/src/deploy.ts b/packages/vinext/src/deploy.ts index bcfb91658..276a27cfc 100644 --- a/packages/vinext/src/deploy.ts +++ b/packages/vinext/src/deploy.ts @@ -578,6 +578,12 @@ const i18nConfig = vinextConfig?.i18n ?? null; const configRedirects = vinextConfig?.redirects ?? []; const configRewrites = vinextConfig?.rewrites ?? { beforeFiles: [], afterFiles: [], fallback: [] }; const configHeaders = vinextConfig?.headers ?? []; +// Public/ files scanned at build time. Rewrites/redirects can land on these +// paths (issue #1336: \`source: '/:locale/rewrite-files/:path*'\` → +// \`destination: '/:path*'\` rewrites to public file paths), so the worker +// has to recognise them and fetch from \`env.ASSETS\` instead of falling +// through to \`renderPage\` (which returns 404 for unknown routes). +const publicFiles: ReadonlySet = new Set(vinextConfig?.publicFiles ?? []); const imageConfig: ImageConfig | undefined = vinextConfig?.images ? { dangerouslyAllowSVG: vinextConfig.images.dangerouslyAllowSVG, dangerouslyAllowLocalIP: vinextConfig.images.dangerouslyAllowLocalIP, @@ -595,6 +601,14 @@ function stripBasePath(pathname: string, basePath: string): string { return pathname.slice(basePath.length) || "/"; } +// Fetch a public/ file from the bound Workers Assets binding after a rewrite +// resolves to a static asset. The url constructed here preserves the worker +// origin so env.ASSETS treats the request as same-origin. +function fetchPublicAsset(env: Env, request: Request, pathname: string): Promise { + const assetUrl = new URL(pathname, request.url); + return Promise.resolve(env.ASSETS.fetch(new Request(assetUrl, { headers: request.headers }))); +} + export default { async fetch(request: Request, env: Env, ctx: ExecutionContext): Promise { try { @@ -855,6 +869,23 @@ export default { }); } + // ── 6b. Serve public/ file when a beforeFiles rewrite lands on one ── + // Next.js routing order (resolve-routes.ts) checks the filesystem (which + // includes public/) between beforeFiles and afterFiles rewrites. In a + // Worker, Cloudflare Assets serves direct hits to public/ paths BEFORE + // the worker runs, but a rewrite produces a path we never see directly. + // Without this branch a rewrite like + // { source: '/:locale/rewrite-files/:path*', destination: '/:path*', + // locale: false } + // (#1336: i18n-ignore-rewrite-source-locale) would 404 because + // \`renderPage\` doesn't know about public/ files. + if (configRewriteFired && publicFiles.has(resolvedPathname)) { + if (request.method === "GET" || request.method === "HEAD") { + const assetResponse = await fetchPublicAsset(env, request, resolvedPathname); + return mergeHeaders(assetResponse, middlewareHeaders, middlewareRewriteStatus); + } + } + // ── 7. API routes ───────────────────────────────────────────── // Forward ctx so handlePagesApiRoute can wrap the user handler in // runWithExecutionContext, making ctx.waitUntil() reachable from @@ -890,6 +921,13 @@ export default { } resolvedUrl = mergeRewriteQuery(resolvedUrl, rewritten); resolvedPathname = resolvedUrl.split("?")[0]; + // Same public/ short-circuit as the beforeFiles branch above. + if (publicFiles.has(resolvedPathname)) { + if (request.method === "GET" || request.method === "HEAD") { + const assetResponse = await fetchPublicAsset(env, request, resolvedPathname); + return mergeHeaders(assetResponse, middlewareHeaders, middlewareRewriteStatus); + } + } } } diff --git a/packages/vinext/src/entries/pages-server-entry.ts b/packages/vinext/src/entries/pages-server-entry.ts index 0992d9af7..77fd3d05c 100644 --- a/packages/vinext/src/entries/pages-server-entry.ts +++ b/packages/vinext/src/entries/pages-server-entry.ts @@ -14,6 +14,7 @@ import { createValidFileMatcher } from "../routing/file-matcher.js"; import { type ResolvedNextConfig } from "../config/next-config.js"; import { isProxyFile } from "../server/middleware.js"; import { findFileWithExts } from "./pages-entry-helpers.js"; +import { scanPublicFileRoutes } from "../utils/public-routes.js"; const _requestContextShimPath = resolveEntryPath("../shims/request-context.js", import.meta.url); const _middlewareRuntimePath = resolveEntryPath("../server/middleware-runtime.js", import.meta.url); @@ -43,9 +44,16 @@ export async function generateServerEntry( fileMatcher: ReturnType, middlewarePath: string | null, instrumentationPath: string | null, + projectRoot: string | null = null, ): Promise { const pageRoutes = await pagesRouter(pagesDir, nextConfig?.pageExtensions, fileMatcher); const apiRoutes = await apiRouter(pagesDir, nextConfig?.pageExtensions, fileMatcher); + // Scan the user's public/ directory once at build time. The resulting set + // lets the generated worker entry recognise rewrite/redirect destinations + // that resolve to a static asset (e.g. `/file.txt` under `public/`) so it + // can serve them from `env.ASSETS` instead of returning a 404. + // See issue #1336 (test/e2e/i18n-ignore-rewrite-source-locale). + const publicFiles = projectRoot ? scanPublicFileRoutes(projectRoot) : []; // Generate import statements using absolute paths since virtual // modules don't have a real file location for relative resolution. @@ -128,6 +136,7 @@ export async function generateServerEntry( // `.nextjs-ref/packages/next/src/pages/_document.tsx` getScripts(). disableOptimizedLoading: nextConfig?.disableOptimizedLoading === true, clientTraceMetadata: nextConfig?.clientTraceMetadata, + publicFiles, images: { deviceSizes: nextConfig?.images?.deviceSizes, imageSizes: nextConfig?.images?.imageSizes, diff --git a/packages/vinext/src/index.ts b/packages/vinext/src/index.ts index 9d585a62d..afe229ae6 100644 --- a/packages/vinext/src/index.ts +++ b/packages/vinext/src/index.ts @@ -712,6 +712,7 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] { fileMatcher, middlewarePath, instrumentationPath, + root, ); } diff --git a/packages/vinext/src/server/prod-server.ts b/packages/vinext/src/server/prod-server.ts index 1b5baa09c..bd67e0489 100644 --- a/packages/vinext/src/server/prod-server.ts +++ b/packages/vinext/src/server/prod-server.ts @@ -1886,6 +1886,30 @@ async function startPagesRouterServer(options: PagesRouterServerOptions) { } } + // ── 7a. Serve public/ file when a beforeFiles rewrite lands on one ── + // Mirrors Next.js's `check_fs` step between beforeFiles and afterFiles + // rewrites (resolve-routes.ts). Without this branch a rewrite like + // `{ source: '/:locale/rewrite-files/:path*', destination: '/:path*', + // locale: false }` (#1336) cannot reach the static file it targets, + // because step 5b only saw the pre-rewrite pathname. + if ( + configRewriteFired && + resolvedPathname !== "/" && + !resolvedPathname.startsWith("/api/") && + !resolvedPathname.startsWith(`/${ASSET_PREFIX_URL_DIR}/`) && + (await tryServeStatic( + req, + res, + clientDir, + resolvedPathname, + compress, + staticCache, + middlewareHeaders, + )) + ) { + return; + } + // ── 7b. Reject out-of-basePath requests that no rule rewrote ── // When `basePath` is configured and the request was outside it, // only `basePath: false` rules can keep it alive. If none matched, diff --git a/tests/deploy.test.ts b/tests/deploy.test.ts index 5d84486af..ec27ab457 100644 --- a/tests/deploy.test.ts +++ b/tests/deploy.test.ts @@ -627,6 +627,35 @@ describe("generatePagesRouterWorkerEntry", () => { expect(content).toContain('from "vinext/server/request-pipeline"'); }); + // Ported from Next.js: test/e2e/i18n-ignore-rewrite-source-locale/rewrites.test.ts + // — "get public file by skipping locale in rewrite". A rewrite with + // `locale: false` whose destination resolves to a `public/` file must + // serve the file via `env.ASSETS` after the rewrite fires. Without this + // branch the rewritten path (`/file.txt`) misses every page route and + // returns the 404 page instead of the asset contents (issue #1336). + it("serves public files after a beforeFiles rewrite lands on one", () => { + const content = generatePagesRouterWorkerEntry(); + // The worker template must read the build-time publicFiles list. + expect(content).toContain("vinextConfig?.publicFiles"); + expect(content).toContain("const publicFiles: ReadonlySet"); + // After the rewrite fires, the worker must short-circuit through + // env.ASSETS for any path whose pathname is in the publicFiles set. + expect(content).toMatch(/configRewriteFired\s*&&\s*publicFiles\.has\(resolvedPathname\)/); + expect(content).toContain("env.ASSETS.fetch"); + }); + + it("serves public files after an afterFiles rewrite lands on one", () => { + const content = generatePagesRouterWorkerEntry(); + // The afterFiles branch updates resolvedPathname when a rewrite matches; + // the same publicFiles short-circuit must apply there so an `afterFiles` + // rule that targets `/public-asset.png` is honoured too. + const afterFilesPos = content.indexOf("Apply afterFiles rewrites"); + expect(afterFilesPos).toBeGreaterThan(-1); + const afterFilesBlock = content.slice(afterFilesPos, afterFilesPos + 1200); + expect(afterFilesBlock).toContain("publicFiles.has(resolvedPathname)"); + expect(afterFilesBlock).toContain("fetchPublicAsset"); + }); + it("handles basePath stripping and creates a new request with stripped URL for middleware", () => { const content = generatePagesRouterWorkerEntry(); expect(content).toContain("basePath"); diff --git a/tests/entry-templates.test.ts b/tests/entry-templates.test.ts index b495e1ce0..4b15138ed 100644 --- a/tests/entry-templates.test.ts +++ b/tests/entry-templates.test.ts @@ -798,4 +798,69 @@ describe("Pages Router entry template", () => { fs.rmSync(tmpDir, { recursive: true, force: true }); } }); + + // Issue #1336 (test/e2e/i18n-ignore-rewrite-source-locale): rewrites whose + // destination resolves to a `public/` file need to serve the file via the + // Workers Assets binding. The generated server entry embeds the public-file + // list at build time so the worker can detect those rewrites at runtime + // without re-scanning the filesystem. + it("embeds the project's public/ file list in vinextConfig.publicFiles", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "vinext-pages-public-")); + const pagesDir = path.join(tmpDir, "pages"); + const publicDir = path.join(tmpDir, "public"); + + try { + fs.mkdirSync(pagesDir, { recursive: true }); + fs.mkdirSync(publicDir, { recursive: true }); + fs.writeFileSync( + path.join(pagesDir, "index.tsx"), + "export default function Page() { return null; }", + ); + fs.writeFileSync(path.join(publicDir, "file.txt"), "hello from file.txt"); + fs.writeFileSync(path.join(publicDir, "logo.svg"), ""); + + const code = await generateServerEntry( + pagesDir, + await resolveNextConfig({}), + createValidFileMatcher(), + null, + null, + tmpDir, + ); + + // The serialized vinextConfig must contain the publicFiles slot with + // both files. Order is sorted ASCII (`file.txt` < `logo.svg`). + expect(code).toContain('"publicFiles":["/file.txt","/logo.svg"]'); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); + + it("emits an empty publicFiles list when no project root is provided", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "vinext-pages-no-public-")); + const pagesDir = path.join(tmpDir, "pages"); + + try { + fs.mkdirSync(pagesDir, { recursive: true }); + fs.writeFileSync( + path.join(pagesDir, "index.tsx"), + "export default function Page() { return null; }", + ); + + // Older callers pass no root — they get an empty publicFiles list and + // the public-file rewrite branch is a no-op. This guards against the + // worker template crashing when `vinextConfig.publicFiles` is missing. + const code = await generateServerEntry( + pagesDir, + await resolveNextConfig({}), + createValidFileMatcher(), + null, + null, + ); + + expect(code).toContain('"publicFiles":[]'); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + }); }); diff --git a/tests/fixtures/pages-basic/next.config.mjs b/tests/fixtures/pages-basic/next.config.mjs index 34c5f89a0..97ccf9fae 100644 --- a/tests/fixtures/pages-basic/next.config.mjs +++ b/tests/fixtures/pages-basic/next.config.mjs @@ -48,6 +48,16 @@ const nextConfig = { has: [{ type: "cookie", key: "mw-before-user" }], destination: "/about", }, + // Issue #1336 (test/e2e/i18n-ignore-rewrite-source-locale): a + // beforeFiles rewrite whose destination is a public/ file must + // serve the file rather than 404. Used by Vitest: + // pages-router.test.ts to verify the post-rewrite public-file + // resolution branch in prod-server (and the analogous worker + // branch in deploy.ts). + { + source: "/aliased-asset", + destination: "/static-asset.txt", + }, ], afterFiles: [ { diff --git a/tests/fixtures/pages-basic/public/static-asset.txt b/tests/fixtures/pages-basic/public/static-asset.txt new file mode 100644 index 000000000..e0aaf6a8d --- /dev/null +++ b/tests/fixtures/pages-basic/public/static-asset.txt @@ -0,0 +1 @@ +hello from static-asset.txt diff --git a/tests/pages-router.test.ts b/tests/pages-router.test.ts index 28978aa1f..7fc9ad2f7 100644 --- a/tests/pages-router.test.ts +++ b/tests/pages-router.test.ts @@ -4642,6 +4642,18 @@ describe("Production server next.config.js features (Pages Router)", () => { expect(html).toContain("About"); }); + // Issue #1336 (Next.js test/e2e/i18n-ignore-rewrite-source-locale): + // a beforeFiles rewrite whose destination resolves to a public/ file + // must serve the file, not return the 404 page. Mirrors Next.js's + // `check_fs` step (resolve-routes.ts) which runs between beforeFiles + // and afterFiles rewrites. + it("serves a public file after a beforeFiles rewrite resolves to one", async () => { + const res = await fetch(`${prodUrl}/aliased-asset`); + expect(res.status).toBe(200); + const body = await res.text(); + expect(body).toContain("hello from static-asset.txt"); + }); + it("applies rewrites with repeated dynamic params in production", async () => { const res = await fetch(`${prodUrl}/repeat-rewrite/hello`); expect(res.status).toBe(200); From b5584777242a3b9332a1d699c545c43a6ba2ba02 Mon Sep 17 00:00:00 2001 From: James Date: Thu, 28 May 2026 14:12:51 +0100 Subject: [PATCH 2/2] fix(prod-server): re-run tryServeStatic after afterFiles rewrite Addresses bigbonk review on #1645: deploy.ts handled both rewrite stages but prod-server only handled beforeFiles. An afterFiles rewrite that lands on a public/ file now serves the file on Node prod-server too, matching the worker entry. Also drops the unnecessary Promise.resolve() wrapper in fetchPublicAsset (env.ASSETS.fetch already returns a Promise). Adds a `/aliased-asset-after` afterFiles rewrite to the pages-basic fixture and a regression test mirroring the existing beforeFiles case. Refs #1336. --- packages/vinext/src/deploy.ts | 2 +- packages/vinext/src/server/prod-server.ts | 20 ++++++++++++++++++++ tests/fixtures/pages-basic/next.config.mjs | 8 ++++++++ tests/pages-router.test.ts | 11 +++++++++++ 4 files changed, 40 insertions(+), 1 deletion(-) diff --git a/packages/vinext/src/deploy.ts b/packages/vinext/src/deploy.ts index 276a27cfc..e8d63512a 100644 --- a/packages/vinext/src/deploy.ts +++ b/packages/vinext/src/deploy.ts @@ -606,7 +606,7 @@ function stripBasePath(pathname: string, basePath: string): string { // origin so env.ASSETS treats the request as same-origin. function fetchPublicAsset(env: Env, request: Request, pathname: string): Promise { const assetUrl = new URL(pathname, request.url); - return Promise.resolve(env.ASSETS.fetch(new Request(assetUrl, { headers: request.headers }))); + return env.ASSETS.fetch(new Request(assetUrl, { headers: request.headers })); } export default { diff --git a/packages/vinext/src/server/prod-server.ts b/packages/vinext/src/server/prod-server.ts index bd67e0489..555a49957 100644 --- a/packages/vinext/src/server/prod-server.ts +++ b/packages/vinext/src/server/prod-server.ts @@ -1986,6 +1986,26 @@ async function startPagesRouterServer(options: PagesRouterServerOptions) { } resolvedUrl = mergeRewriteQuery(resolvedUrl, rewritten); resolvedPathname = resolvedUrl.split("?")[0]; + // Same public/ short-circuit as the beforeFiles branch at step 7a. + // Without this an `afterFiles` rewrite that lands on a public file + // would 404 here while the equivalent worker entry (deploy.ts) + // serves it from `env.ASSETS` — see PR review on #1645. + if ( + resolvedPathname !== "/" && + !resolvedPathname.startsWith("/api/") && + !resolvedPathname.startsWith(`/${ASSET_PREFIX_URL_DIR}/`) && + (await tryServeStatic( + req, + res, + clientDir, + resolvedPathname, + compress, + staticCache, + middlewareHeaders, + )) + ) { + return; + } } } diff --git a/tests/fixtures/pages-basic/next.config.mjs b/tests/fixtures/pages-basic/next.config.mjs index 97ccf9fae..7a707b958 100644 --- a/tests/fixtures/pages-basic/next.config.mjs +++ b/tests/fixtures/pages-basic/next.config.mjs @@ -77,6 +77,14 @@ const nextConfig = { has: [{ type: "cookie", key: "mw-user" }], destination: "/about", }, + // Issue #1336 (PR #1645 review): the afterFiles branch must also + // re-attempt static file serving once it rewrites to a public/ path. + // Without this an afterFiles rewrite to a public file 404s on the + // Node prod server even though the equivalent worker entry serves it. + { + source: "/aliased-asset-after", + destination: "/static-asset.txt", + }, ], fallback: [ { diff --git a/tests/pages-router.test.ts b/tests/pages-router.test.ts index 7fc9ad2f7..0a789ee38 100644 --- a/tests/pages-router.test.ts +++ b/tests/pages-router.test.ts @@ -4654,6 +4654,17 @@ describe("Production server next.config.js features (Pages Router)", () => { expect(body).toContain("hello from static-asset.txt"); }); + // Same parity guarantee for afterFiles rewrites — bonk review on #1645 + // flagged that deploy.ts handled both rewrite stages but prod-server + // only handled beforeFiles. AGENTS.md requires fixing the same bug in + // every affected pipeline. + it("serves a public file after an afterFiles rewrite resolves to one", async () => { + const res = await fetch(`${prodUrl}/aliased-asset-after`); + expect(res.status).toBe(200); + const body = await res.text(); + expect(body).toContain("hello from static-asset.txt"); + }); + it("applies rewrites with repeated dynamic params in production", async () => { const res = await fetch(`${prodUrl}/repeat-rewrite/hello`); expect(res.status).toBe(200);