Skip to content

fix(routing): decode matched params with decodeURIComponent - #1049

Merged
james-elicx merged 2 commits into
cloudflare:mainfrom
NathanDrake2406:fix/decode-dynamic-params
May 4, 2026
Merged

fix(routing): decode matched params with decodeURIComponent#1049
james-elicx merged 2 commits into
cloudflare:mainfrom
NathanDrake2406:fix/decode-dynamic-params

Conversation

@NathanDrake2406

Copy link
Copy Markdown
Contributor

Summary

  • Dynamic params captured by the route matcher are now decoded with decodeURIComponent, matching Next.js behavior
  • %2F/, %23#, %3F? in param values (pages, route handlers, metadata, useParams)
  • Decode applies to both single dynamic params and individual catch-all array elements

Problem

Vinext's segment-safe normalization preserves encoded delimiters for correct route matching (so /files/a%2Fb matches /files/[name] instead of splitting on the encoded slash). However, the captured param values were returned undecoded: the trie would correctly match but return name = "a%2Fb" instead of name = "a/b".

Next.js applies decodeURIComponent at the route-matcher level:
https://github.com/vercel/next.js/blob/canary/packages/next/src/shared/lib/router/utils/route-matcher.ts#L25-L27

This divergence affects every consumer of matched params: page components, generateMetadata, route handlers, and useParams.

Fix

Add decodeURIComponent wrappers at the two route matching entry points:

  • trieMatch in packages/vinext/src/routing/route-trie.ts — used by the App Router and Pages Router trie-based matchers
  • matchRoutePattern in packages/vinext/src/routing/route-pattern.ts — used for slot-specific param extraction, route handlers, and metadata routes

The decode helpers handle both string and string[] (catch-all) param values and gracefully preserve malformed percent escapes (the normalization layer already validates encoding in the strict request path).

Tests

Edge case Coverage
%2F, %23, %3F in dynamic segment Decoded without splitting segments
Catch-all and optional catch-all arrays Each element individually decoded
Malformed %GG Preserved without throwing
Double-encoded %252F Single decode pass only (→ %2F)

Verification

  • vp test run tests/route-trie.test.ts — 64 tests pass
  • vp test run tests/route-pattern.test.ts — 9 tests pass
  • vp test run tests/routing.test.ts tests/route-sorting.test.ts — 143 tests pass
  • vp test run tests/app-router.test.ts — 300 tests pass
  • vp test run tests/pages-router.test.ts — 200 tests pass
  • vp lint — 0 warnings, 0 errors

References

Vinext's segment-safe normalization preserves encoded delimiters (%2F) for
correct route matching, but the captured param values were left undecoded.
This caused dynamic params to return "a%2Fb" instead of "a/b", diverging
from Next.js which decodes at the route-matcher level.

Add decodeURIComponent pass in trieMatch and matchRoutePattern — the two
route matching entry points — so all downstream consumers (pages, route
handlers, metadata, useParams) receive decoded param values.

Mirrors Next.js: packages/next/src/shared/lib/router/utils/route-matcher.ts L25-27
Copilot AI review requested due to automatic review settings May 4, 2026 05:46
@pkg-pr-new

pkg-pr-new Bot commented May 4, 2026

Copy link
Copy Markdown

Open in StackBlitz

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

commit: 3c18ddd

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aligns Vinext’s route matching behavior with Next.js by decoding captured dynamic route params at match time while preserving segment-safe matching. It updates both trie-based matching and pattern-based matching so page routes, route handlers, metadata routes, and related consumers all receive decoded param values consistently.

Changes:

  • Decode matched dynamic params in trieMatch, including individual elements of catch-all arrays.
  • Decode matched params in matchRoutePattern for slot extraction, route handlers, and metadata matching.
  • Add focused regression tests covering encoded delimiters, catch-all params, malformed escapes, and single-pass decoding.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
packages/vinext/src/routing/route-trie.ts Adds post-match param decoding for trie-based route matches and documents the behavior.
packages/vinext/src/routing/route-pattern.ts Adds equivalent param decoding for pattern-based matching paths.
tests/route-trie.test.ts Adds regression coverage for decoded trie params across dynamic and catch-all cases.
tests/route-pattern.test.ts Adds regression coverage for decoded pattern-matched params.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@james-elicx

Copy link
Copy Markdown
Member

/bigbonk review

@ask-bonk ask-bonk Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, well-scoped fix. The decode placement at the two route-matching exit points (trieMatch and matchRoutePattern) is correct and mirrors the Next.js route-matcher.ts implementation. Test coverage is thorough — encoded delimiters, catch-all arrays, malformed escapes, and double-encoding are all covered.

I verified:

  • All call sites of trieMatch (4) and matchRoutePattern (4, via wrappers) receive params that have been through normalizePathnameForRouteMatch[Strict], which decode-then-re-encodes path delimiters (%2F, %23, %3F, %5C). The new decodeURIComponent correctly finalizes decoding of those re-encoded delimiters. Non-delimiter characters are already decoded to literal UTF-8 at that point, so decodeURIComponent is a no-op on them — no double-decode risk for normal URLs.
  • The strict normalization layer (app-rsc-request-normalization.ts:66-68) rejects malformed percent-encoding with a 400 before params ever reach the matcher, so the graceful try/catch fallback in decodeParam is defense-in-depth only (which is fine).
  • matchConfigPattern in config-matchers.ts (the only other param extraction path) is intentionally not affected — those params are used for rewrite/redirect destination interpolation, not page component props.

Two minor suggestions below — neither is blocking.

} catch {
return value;
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: decodeParam / decodeParams in this file and decodePatternParam / decodePatternParams in route-pattern.ts are identical implementations. Consider extracting to a shared helper in routing/utils.ts (which already has decodeRouteSegment) to avoid the duplication. Something like:

// routing/utils.ts
export function decodeParamValue(value: string): string {
  try {
    return decodeURIComponent(value);
  } catch {
    return value;
  }
}

export function decodeMatchedParams<T extends Record<string, string | string[]>>(params: T): void {
  for (const key of Object.keys(params)) {
    const value = params[key];
    if (Array.isArray(value)) {
      params[key] = value.map(decodeParamValue) as T[string] & string[];
    } else {
      params[key] = decodeParamValue(value) as T[string] & string;
    }
  }
}

Non-blocking — the current approach is fine for two call sites.

* upstream normalization layer; this step only decodes the captured
* param strings the caller sees.
*
* Mirrors Next.js route-matcher.ts:25-27.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth noting: Next.js throws DecodeError (which bubbles to a 400 response) on malformed params, while this gracefully preserves the raw value. The test comment at route-trie.test.ts:478 explains the rationale well (the strict normalization layer already rejects malformed encoding). Might be worth adding a brief inline comment here too, since someone reading this file won't see the test comment:

Suggested change
* Mirrors Next.js route-matcher.ts:25-27.
* Mirrors Next.js route-matcher.ts:25-27. Next.js throws DecodeError on
* malformed params; vinext's normalization layer rejects them earlier (400),
* so this layer uses a silent fallback instead.

@ask-bonk

ask-bonk Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

Review submitted. Here's a summary of the review:

Approved with two non-blocking suggestions:

  1. Code duplicationdecodeParam/decodeParams in route-trie.ts and decodePatternParam/decodePatternParams in route-pattern.ts are identical. Suggested extracting to a shared helper in routing/utils.ts.

  2. Inline doc improvement — The JSDoc on trieMatch could note the behavioral difference from Next.js (vinext silently preserves malformed params vs Next.js throwing DecodeError), since the rationale is currently only documented in the test file.

The fix itself is correct and well-placed. I traced all 8 call sites of trieMatch and matchRoutePattern and confirmed there's no double-decode risk for normal URLs — the upstream normalization decodes non-delimiter characters to literal UTF-8 (making decodeURIComponent a no-op on them) and re-encodes path delimiters which the new decode step correctly finalizes. The strict normalization layer rejects malformed encoding at the request boundary before params ever reach the matcher.

github run

@james-elicx
james-elicx merged commit 6c786d6 into cloudflare:main May 4, 2026
27 checks passed
james-elicx added a commit that referenced this pull request May 4, 2026
Extract the identical `decodeParam`/`decodeParams` and
`decodePatternParam`/`decodePatternParams` helpers from `route-trie.ts`
and `route-pattern.ts` into a single `decodeMatchedParams` in
`routing/utils.ts`. Addresses review feedback on #1049.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
james-elicx added a commit that referenced this pull request May 5, 2026
Extract the repeated try/catch JSON.parse(decodeURIComponent(...))
block into a single helper. Follow-up to #1049.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
james-elicx added a commit that referenced this pull request May 5, 2026
Remove the dead `parseCookieLocaleFromHeader` helper from the Pages SSR
entry template. It duplicated the regex + decodeURIComponent pattern
already exported from `pages-i18n.ts` (used by `resolvePagesI18nRequest`)
and had no callers in the generated server entry — locale resolution
goes through `resolvePagesI18nRequest` which handles the cookie itself.

Follow-up to #1049.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
james-elicx added a commit that referenced this pull request May 5, 2026
Extract the repeated `new Response("Forbidden", { status: 403, ... })`
pattern from dev-origin-check.ts and request-pipeline.ts into a single
shared helper. Follow-up to #1049.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
james-elicx added a commit that referenced this pull request May 5, 2026
* refactor(shims): dedupe cookie value serialization

Extract the shared encodeURIComponent-based value encoding from
headers.ts and server.ts ResponseCookies into a single helper.
Follow-up to #1049.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(shims): drop unused export of SerializeSetCookieOptions

knip flagged this as an unused export — the type is only consumed
within cookie-serialize.ts itself.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@NathanDrake2406
NathanDrake2406 deleted the fix/decode-dynamic-params branch May 6, 2026 04:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants