Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions packages/vinext/src/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> = new Set(vinextConfig?.publicFiles ?? []);
const imageConfig: ImageConfig | undefined = vinextConfig?.images ? {
dangerouslyAllowSVG: vinextConfig.images.dangerouslyAllowSVG,
dangerouslyAllowLocalIP: vinextConfig.images.dangerouslyAllowLocalIP,
Expand All @@ -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<Response> {
const assetUrl = new URL(pathname, request.url);
return env.ASSETS.fetch(new Request(assetUrl, { headers: request.headers }));
}

export default {
async fetch(request: Request, env: Env, ctx: ExecutionContext): Promise<Response> {
try {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}
}
}

Expand Down
9 changes: 9 additions & 0 deletions packages/vinext/src/entries/pages-server-entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -43,9 +44,16 @@ export async function generateServerEntry(
fileMatcher: ReturnType<typeof createValidFileMatcher>,
middlewarePath: string | null,
instrumentationPath: string | null,
projectRoot: string | null = null,
): Promise<string> {
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.
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions packages/vinext/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,7 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] {
fileMatcher,
middlewarePath,
instrumentationPath,
root,
);
}

Expand Down
44 changes: 44 additions & 0 deletions packages/vinext/src/server/prod-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -1962,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;
}
}
}

Expand Down
29 changes: 29 additions & 0 deletions tests/deploy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string>");
// 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");
Expand Down
65 changes: 65 additions & 0 deletions tests/entry-templates.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"), "<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 });
}
});
});
18 changes: 18 additions & 0 deletions tests/fixtures/pages-basic/next.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
{
Expand All @@ -67,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: [
{
Expand Down
1 change: 1 addition & 0 deletions tests/fixtures/pages-basic/public/static-asset.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
hello from static-asset.txt
23 changes: 23 additions & 0 deletions tests/pages-router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4642,6 +4642,29 @@ 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");
});

// 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);
Expand Down
Loading