Skip to content

Commit bb38bab

Browse files
committed
fix(rewrites): preserve external destination path prefixes
1 parent 2efac2c commit bb38bab

14 files changed

Lines changed: 333 additions & 16 deletions

File tree

examples/pages-router-cloudflare/next.config.mjs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,12 @@ export default {
2525

2626
async rewrites() {
2727
return {
28-
beforeFiles: [],
28+
beforeFiles: [
29+
{
30+
source: "/external-prefix/:path*",
31+
destination: "http://127.0.0.1:4231/v1/:path*",
32+
},
33+
],
2934
afterFiles: [{ source: "/nav-test", destination: "/about" }],
3035
fallback: [],
3136
};

packages/vinext/src/config/config-matchers.ts

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1137,7 +1137,11 @@ export function matchesRewriteSource(
11371137
* Handles repeated params (e.g. `/api/:id/:id`) and catch-all suffix forms
11381138
* (`:path*`, `:path+`) in a single pass. Unknown params are left intact.
11391139
*/
1140-
function substituteDestinationParams(destination: string, params: Record<string, string>): string {
1140+
function substituteDestinationParams(
1141+
destination: string,
1142+
params: Record<string, string>,
1143+
encodePathParams = false,
1144+
): string {
11411145
const keys = Object.keys(params);
11421146
if (keys.length === 0) return destination;
11431147

@@ -1157,8 +1161,13 @@ function substituteDestinationParams(destination: string, params: Record<string,
11571161
_compiledDestinationParamCache.set(cacheKey, paramRe);
11581162
}
11591163

1160-
const replaceParams = (value: string, encodeParam: (value: string) => string): string =>
1161-
value.replace(paramRe, (_token, key: string) => encodeParam(params[key]));
1164+
const replaceParams = (
1165+
value: string,
1166+
encodeParam: (value: string, modifier: string | undefined) => string,
1167+
): string =>
1168+
value.replace(paramRe, (_token, key: string, modifier: string | undefined) =>
1169+
encodeParam(params[key], modifier),
1170+
);
11621171

11631172
const hashIndex = destination.indexOf("#");
11641173
const beforeHash = hashIndex === -1 ? destination : destination.slice(0, hashIndex);
@@ -1168,13 +1177,30 @@ function substituteDestinationParams(destination: string, params: Record<string,
11681177
if (queryIndex !== -1) {
11691178
const beforeQuery = beforeHash.slice(0, queryIndex);
11701179
const query = beforeHash.slice(queryIndex + 1);
1171-
return `${replaceParams(beforeQuery, (value) => value)}?${replaceParams(
1180+
return `${replaceParams(beforeQuery, encodePathParams ? encodeDestinationPathParamValue : (value) => value)}?${replaceParams(
11721181
query,
1173-
encodeDestinationQueryParamValue,
1182+
(value) => encodeDestinationQueryParamValue(value),
11741183
)}${replaceParams(hash, (value) => value)}`;
11751184
}
11761185

1177-
return replaceParams(destination, (value) => value);
1186+
return replaceParams(
1187+
destination,
1188+
encodePathParams ? encodeDestinationPathParamValue : (value) => value,
1189+
);
1190+
}
1191+
1192+
function encodeDestinationPathParamValue(value: string, modifier: string | undefined): string {
1193+
if (modifier === "*" || modifier === "+") {
1194+
return value.split("/").map(encodeDestinationPathSegment).join("/");
1195+
}
1196+
return encodeDestinationPathSegment(value);
1197+
}
1198+
1199+
function encodeDestinationPathSegment(value: string): string {
1200+
if (/^(?:\.|%2e){1,2}$/i.test(value)) {
1201+
return value.replace(/\.|%2e/gi, (dot) => (dot === "." ? "%252e" : dot.replace("%", "%25")));
1202+
}
1203+
return value;
11781204
}
11791205

11801206
function encodeDestinationQueryParamValue(value: string): string {
@@ -1207,7 +1233,9 @@ function substituteAndSanitizeRewriteDestination(
12071233
destination: string,
12081234
params: Record<string, string>,
12091235
): string {
1210-
const rewritten = substituteAndSanitizeDestination(destination, params);
1236+
const rewritten = sanitizeDestination(
1237+
substituteDestinationParams(destination, params, isExternalUrl(destination)),
1238+
);
12111239
if (!shouldAppendRewriteParamsToQuery(destination, params)) return rewritten;
12121240

12131241
const existingQueryKeys = getDestinationQueryKeys(destination);

packages/vinext/src/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ import {
9999
VINEXT_TIMING_HEADER,
100100
} from "./server/headers.js";
101101
import { logRequest, now } from "./server/request-log.js";
102-
import { normalizePath } from "./server/normalize-path.js";
102+
import { escapeUrlDotSegments, normalizePath } from "./server/normalize-path.js";
103103
import {
104104
filterInternalHeaders,
105105
INTERNAL_HEADERS,
@@ -4739,7 +4739,7 @@ export const loadServerActionClient = ${
47394739
// receives the decoded path for config rule matching.
47404740
{
47414741
const qs = url.includes("?") ? url.slice(url.indexOf("?")) : "";
4742-
url = pathname + qs;
4742+
url = escapeUrlDotSegments(pathname) + qs;
47434743
}
47444744

47454745
const capturedMiddlewarePath = middlewarePath;

packages/vinext/src/server/normalize-path.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,15 @@ export function decodePathParams(pathname: string): string {
4343
.join("/");
4444
}
4545

46+
export function escapeUrlDotSegments(pathname: string): string {
47+
return pathname
48+
.split("/")
49+
.map((segment) =>
50+
/^(?:\.|%2e){1,2}$/i.test(segment) ? segment.replaceAll("%", "%25") : segment,
51+
)
52+
.join("/");
53+
}
54+
4655
export function isInterceptionMatchedUrlPath(value: string): boolean {
4756
return (
4857
value.startsWith("/") &&

packages/vinext/src/server/prod-server.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import {
3434
DEFAULT_IMAGE_SIZES,
3535
type ImageConfig,
3636
} from "./image-optimization.js";
37-
import { normalizePath } from "./normalize-path.js";
37+
import { escapeUrlDotSegments, normalizePath } from "./normalize-path.js";
3838
import { filterInternalHeaders, isOpenRedirectShaped } from "./request-pipeline.js";
3939
import { notFoundResponse } from "./http-error-responses.js";
4040
import {
@@ -1439,7 +1439,7 @@ async function startAppRouterServer(options: AppRouterServerOptions) {
14391439
// RSC handler receives an already-canonical path and doesn't need to
14401440
// re-normalize. This deduplicates the normalizePath work done above.
14411441
const qs = rawUrl.includes("?") ? rawUrl.slice(rawUrl.indexOf("?")) : "";
1442-
const normalizedUrl = pathname + qs;
1442+
const normalizedUrl = escapeUrlDotSegments(pathname) + qs;
14431443

14441444
// Convert Node.js request to Web Request and call the RSC handler
14451445
const request = nodeToWebRequest(req, normalizedUrl, prerenderSecret);
@@ -1660,7 +1660,7 @@ async function startPagesRouterServer(options: PagesRouterServerOptions) {
16601660
res.end("Bad Request");
16611661
return;
16621662
}
1663-
let url = pathname + rawQs;
1663+
let url = escapeUrlDotSegments(pathname) + rawQs;
16641664

16651665
// Internal prerender endpoint — only reachable with the correct build-time secret.
16661666
// Used by the prerender phase to fetch getStaticPaths results via HTTP.

playwright.config.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { defineConfig } from "@playwright/test";
22

33
const appRouterBrowserSpecificTests = "**/app-router/**/*.browser.spec.ts";
44
const appRouterServer = {
5-
command: "npx vp dev --port 4174",
5+
command: "TEST_EXTERNAL_REWRITE_ORIGIN=http://127.0.0.1:4228 npx vp dev --port 4174",
66
cwd: "./tests/fixtures/app-basic",
77
port: 4174,
88
reuseExistingServer: !process.env.CI,
@@ -35,7 +35,8 @@ const projectServers = {
3535
testDir: "./tests/e2e/pages-router",
3636
use: { baseURL: "http://localhost:4173" },
3737
server: {
38-
command: "npx vp run vinext#build && npx vp dev --port 4173",
38+
command:
39+
"TEST_EXTERNAL_REWRITE_ORIGIN=http://127.0.0.1:4229 npx vp run vinext#build && TEST_EXTERNAL_REWRITE_ORIGIN=http://127.0.0.1:4229 npx vp dev --port 4173",
3940
cwd: "./tests/fixtures/pages-basic",
4041
port: 4173,
4142
reuseExistingServer: !process.env.CI,
@@ -119,7 +120,7 @@ const projectServers = {
119120
// Use node to invoke the CLI directly — npx vinext may not be on PATH
120121
// in fixture subdirectories since vinext is a workspace dependency.
121122
command:
122-
"npx vp run vinext#build && node ../../../packages/vinext/dist/cli.js build && node ../../../packages/vinext/dist/cli.js start --port 4175",
123+
"TEST_EXTERNAL_REWRITE_ORIGIN=http://127.0.0.1:4230 npx vp run vinext#build && TEST_EXTERNAL_REWRITE_ORIGIN=http://127.0.0.1:4230 node ../../../packages/vinext/dist/cli.js build && TEST_EXTERNAL_REWRITE_ORIGIN=http://127.0.0.1:4230 node ../../../packages/vinext/dist/cli.js start --port 4175",
123124
cwd: "./tests/fixtures/pages-basic",
124125
port: 4175,
125126
reuseExistingServer: !process.env.CI,

tests/app-router-production-server.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,35 @@ type StartedTextSequenceTarget = {
109109
url: string;
110110
};
111111

112+
async function startPathEchoTarget(): Promise<StartedTextSequenceTarget> {
113+
const upstream = http.createServer((req, res) => {
114+
res.setHeader("content-type", "application/json");
115+
res.end(JSON.stringify({ pathname: new URL(req.url ?? "/", "http://localhost").pathname }));
116+
});
117+
118+
await new Promise<void>((resolve, reject) => {
119+
upstream.once("error", reject);
120+
upstream.listen(0, "127.0.0.1", () => {
121+
upstream.off("error", reject);
122+
resolve();
123+
});
124+
});
125+
126+
const address = upstream.address();
127+
if (!address || typeof address === "string") {
128+
throw new Error("Path echo target did not bind to a TCP port");
129+
}
130+
131+
return {
132+
url: `http://127.0.0.1:${address.port}`,
133+
close() {
134+
return new Promise<void>((resolve, reject) => {
135+
upstream.close((error) => (error ? reject(error) : resolve()));
136+
});
137+
},
138+
};
139+
}
140+
112141
async function startTextSequenceTarget(options?: {
113142
prefix?: string;
114143
recordRequest?: (req: http.IncomingMessage) => void;
@@ -212,6 +241,7 @@ describe("App Router Production server (startProdServer)", () => {
212241
let appStaticDynamicTarget: StartedTextSequenceTarget | undefined;
213242
let appStaticRevalidateTarget: StartedTextSequenceTarget | undefined;
214243
let revalidatePathFetchTarget: StartedTextSequenceTarget | undefined;
244+
let externalRewriteTarget: StartedTextSequenceTarget | undefined;
215245

216246
function extractRequestId(html: string): string | undefined {
217247
return (
@@ -257,6 +287,7 @@ describe("App Router Production server (startProdServer)", () => {
257287
}
258288

259289
beforeAll(async () => {
290+
externalRewriteTarget = await startPathEchoTarget();
260291
appStaticDelayTarget = await startDelayedJsonSequenceTarget(750);
261292
appStaticDynamicTarget = await startTextSequenceTarget({ prefix: "dynamic" });
262293
appStaticRevalidateTarget = await startTextSequenceTarget({ prefix: "revalidate" });
@@ -265,6 +296,7 @@ describe("App Router Production server (startProdServer)", () => {
265296
process.env.TEST_APP_STATIC_DYNAMIC_TARGET = appStaticDynamicTarget.url;
266297
process.env.TEST_APP_STATIC_REVALIDATE_TARGET = appStaticRevalidateTarget.url;
267298
process.env.TEST_REVALIDATE_PATH_REWRITES_TARGET = revalidatePathFetchTarget.url;
299+
process.env.TEST_EXTERNAL_REWRITE_ORIGIN = externalRewriteTarget.url;
268300

269301
try {
270302
// Build the app-basic fixture to the default dist/ directory
@@ -289,15 +321,18 @@ describe("App Router Production server (startProdServer)", () => {
289321
await appStaticDynamicTarget?.close();
290322
await appStaticRevalidateTarget?.close();
291323
await revalidatePathFetchTarget?.close();
324+
await externalRewriteTarget?.close();
292325
} finally {
293326
appStaticDelayTarget = undefined;
294327
appStaticDynamicTarget = undefined;
295328
appStaticRevalidateTarget = undefined;
296329
revalidatePathFetchTarget = undefined;
330+
externalRewriteTarget = undefined;
297331
delete process.env.TEST_APP_STATIC_DELAY_TARGET;
298332
delete process.env.TEST_APP_STATIC_DYNAMIC_TARGET;
299333
delete process.env.TEST_APP_STATIC_REVALIDATE_TARGET;
300334
delete process.env.TEST_REVALIDATE_PATH_REWRITES_TARGET;
335+
delete process.env.TEST_EXTERNAL_REWRITE_ORIGIN;
301336
}
302337
throw error;
303338
}
@@ -309,10 +344,12 @@ describe("App Router Production server (startProdServer)", () => {
309344
await appStaticDynamicTarget?.close();
310345
await appStaticRevalidateTarget?.close();
311346
await revalidatePathFetchTarget?.close();
347+
await externalRewriteTarget?.close();
312348
delete process.env.TEST_APP_STATIC_DELAY_TARGET;
313349
delete process.env.TEST_APP_STATIC_DYNAMIC_TARGET;
314350
delete process.env.TEST_APP_STATIC_REVALIDATE_TARGET;
315351
delete process.env.TEST_REVALIDATE_PATH_REWRITES_TARGET;
352+
delete process.env.TEST_EXTERNAL_REWRITE_ORIGIN;
316353
fs.rmSync(outDir, { recursive: true, force: true });
317354
});
318355

@@ -325,6 +362,16 @@ describe("App Router Production server (startProdServer)", () => {
325362
expect(html).toContain("<script");
326363
});
327364

365+
it("preserves external rewrite path prefixes", async () => {
366+
for (const pathname of ["%252e%252e", ".%252e", "%252e."]) {
367+
const response = await fetch(`${baseUrl}/external-prefix/${pathname}/outside`);
368+
expect(response.status).toBe(200);
369+
expect(await response.json()).toEqual({
370+
pathname: expect.stringMatching(/^\/v1(?:\/|$)/),
371+
});
372+
}
373+
});
374+
328375
// Ported from Next.js: test/e2e/app-dir/app-static/app-static.test.ts
329376
// https://github.com/vercel/next.js/blob/v16.2.6/test/e2e/app-dir/app-static/app-static.test.ts
330377
it("contains dynamic = 'error' failures without terminating later App Router requests", async () => {

tests/e2e/app-router/route-config.spec.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,32 @@
1+
import { createServer, type Server } from "node:http";
12
import { test, expect } from "@playwright/test";
23

34
const BASE = "http://localhost:4174";
45

6+
let externalRewriteServer: Server;
7+
8+
test.beforeAll(async () => {
9+
externalRewriteServer = createServer((request, response) => {
10+
response.writeHead(200, { "content-type": "application/json" });
11+
response.end(
12+
JSON.stringify({
13+
pathname: new URL(request.url ?? "/", "http://127.0.0.1:4228").pathname,
14+
}),
15+
);
16+
});
17+
18+
await new Promise<void>((resolve, reject) => {
19+
externalRewriteServer.once("error", reject);
20+
externalRewriteServer.listen(4228, "127.0.0.1", resolve);
21+
});
22+
});
23+
24+
test.afterAll(async () => {
25+
await new Promise<void>((resolve, reject) => {
26+
externalRewriteServer.close((error) => (error ? reject(error) : resolve()));
27+
});
28+
});
29+
530
test.describe("Route segment configs", () => {
631
test("force-static page renders", async ({ page }) => {
732
const response = await page.goto(`${BASE}/static-test`);
@@ -23,4 +48,22 @@ test.describe("Route segment configs", () => {
2348
await expect(page.locator("h1")).toHaveText("ISR Revalidate Page");
2449
await expect(page.locator('[data-testid="timestamp"]')).not.toBeEmpty();
2550
});
51+
52+
test("external catch-all rewrites preserve their destination path prefix", async ({
53+
request,
54+
}) => {
55+
const normal = await request.get(`${BASE}/external-prefix/resource`);
56+
expect(normal.status()).toBe(200);
57+
await expect(normal.json()).resolves.toEqual({ pathname: "/v1/resource" });
58+
59+
for (const pathname of ["%252e%252e", ".%252e", "%252e."]) {
60+
const traversal = await request.get(`${BASE}/external-prefix/${pathname}/outside`);
61+
expect(traversal.status()).toBe(200);
62+
const traversalResult = (await traversal.json()) as { pathname: string };
63+
expect(traversalResult.pathname).toMatch(/^\/v1(?:\/|$)/);
64+
}
65+
66+
const confusedPrefix = await request.get(`${BASE}/external-prefix../outside`);
67+
expect(confusedPrefix.status()).toBe(404);
68+
});
2669
});
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import { createServer, type Server } from "node:http";
2+
import { expect, test } from "@playwright/test";
3+
4+
let externalRewriteServer: Server;
5+
6+
test.beforeAll(async () => {
7+
externalRewriteServer = createServer((request, response) => {
8+
response.writeHead(200, { "content-type": "application/json" });
9+
response.end(
10+
JSON.stringify({
11+
pathname: new URL(request.url ?? "/", "http://127.0.0.1:4231").pathname,
12+
}),
13+
);
14+
});
15+
await new Promise<void>((resolve, reject) => {
16+
externalRewriteServer.once("error", reject);
17+
externalRewriteServer.listen(4231, "127.0.0.1", resolve);
18+
});
19+
});
20+
21+
test.afterAll(async () => {
22+
await new Promise<void>((resolve, reject) => {
23+
externalRewriteServer.close((error) => (error ? reject(error) : resolve()));
24+
});
25+
});
26+
27+
test("external catch-all rewrites preserve their destination path prefix", async ({ request }) => {
28+
const normal = await request.get("/external-prefix/resource");
29+
expect(normal.status()).toBe(200);
30+
await expect(normal.json()).resolves.toEqual({ pathname: "/v1/resource" });
31+
32+
for (const pathname of ["%252e%252e", ".%252e", "%252e."]) {
33+
const traversal = await request.get(`/external-prefix/${pathname}/outside`);
34+
expect(traversal.status()).toBe(200);
35+
const traversalResult = (await traversal.json()) as { pathname: string };
36+
expect(traversalResult.pathname).toMatch(/^\/v1(?:\/|$)/);
37+
}
38+
39+
expect((await request.get("/external-prefix../outside")).status()).toBe(404);
40+
});

0 commit comments

Comments
 (0)