fix(routing): re-encode trailing-slash Location for non-Latin-1 paths

[Xplod13]

Summary

• Percent-encode the trailing-slash redirect target before building the Location header, so paths containing characters above U+00FF (CJK slugs, emoji) no longer crash the Headers constructor.
• Encode every character that is invalid raw in an RFC 3986 path while keeping existing %xx escapes and / delimiters intact, so already-encoded delimiters such as %23 are not double-encoded (a naive encodeURI would turn %23 into %2523).
• Add unit regression coverage in tests/request-pipeline.test.ts for non-Latin-1 slugs, emoji (astral/surrogate pairs), spaces, already-encoded delimiters, and reserved ASCII.

Root Cause

normalizeTrailingSlash built the redirect Location as basePath + normalizedPathname + search, where normalizedPathname arrives already percent-decoded (see app-rsc-request-normalization → normalizePathnameForRouteMatchStrict, which decodeURIComponents each segment and only re-encodes the path delimiters # ? / \).

For any path with a character above U+00FF — e.g. /日本 or an emoji under trailingSlash: true — new Response(null, { headers: { Location: "/日本/" } }) throws TypeError: Cannot convert argument to a ByteString in the Workers/undici Headers implementation (which only accepts ByteStrings, code points ≤ 0xFF). The throw is caught by the outer handler and surfaces as a 500 Internal Server Error instead of the expected 308, making the page unreachable. Latin-1 characters such as spaces did not throw but emitted a malformed, un-percent-encoded Location like /about us/.

The fix re-encodes the redirect target. encodeURI is the wrong tool here because it escapes %, double-encoding the delimiter sequences the upstream normalizer deliberately left in place. Instead we encode against an RFC 3986 path allowlist, preserving % and /:

normalizedPathname.replace(/[^A-Za-z0-9\-._~!$&'()*+,;=:@/%]/gu, encodeURIComponent)

The u flag makes the regex match astral code points whole, so emoji surrogate pairs are not split into lone surrogates. Control characters (CR/LF/NUL) fall outside the allowlist and are encoded as well, hardening the redirect against header injection. The query string is taken verbatim from the request URL (already encoded) and is intentionally not re-encoded.

References

• Issue: Trailing slash: redirect Location built from decoded pathname — 500 on non-Latin-1 paths (ByteString throw) #1979
• Next.js trailing-slash behavior (parity reference): https://github.com/vercel/next.js/blob/canary/test/e2e/trailing-slashes/with-trailing-slash.test.ts
• RFC 3986 §3.3 (path pchar grammar): https://www.rfc-editor.org/rfc/rfc3986#section-3.3

Verification

• pnpm test tests/request-pipeline.test.ts — 102 passed (5 new regression cases for Trailing slash: redirect Location built from decoded pathname — 500 on non-Latin-1 paths (ByteString throw) #1979)
• npx vp check packages/vinext/src/server/request-pipeline.ts tests/request-pipeline.test.ts — format, lint, types clean

[james-elicx]

/bigbonk review for issues

[pkg-pr-new]

Open in StackBlitz

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

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

commit: ab208c1

[ask-bonk]

Review: trailing-slash Location re-encoding

Solid, well-scoped fix. I verified the root-cause analysis against the actual pipeline and it holds up:

• normalizePathnameForRouteMatchStrict (routing/utils.ts:149) decodes each segment via decodeURIComponent and re-encodes only the path delimiters # ? / \ (encodePathDelimiters, routing/utils.ts:105-109). So pathname reaching normalizeTrailingSlash is segment-decoded with %23/%2f/%3f/%5c left intact — exactly as the PR describes.
• Keeping % plus hex digits in the allowlist correctly preserves those %xx delimiter escapes (no %23→%2523 double-encoding), while encodeURI would have broken them. Good call.
• The u flag matching astral code points whole is the right way to avoid lone-surrogate output; the emoji test confirms /%F0%9F%98%80/.
• search is taken from url.search of a URL object at both real callers (app-rsc-handler.ts:477, pages-request-pipeline.ts:232), which is already percent-encoded, so not re-encoding it is correct.

Parity is preserved. normalizeTrailingSlash is the single shared helper used by all three request paths — app-rsc-handler.ts:473, pages-request-pipeline.ts:232, and dev index.ts:3451 — so the fix lands uniformly with no dev/prod divergence.

Verification I ran locally:

• vp test run tests/request-pipeline.test.ts -t normalizeTrailingSlash → 25 passed (incl. 5 new #1979 cases)
• vp check packages/vinext/src/server/request-pipeline.ts tests/request-pipeline.test.ts → format/lint/types clean

The security framing (control chars CR/LF/NUL falling outside the allowlist and being encoded) is a nice hardening bonus, and isOpenRedirectShaped still runs on the decoded pathname before encoding, so the open-redirect guard is unaffected.

One non-blocking process note below re: the hand-authored changeset.

[ask-bonk]

Non-blocking process note: AGENTS.md says changesets should not be authored by hand — they're generated in CI from the Conventional Commit subject (fix(routing): ...), and the commit here already qualifies. The repo's .changeset/README.md does state that hand-committed changesets are still honored, so this won't break the release; it's just redundant with the auto-generated one. If you'd prefer to follow the documented convention you could drop this file and rely on the commit message. Either way the content is accurate. Up to a maintainer's preference.

[Xplod13]

Good catch — dropped the hand-authored changeset. The fix(routing): commit subject already drives the auto-generated patch changeset (per AGENTS.md "never create changesets manually"), so the committed file was redundant. Removed in ab208c12.

[ask-bonk]

Allowlist matches RFC 3986 pchar + / + %, which is correct. Worth being aware that + is intentionally kept raw here — that's consistent with how Next.js treats + in path segments (it's a valid sub-delim and only carries space semantics in query strings, which you correctly leave untouched). No change needed; just noting the deliberate choice for future readers, since the comment block enumerates everything else but not why +/,/; stay raw.

[Xplod13]

Added the rationale to the comment block: sub-delimiters (!$&'()*+,;=) and :@ are valid pchar and kept raw to match Next.js, and + only carries space semantics in the query string (which stays untouched). Thanks for flagging — clearer for future readers now. ab208c12.

[ask-bonk]

Review posted successfully. Here's my summary:

Review complete — PR #2011

Verdict: Approve-quality fix. The change is correct, well-reasoned, and well-tested.

What I verified

• Root-cause analysis is accurate. Confirmed pathname reaching normalizeTrailingSlash is segment-decoded with path delimiters (%23/%2f/%3f/%5c) re-encoded by encodePathDelimiters (routing/utils.ts:105-109). The Headers ByteString throw for chars > U+00FF is real.
• The allowlist regex is right. Keeping % + hex in the safe set preserves the upstream %xx delimiter escapes (avoids %23→%2523), and the u flag handles emoji surrogate pairs whole.
• search correctly not re-encoded — it comes from url.search (already encoded) at both real callers.
• Parity preserved — normalizeTrailingSlash is the single shared helper for app-rsc, pages, and dev paths, so no divergence.
• Ran locally: 25 trailing-slash tests pass (incl. 5 new Trailing slash: redirect Location built from decoded pathname — 500 on non-Latin-1 paths (ByteString throw) #1979 cases); vp check clean on both changed files.

Comments posted (both non-blocking)

1. Changeset — AGENTS.md says not to hand-author changesets (CI generates them from the Conventional Commit); the fix(routing): ... subject already qualifies. Content is correct and won't break release, just redundant.
2. Allowlist note — flagged that +/,/; are intentionally kept raw (valid RFC 3986 sub-delims, consistent with Next.js) for future-reader clarity.

No correctness, security, or parity issues found.

github run

[james-elicx]

/bigbonk re-review for correctness and nextjs parity issues

[ask-bonk]

@james-elicx Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk re-review for nextjs parity issues

[ask-bonk]

Re-review: correctness + Next.js parity

Re-reviewed the latest commit (ab208c1) focused on correctness and parity. No blocking issues — the fix is sound.

Correctness verified

• The %-in-allowlist + lone-% edge case is unreachable. All three callers — app-rsc-handler.ts:473, pages-request-pipeline.ts:232, and dev index.ts:3451 — run normalizePathnameForRouteMatchStrict first, which throws on malformed percent-encoding and returns 400 before normalizeTrailingSlash ever sees the path (index.ts:3417-3424). So a malformed escape like /100% or /a%zzb (which the allowlist would otherwise pass through raw) can't reach the encoder in practice. Keeping % in the safe set is therefore both necessary (to avoid %23→%2523) and safe.
• Open-redirect guard is unaffected. isOpenRedirectShaped(pathname) runs on the decoded path before encoding (request-pipeline.ts:326). Encoding only hardens: / stays in the allowlist (delimiter preserved) and backslash is encoded to %5C, so the encode step can never reintroduce a //- or /\-shaped Location. Confirmed:
  • /\\evil.com → /%5C%5Cevil.com
  • control chars /\tx → /%09x (CR/LF/NUL hardening as described)

Parity verified

• Wire output matches Next.js. In Next.js the request path is normally already percent-encoded (/%E6%97%A5%E6%9C%AC, /about%20us). vinext's pipeline decodes segment-wise then this fix re-encodes, producing the same on-the-wire Location. So a raw space round-trips back to %20, and CJK/emoji round-trip to their UTF-8 %xx form — identical to what Next.js emits.
• Single shared helper, no dev/prod divergence. normalizeTrailingSlash is the one helper used by all three request paths, so the fix lands uniformly.
• search correctly not re-encoded — both real callers pass url.search from a URL object (already encoded).

Tests

vp test run tests/request-pipeline.test.ts -t normalizeTrailingSlash → 25 passed (incl. the 5 new #1979 cases). Coverage is good: CJK, emoji surrogate pairs, spaces, already-encoded delimiters, and reserved ASCII.

The author already addressed the prior review (dropped the hand-authored changeset, expanded the rationale comment). LGTM.

[ask-bonk]

Re-review posted. Summary of my findings:

PR #2011 — fix(routing): re-encode trailing-slash Location for non-Latin-1 paths

Verdict: No blocking issues. The fix is correct and preserves Next.js parity.

Key things I verified on this re-review (correctness + parity focus):

1. The %-in-allowlist edge case is unreachable. A literal lone % (e.g. /100%, /a%zzb) would pass through the allowlist raw, producing an invalid URI — but all three callers run normalizePathnameForRouteMatchStrict first, which throws on malformed percent-encoding and returns 400 before normalizeTrailingSlash is reached. So keeping % safe is both necessary (avoids %23→%2523) and not a hazard.

2. Open-redirect guard intact. isOpenRedirectShaped runs on the decoded path before encoding; encoding only hardens (/ preserved, \→%5C, CR/LF/NUL encoded), so it can't reintroduce a protocol-relative shape.

3. Wire-level parity with Next.js. Decode-then-re-encode produces the same Location bytes Next.js emits (%E6%97%A5%E6%9C%AC, %20, etc.).

4. Single shared helper → no dev/prod divergence; search correctly left un-re-encoded.

5. Tests pass (25 incl. 5 new Trailing slash: redirect Location built from decoded pathname — 500 on non-Latin-1 paths (ByteString throw) #1979 cases).

The author already addressed the earlier review (removed the hand-authored changeset, expanded the rationale comment).

github run