fix(app-router): preserve semicolons in redirect digests

[NathanDrake2406]

Overview

Field	Details
Goal	Preserve raw redirect URLs when parsing NEXT_REDIRECT digests.
Core change	Parse semicolon-containing redirect targets through a tiny dependency-free redirect digest utility.
Boundary	Only 303, 307, and 308 in the final digest segment are treated as redirect status. Other semicolon-delimited content stays in the URL.
Primary files	packages/vinext/src/utils/redirect-digest.ts, packages/vinext/src/server/next-error-digest.ts, packages/vinext/src/shims/navigation-errors.ts
Expected impact	Redirects whose target URL contains semicolons keep the original destination instead of truncating or misclassifying part of the URL.

Why

Redirect digest parsing has to separate control metadata from the redirect target without assuming semicolons only appear as digest separators. Vinext formats redirect digests with raw URLs, so the URL portion can contain semicolons. This PR centralizes that parsing in a small neutral utility so the server and navigation shim share behavior without making hot server code import the public shim graph.

Area	Principle / invariant	What this PR changes
Redirect URL parsing	Raw redirect URLs are data, even when they contain semicolons.	Joins the URL portion until a valid final status segment is found.
Status parsing	A redirect status must be one of the supported redirect statuses.	Recognizes only final 303, 307, or 308; omitted status still defaults to 307.
Startup graph	Shared parsing should not cross from server code into public navigation shims.	Both server and shim import utils/redirect-digest.ts; server does not import vinext/shims/navigation-errors.

What changed

Scenario	Before	After
URL contains semicolons	The parser could treat semicolon-delimited URL content as digest structure.	Semicolon content remains part of the decoded URL unless the final segment is a valid redirect status.
Status omitted	Defaulted to 307.	Still defaults to 307.
Malformed status-like suffix	parseInt() could accept prefixes like 307garbage.	The suffix stays URL content unless it exactly matches 303, 307, or 308.
Parser ownership	Server and shim parsing could drift, or server could import the shim.	Both use the same dependency-free utility.

Maintainer review path

File	Review focus
packages/vinext/src/utils/redirect-digest.ts	Check the shared parser boundary for raw URLs, final status parsing, and malformed status-like suffixes.
packages/vinext/src/server/next-error-digest.ts	Confirm server-side digest parsing delegates to the utility without importing the navigation shim.
packages/vinext/src/shims/navigation-errors.ts	Confirm navigation error decoding delegates to the same utility and keeps push / replace behavior.
tests/next-error-digest.test.ts	Review coverage for semicolon URLs, omitted status, strict status parsing, and unsupported digests.
tests/error-boundary.test.ts	Confirm redirect boundary behavior still receives the decoded target and navigation type.

Validation

• vp test run tests/next-error-digest.test.ts tests/error-boundary.test.ts
• vp check packages/vinext/src/utils/redirect-digest.ts packages/vinext/src/server/next-error-digest.ts packages/vinext/src/shims/navigation-errors.ts tests/error-boundary.test.ts tests/next-error-digest.test.ts
• Pre-commit staged checks, full check, and knip passed during the final amend.

Risk / compatibility

Surface	Notes
Public API	No public API change.
Redirect behavior	Preserves default status 307 and existing push / replace decoding.
Compatibility	Tightens malformed final status parsing to exact supported redirect statuses. Valid digests remain supported.
Runtime impact	Parser-only change. Shared helper is dependency-free and does not import the navigation shim into server startup.

Non-goals

• Does not change how redirect digests are formatted.
• Does not change redirect response shaping, headers, or browser navigation behavior.
• Does not address RSC page probing or cache-state behavior.

[pkg-pr-new]

Open in StackBlitz

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

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

commit: d08c275

[github-actions]

Performance benchmarks

Compared d08c275 against base 17216d6 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.5 KB	⚫ +0.1%
Client entry size (gzip)	vinext	120.5 KB	120.5 KB	⚫ +0.1%
Dev server cold start	vinext	3.06 s	3.07 s	⚫ +0.4%
Production build time	vinext	3.74 s	3.69 s	⚫ -1.4%
RSC entry closure size (gzip)	vinext	98.0 KB	98.1 KB	⚫ +0.1%
Server bundle size (gzip)	vinext	164.5 KB	164.6 KB	⚫ +0.1%

View detailed results and traces

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

[NathanDrake2406]

Small follow-up nit: the server comment still describes the redirect shape as NEXT_REDIRECT;<type>;<encodedUrl>;<status>, but the shared parser now intentionally accepts both vinext’s encoded three-part form and Next’s raw five-part form. Might be worth updating that wording so future readers don’t “fix” this back to a single encoding model.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Reviewed the parser against Next.js's authoritative isRedirectError (packages/next/src/client/components/redirect-error.ts), which uses digest.slice(2, -2).join(';') for the URL and digest.at(-2) for the status. Two things check out well:

• Status allowlist is correct. 303/307/308 exactly matches Next.js's RedirectStatusCode enum (SeeOther=303, TemporaryRedirect=307, PermanentRedirect=308). I verified there is no valid Next.js redirect digest carrying any other status, so the tightening from parseInt() is aligned with Next.js, not a regression.
• Raw-URL parsing matches Next.js. I diffed the parser against Next.js's exact slice(2,-2) algorithm across canonical digests, including URLs whose query/segment ends in a status-like number (e.g. /redirect?to=308;307; and /a;308;308;). Results are identical.

Two smaller points below. Neither blocks the PR; the second is a cleanup.

Validation run locally: vp test run tests/next-error-digest.test.ts tests/error-boundary.test.ts (43 passed) and vp check on the three changed source files (clean).

[ask-bonk]

The malformed-status fallback branch /;\d[^;]*;$/ is inconsistent, because it only strips the trailing ; when the final segment starts with a digit:

• NEXT_REDIRECT;replace;/foo;307garbage; → url /foo;307garbage (one trailing ; stripped) — this is the case covered by the new test.
• NEXT_REDIRECT;replace;/foo;abc; → url /foo;abc; (both semicolons kept, because abc doesn't start with a digit).

So two equally-malformed digests are treated differently, and the digit-prefix rule has no counterpart in Next.js (isRedirectError treats both as non-redirects since Number(status) is NaN). Since neither shape is emitted by vinext or Next.js in practice, I'd lean toward dropping the special case entirely and just using rest for the non-status branch — it's simpler and the asymmetry goes away. If you keep it, worth a comment explaining why the digit prefix matters.

[ask-bonk]

This re-export doesn't appear to be consumed anywhere — navigation.ts and navigation-server.ts re-export the other helpers from this module but not parseRedirectDigest/RedirectDigest, and nothing else imports them from here (consumers import from utils/redirect-digest.js or the server wrapper directly). If it's not needed as public shim surface, dropping it keeps the shim's exported API minimal:

Suggested change

export { parseRedirectDigest, type RedirectDigest } from "../utils/redirect-digest.js";

[ask-bonk]

Review posted successfully. Here's a summary of my review of PR #2487:

Review Summary

Verdict: Solid PR, no blocking issues. The core parsing logic is correct and I verified it against Next.js's authoritative implementation.

What I verified as correct

• Status allowlist (303/307/308) exactly matches Next.js's RedirectStatusCode enum. I initially suspected a regression where the old parseInt() accepted arbitrary statuses (e.g. 302) but the new parser doesn't — however, I confirmed via the Next.js source that no valid redirect digest ever carries a status outside {303, 307, 308}, so the tightening is aligned with Next.js.
• Raw-URL parsing matches Next.js's digest.slice(2, -2).join(';') / digest.at(-2) algorithm across canonical digests, including tricky cases where the URL itself ends in a status-like number.
• Existing tests still pass; ran the two relevant test files (43 passed) and vp check (clean).

Two non-blocking points raised inline

1. Inconsistent fallback heuristic (redirect-digest.ts:19): the /;\d[^;]*;$/branch treats;307garbage;differently from;abc;(only strips a trailing;` when the segment starts with a digit). Suggested simplifying since neither malformed shape occurs in practice.
2. Unused re-export (navigation-errors.ts:10): export { parseRedirectDigest, type RedirectDigest } isn't consumed anywhere — suggested removing it to keep the shim's public surface minimal.

Also confirmed the author already addressed the earlier self-review nit about the doc comment describing the digest shape.

github run