fix(rewrites): preserve external destination path prefixes

[james-elicx]

Summary

• preserve configured path prefixes when interpolating catch-all parameters into external rewrite destinations
• keep encoded dot segments stable across Node and Workers URL construction
• cover App Router and Pages Router in dev, production, and Cloudflare Workers runtimes

Tests

• vp check packages/vinext/src/config/config-matchers.ts packages/vinext/src/server/normalize-path.ts packages/vinext/src/index.ts packages/vinext/src/server/prod-server.ts playwright.config.ts tests/shims.test.ts tests/app-router-production-server.test.ts tests/e2e/app-router/route-config.spec.ts tests/e2e/pages-router/config.spec.ts tests/e2e/pages-router-prod/production.spec.ts tests/e2e/cloudflare-pages-router/external-rewrite.spec.ts tests/fixtures/app-basic/next.config.ts tests/fixtures/pages-basic/next.config.mjs examples/pages-router-cloudflare/next.config.mjs
• vp test run tests/shims.test.ts -t "escapeUrlDotSegments|matchRewrite with external URLs"
• vp test run tests/app-router-production-server.test.ts -t "preserves external rewrite path prefixes"
• PLAYWRIGHT_PROJECT=app-router npx playwright test tests/e2e/app-router/route-config.spec.ts --project=app-router --workers=1
• PLAYWRIGHT_PROJECT=pages-router npx playwright test tests/e2e/pages-router/config.spec.ts --project=pages-router --workers=1
• PLAYWRIGHT_PROJECT=pages-router-prod npx playwright test tests/e2e/pages-router-prod/production.spec.ts --project=pages-router-prod --workers=1 -g "external catch-all"
• CI=1 PLAYWRIGHT_PROJECT=cloudflare-pages-router npx playwright test tests/e2e/cloudflare-pages-router/external-rewrite.spec.ts --project=cloudflare-pages-router --workers=1

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@2555

npm i https://pkg.pr.new/create-vinext-app@2555

npm i https://pkg.pr.new/vinext@2555

commit: 66d8395

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original
workers-cache	preview	production
web	preview	production

[ask-bonk]

Review: fix(rewrites): preserve external destination path prefixes

I traced the full request pipeline for every runtime touched by this PR. The matchRewrite/substituteDestinationParams encoding changes are reasonable, but there is a real dev/prod-parity gap between the unit tests and the actual request pipeline, plus a couple of smaller concerns. Details below.

1. (Blocking) The added unit tests assert behavior that cannot occur in the real pipeline — single-encoded %2e and literal .. are collapsed by normalizePath before escapeUrlDotSegments ever runs

tests/shims.test.ts calls matchRewrite("/proxy/%2e%2e/outside", …) directly and asserts the destination becomes …/v1/%252e%252e/outside. That works only because the unit test bypasses the request-entry normalization. In every real code path the escape runs after normalizePath:

• packages/vinext/src/index.ts:4731 → 4742 (dev)
• packages/vinext/src/server/prod-server.ts:1332 → 1442 (App Router prod)
• packages/vinext/src/server/prod-server.ts:1656 → 1663 (Pages Router prod)

By the time escapeUrlDotSegments(pathname) runs, the pathname has already been through normalizePath(normalizePathnameForRouteMatchStrict(pathname)), which:

1. decodes each segment (%2e%2e → ..)
2. resolves ./.. segments (/proxy/../outside → /outside)

I verified this with a faithful simulation of the entry code:

/proxy/%2e%2e/outside -> after normalize: /outside -> escaped: /outside
/proxy/../outside     -> after normalize: /outside -> escaped: /outside
/proxy/.%2e/outside   -> after normalize: /outside -> escaped: /outside
/proxy/%2e./outside   -> after normalize: /outside -> escaped: /outside

So escapeUrlDotSegments is effectively a no-op for exactly the single-encoded/literal inputs the unit test claims it handles. The E2E specs pass only because they use double-encoded input (%252e%252e), which survives normalizePath as an opaque segment. Those two test suites are therefore asserting on two different, incompatible inputs, and the unit test gives false confidence that single-encoded traversal is preserved.

Please either:

• move the escape ahead of normalizePath (if the intent really is to preserve single-encoded dot segments through the pipeline — but confirm this against Next.js first, since it changes traversal-normalization semantics), or
• change the unit tests to assert the values that actually reach matchRewrite in production (double-encoded), so unit and E2E coverage agree and the tests reflect reality.

As written, a future refactor that drops escapeUrlDotSegments from the call sites would keep all E2E green while silently breaking nothing the unit test actually exercises through the real pipeline.

2. (Blocking) The Cloudflare Workers path does not call escapeUrlDotSegments at all

escapeUrlDotSegments is imported/used only in index.ts and prod-server.ts. The Workers entry (server/fetch-handler.ts → virtual:vinext-worker-entry) does not use it, and in dev the Cloudflare plugin short-circuits before the dev-server escape runs (index.ts:4788 if (hasCloudflarePlugin) return next();). The new tests/e2e/cloudflare-pages-router/external-rewrite.spec.ts passes anyway because, as in point 1, it uses double-encoded input that is preserved by matchRewrite's own encodeDestinationPathSegment — not by escapeUrlDotSegments. So the PR's stated goal ("keep encoded dot segments stable across Node and Workers URL construction") is only partially realized: the Node and Workers paths take structurally different routes to the same output. Worth documenting explicitly or unifying, otherwise the two runtimes will drift.

3. (Non-blocking) escapeUrlDotSegments and encodeDestinationPathSegment duplicate the same regex/logic with subtly different encodings

• normalize-path.ts:50: /^(?:\.|%2e){1,2}$/i then segment.replaceAll("%", "%25") — turns . unchanged (a literal . stays .) and %2e → %252e.
• config-matchers.ts:1200: same regex but maps . → %252e and %2e → %252e.

So for a literal ./.. segment the two helpers produce different output (escapeUrlDotSegments leaves ./.. literal; encodeDestinationPathSegment encodes them to %252e). That divergence is another reason the two test suites can't both be right about literal-dot inputs. If both are meant to "make dot segments stable", they should share one implementation and one encoding convention.

4. (Nit) playwright.config.ts chains the env var onto the build command too

TEST_EXTERNAL_REWRITE_ORIGIN=… npx vp run vinext#build && TEST_EXTERNAL_REWRITE_ORIGIN=… npx vp dev …

next.config.* reads process.env.TEST_EXTERNAL_REWRITE_ORIGIN at config-eval time, so for the built Pages Router prod project (port 4175) the value must match at start time — the triple repetition is correct but brittle. Consider hoisting to a single env: block on the webServer entry to avoid three copies that can drift (they already use three different ports: 4228/4229/4230/4231).

What I verified

• Added unit tests pass: vp test run tests/shims.test.ts -t "escapeUrlDotSegments|matchRewrite with external URLs" → 18 passed.
• Traced normalizePathnameForRouteMatchStrict (routing/utils.ts:213) + normalizePath (normalize-path.ts:85) + the three escape call sites and reproduced the collapse above.
• Confirmed matchConfigPattern catch-all does not decode (config-matchers.ts:899-901), and pages-request-pipeline.ts:256-257 derives pathname from new URL(request.url).pathname, i.e. the escaped url — so double-encoded input is what actually reaches matchRewrite in prod/Workers.

Overall: the destination-encoding change is sound, but the tests should exercise the inputs that actually flow through each runtime, and the single-encoded/literal-dot claims in the unit tests should be reconciled with normalizePath's behavior before merge.

[ask-bonk]

Review posted to PR #2555. Summary of findings:

Review Summary

I traced the full request pipeline across all runtimes touched by the PR. The destination-encoding change itself is sound, but I found a test/pipeline parity gap:

Blocking

1. Unit tests assert behavior that can't happen in the real pipeline. tests/shims.test.ts asserts matchRewrite("/proxy/%2e%2e/outside") → .../v1/%252e%252e/outside, but in every real call site (index.ts:4742, prod-server.ts:1442 and :1663) escapeUrlDotSegments runs after normalizePath, which already collapsed %2e%2e/.. → resolved to /outside. I verified this with a faithful simulation — the escape is a no-op for the single-encoded/literal inputs the unit test claims to cover. The E2E specs pass only because they use double-encoded %252e%252e, which is preserved by a different mechanism (matchRewrite's own encoder), so the two suites test incompatible inputs.

2. The Cloudflare Workers path never calls escapeUrlDotSegments. It's only used in index.ts/prod-server.ts; the CF plugin short-circuits dev at index.ts:4788. The Workers E2E passes via double-encoded input + encodeDestinationPathSegment, not the escape — so the "stable across Node and Workers" goal is only partially realized.

Non-blocking

3. escapeUrlDotSegments and encodeDestinationPathSegment duplicate the same regex with different encodings for literal dots (. stays literal vs. becomes %252e).
4. playwright.config.ts triple-repeats the env var across build+dev+start commands (brittle; suggest an env: block).

The core recommendation: reconcile the unit-test inputs with normalizePath's actual behavior (or move the escape ahead of normalization if single-encoded preservation is truly intended — verify against Next.js first), so unit and E2E coverage agree and reflect what really flows through each runtime.

github run

[james-elicx]

/bigbonk review for issues

[github-actions]

Performance benchmarks

Compared 66d8395 against base 75b9d1a using alternating same-runner rounds. Next.js was unchanged and skipped.

0 improved · 0 regressed · 6 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	126.4 KB	126.8 KB	⚫ +0.3%
Client entry size (gzip)	vinext	120.4 KB	120.8 KB	⚫ +0.3%
Dev server cold start	vinext	2.78 s	2.79 s	⚫ +0.0%
Production build time	vinext	3.20 s	3.24 s	⚫ +1.2%
RSC entry closure size (gzip)	vinext	98.0 KB	98.7 KB	⚫ +0.7%
Server bundle size (gzip)	vinext	164.6 KB	165.3 KB	⚫ +0.5%

View detailed results and traces

_{🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head}

[ask-bonk]

Review: fix(rewrites): preserve external destination path prefixes

I independently re-traced the full request pipeline across dev, Node prod (App + Pages), and Cloudflare Workers (App + Pages). The destination-encoding change (substituteDestinationParams gaining path-aware param encoding via encodeDestinationPathParamValue/encodeDestinationPathSegment) is sound and is what actually produces the correct external-rewrite output. I confirm the two blocking issues already raised, and add one more concrete finding that strengthens them.

1. (Blocking — confirmed) Unit tests assert behavior that cannot reach matchRewrite in the real pipeline

tests/shims.test.ts calls matchRewrite("/proxy/%2e%2e/outside", …) (single-encoded) and asserts …/v1/%252e%252e/outside, and escapeUrlDotSegments("/proxy/.%2e/outside") → /proxy/.%252e/outside. Neither input can reach those functions in production. In every entry (index.ts:4731, prod-server.ts:1656, app-rsc-request-normalization.ts:96-102), normalizePath(normalizePathnameForRouteMatchStrict(pathname)) runs first and:

1. decodes each segment (%2e%2e → .., .%2e → .., %2e. → ..)
2. resolves ./.. (/proxy/../outside → /outside)

I reproduced the pipeline faithfully:

/proxy/%2e%2e/outside -> normalize -> /outside
/proxy/.%2e/outside   -> normalize -> /outside
/proxy/%2e./outside   -> normalize -> /outside
/proxy/%252e%252e/... -> normalize -> /proxy/%2e%2e/...   (double-encoded survives)

Every single-encoded case collapses to /outside, so it never matches /proxy/:path* and escapeUrlDotSegments is a no-op for exactly the inputs the unit test claims to cover. The E2E/integration specs pass only because they use double-encoded %252e%252e. Unit and E2E coverage are therefore testing incompatible inputs, and the unit test gives false confidence about single-encoded traversal preservation.

Fix: change the unit tests to assert the double-encoded values that actually reach matchRewrite/escapeUrlDotSegments in each runtime, so unit and E2E coverage agree. (Or, if single-encoded preservation is genuinely intended, move the escape ahead of normalizePath — but that changes traversal-normalization semantics and must be verified against Next.js first.)

2. (Blocking — confirmed, with new evidence) The runtimes take structurally different routes to the same output; escapeUrlDotSegments is redundant or dead in most of them

escapeUrlDotSegments is only used in index.ts:4742 (dev Pages) and prod-server.ts:1442/:1663. I traced where the escape actually influences the value matchRewrite sees:

Runtime	Pathname reaching matchRewrite (for %252e%252e)	Escaping performed by
Pages prod (Node)	%252e%252e	escapeUrlDotSegments (pipeline reads new URL().pathname w/o re-normalize)
Pages dev (Node)	%252e%252e	escapeUrlDotSegments
App Router prod (Node)	%2e%2e	encodeDestinationPathSegment — the escapeUrlDotSegments at prod-server.ts:1442 is undone by the RSC handler's re-normalization (app-rsc-request-normalization.ts runs normalizePath again on request.url, collapsing %252e%252e back to %2e%2e)
CF Workers (App + Pages)	%2e%2e	encodeDestinationPathSegment — escapeUrlDotSegments is never called; pages-router-entry.ts:112-113 uses new URL(request.url).pathname directly and dev short-circuits at index.ts:4788 (if (hasCloudflarePlugin) return next();)

I verified the App-Router-prod round-trip:

prod-server pathname:              /proxy/%2e%2e/outside
escapeUrlDotSegments -> request.url: /proxy/%252e%252e/outside
RSC re-normalize -> matchRewrite:   /proxy/%2e%2e/outside   <-- escape undone

So the final output (%252e%252e) is correct everywhere, but it's produced by two independent mechanisms that happen to agree. encodeDestinationPathSegment is doing the real work in App Router (both runtimes) and CF Workers; escapeUrlDotSegments only matters for Node Pages dev/prod. The PR's stated goal ("keep encoded dot segments stable across Node and Workers") is only coincidentally realized. Recommend either unifying on a single mechanism (encodeDestinationPathSegment already covers all runtimes) or documenting the split explicitly — otherwise a future refactor dropping escapeUrlDotSegments from the call sites would leave every E2E green while changing nothing it appears to guard, and the App Router prod escape is already effectively dead.

3. (Non-blocking — confirmed) The two helpers diverge on literal-dot segments

• normalize-path.ts:50 (escapeUrlDotSegments): ./.. stay literal, %2e → %252e.
• config-matchers.ts:1199 (encodeDestinationPathSegment): ./.. → %252e, %2e → %252e.

"."     escapeUrlDotSegments: "."      encodeDestPathSeg: "%252e"
".."    escapeUrlDotSegments: ".."     encodeDestPathSeg: "%252e%252e"
".%2e"  escapeUrlDotSegments: ".%252e" encodeDestPathSeg: "%252e%252e"

This is latent (literal .. never survives normalizePath to reach either helper), but it's a second reason the two suites can't both be right about literal/mixed-dot inputs. If both helpers are meant to "make dot segments stable," share one implementation and one encoding convention.

4. (Nit — confirmed) playwright.config.ts triple-repeats TEST_EXTERNAL_REWRITE_ORIGIN

The pages-router-prod command chains the same env var across build && build && start. Playwright webServer supports an env: field that applies to the whole command string; hoisting it removes three copies that can drift (they already use four distinct ports 4228–4231).

5. (Info) CodeQL "incomplete string escaping" at config-matchers.ts:1201 is a false positive

CodeQL flags dot.replace("%", "%25") for replacing only the first %. Here dot is a single match of /\.|%2e/gi, so it is either . (no %) or %2e (exactly one %). replace("%", "%25") is sufficient. Consider replaceAll or an inline // codeql[js/incomplete-sanitization] suppression with a rationale to keep the scanner quiet, but there is no correctness bug.

What I verified

• Added unit tests pass: vp test run tests/shims.test.ts -t "escapeUrlDotSegments|matchRewrite with external URLs" → 18 passed.
• Reproduced normalizePathnameForRouteMatchStrict + normalizePath collapsing all single-encoded dot inputs to /outside.
• Traced the App-Router-prod re-normalization that undoes escapeUrlDotSegments, and confirmed CF Workers (pages-router-entry.ts, app-rsc-request-normalization.ts) never call it.
• Confirmed every E2E/integration spec added by this PR uses double-encoded %252e%252e, matching the real pipeline.

Bottom line: the encoding change is correct and safe to ship functionally. Before merge, reconcile the unit-test inputs with normalizePath's actual behavior (use double-encoded inputs), and decide whether escapeUrlDotSegments should stay (and be applied consistently, including CF/App Router) or be dropped in favor of encodeDestinationPathSegment alone — right now it's redundant in 3 of the 4 production runtimes.

[ask-bonk]

Review posted to PR #2555.

Summary

I independently re-traced the full request pipeline across all runtimes and confirmed all findings from the prior review, plus uncovered additional concrete evidence.

Blocking

1. Unit tests assert impossible inputs. tests/shims.test.ts passes single-encoded %2e%2e/.%2e to matchRewrite/escapeUrlDotSegments, but normalizePath(normalizePathnameForRouteMatchStrict(...)) runs first at every entry and collapses all of them to /outside before the escape. I reproduced the collapse. Only double-encoded %252e%252e survives — which is what every E2E/integration spec actually uses. Unit and E2E coverage test incompatible inputs.

2. Runtimes take structurally different routes to the same output. New evidence: in App Router prod, escapeUrlDotSegments at prod-server.ts:1442 is undone by the RSC handler's re-normalization, so encodeDestinationPathSegment does the real work. In CF Workers (both routers), escapeUrlDotSegments is never called at all. It only matters for Node Pages dev/prod. So it's redundant/dead in 3 of 4 production runtimes; the "stable across Node and Workers" output is produced by two independent mechanisms that coincidentally agree.

Non-blocking

3. The two helpers diverge on literal-dot segments (. stays literal in one, becomes %252e in the other).
4. playwright.config.ts triple-repeats the env var — hoist to a webServer.env block.
5. The CodeQL config-matchers.ts:1201 finding is a false positive (each match has at most one %).

The encoding change itself is correct and functionally safe to ship. The recommendation is to reconcile unit-test inputs with the real pipeline (double-encoded) and decide whether escapeUrlDotSegments should be applied consistently or dropped in favor of encodeDestinationPathSegment alone.

github run

[james-elicx]

/bigbonk review for issues

[ask-bonk]

@james-elicx Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review: fix(rewrites): preserve external destination path prefixes

I independently re-traced the full request pipeline for every runtime this PR touches (Pages dev, Pages Node prod, App Router prod, CF Workers Pages + App). The core encoding change is correct and functionally sound, and importantly, my trace contradicts several claims in the earlier ask-bonk reviews — those reviews appear to have analyzed an earlier revision (they reference encodeDestinationPathSegment / encodeDestinationPathParamValue, which do not exist in this diff; the current code uses a single shared encodeUrlDotSegment). Details below.

escapeUrlDotSegments is NOT dead code — it is load-bearing (correcting prior review)

The prior reviews claimed escapeUrlDotSegments is "redundant/dead in 3 of 4 runtimes." That is incorrect. I verified that in the Node paths the escape is required precisely because the pathname is reconstructed through new URL() / new Request() after the escape, and the WHATWG URL parser resolves %2e dot segments:

new URL("http://x/proxy/%2e%2e/outside").pathname   -> /outside        (destroyed)
new URL("http://x/proxy/%252e%252e/outside").pathname -> /proxy/%252e%252e/outside  (preserved)

Traced round-trips (double-encoded input /proxy/%252e%252e/outside):

• App Router prod (prod-server.ts:1442): after normalizePath the value is /proxy/%2e%2e/outside. escapeUrlDotSegments -> %252e%252e, which survives nodeToWebRequest's new URL(). The RSC handler's normalizePathnameForRouteMatchStrict then decodes it once back to %2e%2e, which matchRewrite's encodeUrlDotSegment re-encodes. Without the escape, nodeToWebRequest's new URL() would resolve %2e%2e -> /outside and the rewrite would break. So the escape is essential, not undone.
• Pages Node prod (prod-server.ts:1663): url (already escaped) is fed into new Request(\${proto}://${host}${url}`)and the pipeline readsnew URL(request.url).pathname. The escaped %252e%252esurvives; without the escape it would collapse to/outside`.
• Pages dev (index.ts:4742): same shape.

So the escape is genuinely required in all three Node paths to survive URL reconstruction. This is the strongest reason to keep it.

(Minor, valid) Unit-test inputs are only representative of the App Router prod path

tests/shims.test.ts calls matchRewrite("/proxy/%2e%2e/outside", …) (single-encoded) directly and asserts …/v1/%252e%252e/outside. This value is exactly what reaches matchRewrite in App Router prod (the RSC re-normalization decodes %252e%252e -> %2e%2e before matching), so the unit test is accurate for that runtime. However, it does not reflect what reaches matchRewrite in the other runtimes:

• Pages Node dev/prod: the escape runs in the entry, so matchRewrite receives %252e%252e (already double-encoded); its own encodeUrlDotSegment is a no-op there.
• CF Workers: new URL(request.url).pathname resolves single-encoded %2e -> /outside, so only double-encoded %252e%252e ever reaches matchRewrite.

This isn't a correctness bug — the final output is identical everywhere — but the single-encoded unit input exercises matchRewrite's internal encodeUrlDotSegment in isolation, which only matters on the App Router prod path. A one-line comment noting which runtime's intermediate state each input models would prevent a future reader from assuming single-encoded traversal flows unchanged through every entry (it doesn't; new URL() and normalizePath collapse it). The E2E/integration specs correctly use double-encoded %252e%252e.

(Minor) CF Workers relies on implicit URL-parser preservation, not the escape

The CF Workers entries (pages-router-entry.ts:112, and the App RSC path) never call escapeUrlDotSegments; they read new URL(request.url).pathname directly. Double-encoded input survives because new URL() leaves %25 alone, so the output still matches the Node paths. This works, but it means CF depends on the incoming URL already being double-encoded rather than on an explicit escape step. Worth a short comment so a future refactor doesn't "helpfully" add a decode/normalize to the CF entry and silently break parity.

(Non-issue) CodeQL "incomplete string escaping" at config-matchers.ts is a false positive

normalize-path.ts:112 does dot.replaceAll("%", "%25") where dot is a single match of /\.|%2e/gi — either . (no %) or %2e (exactly one %). replaceAll already handles all occurrences; there is no correctness bug. An inline // codeql[js/incomplete-sanitization] suppression with a one-line rationale would silence the scanner.

Stale points from the prior reviews (do not apply to this diff)

• "Two helpers diverge on literal-dot segments" — there is only one helper now (encodeUrlDotSegment), shared by both escapeUrlDotSegments and substituteDestinationParams. No divergence.
• "playwright.config.ts triple-repeats the env var / suggest an env: block" — this diff already uses env: { …process.env, TEST_EXTERNAL_REWRITE_ORIGIN: … } blocks on each webServer entry. Already addressed.
• "escapeUrlDotSegments is undone / dead in App Router prod" — refuted above; it is required to survive new URL().

What I verified

• encodeUrlDotSegment edge cases: ./../%2e/%2E/.%2e/%2e. encode correctly; ..., %2e%2e%2e, .x, %252e are left untouched.
• new URL() resolves single-encoded %2e dot segments but preserves double-encoded %252e — the mechanism the escape defends against.
• App Router prod / Pages prod / Pages dev round-trips reproduced; the escape is required in each.
• CF Workers Pages entry reads url.pathname raw with no escape; parity holds only via double-encoding.
• Added unit tests pass: vp test run tests/shims.test.ts -t "escapeUrlDotSegments|matchRewrite with external URLs" -> 17 passed.

Bottom line: The fix is correct and safe to ship. No blocking issues. Recommended (non-blocking) follow-ups: (1) annotate the unit-test inputs with the runtime whose intermediate state they model, (2) add a short comment on the CF entry explaining why it needs no explicit escape, and (3) suppress the CodeQL false positive.

[ask-bonk]

Review posted to PR #2555: #2555 (comment)

Summary

I independently traced the full request pipeline across all five runtimes and reached conclusions that correct several claims in the prior ask-bonk reviews (which analyzed an earlier revision referencing functions that no longer exist).

Verdict: The fix is correct and safe to ship. No blocking issues.

Key findings:

1. escapeUrlDotSegments is load-bearing, not dead code (refuting the prior review). In all three Node paths the pathname is reconstructed through new URL()/new Request() after the escape, and the WHATWG URL parser resolves single-encoded %2e dot segments (/proxy/%2e%2e/outside → /outside). The escape to %252e%252e is what survives that reconstruction. Without it, external rewrites with traversal-shaped catch-all segments would break. I verified this round-trip for App Router prod, Pages Node prod, and Pages dev.

2. The core encoding change is sound — a single shared encodeUrlDotSegment handles all edge cases correctly (., .., %2e, mixed case; leaves ..., %252e, .x untouched).

Minor, non-blocking suggestions:

• The single-encoded unit-test inputs only model the App Router prod intermediate state; a comment noting which runtime each input represents would prevent confusion.
• The CF Workers entry relies implicitly on URL-parser preservation of double-encoding rather than an explicit escape — worth a comment to guard against future refactors.
• The CodeQL "incomplete escaping" alert is a confirmed false positive (replaceAll on a single-% match).

Stale prior-review points that don't apply to this diff: the "two diverging helpers" (only one helper exists now) and the playwright env: block suggestion (already implemented).

github run