fix(router-core): encode URL-unsafe ASCII chars in encodePathLikeUrl to break infinite redirect on <test-style paths#7594
Conversation
…to break infinite redirect on `<test`-style paths
`decodePath` decodes `%3C` -> `<` (and the rest of the WHATWG URL "path
percent-encode set": `<`, `>`, `"`, `` ` ``, `{`, `}`), but
`encodePathLikeUrl` only encoded whitespace and non-ASCII characters.
The pair therefore failed to round-trip for those characters.
The SSR redirect comparator in router.ts uses
`latestLocation.publicHref !== nextLocation.publicHref` as the signal
that the URL was rewritten and needs a redirect. For requests like
`/<test` the browser sends `/%3Ctest`, the fast path in `parseLocation`
keeps `publicHref = "/%3Ctest"`, but `buildLocation` rebuilds the URL
from the decoded `pathname = "/<test"` and runs it through
`encodePathLikeUrl`, which left `<` un-encoded - so the new
`publicHref = "/<test"` differs from the latest `"/%3Ctest"`, the
router throws a redirect to `/<test`, the browser re-encodes to
`/%3Ctest`, and the loop continues until the browser gives up with
`ERR_TOO_MANY_REDIRECTS`.
Extending the encode regex to include the WHATWG "path percent-encode
set" makes encode/decode invertible for those characters. `?` and `#`
remain unencoded because the function is called on the combined
`pathname + search + hash` string where they are component separators.
Closes TanStack#7587
📝 WalkthroughWalkthroughThis PR fixes an infinite redirect loop that occurs when request pathnames contain URL-unsafe ASCII characters. The ChangesURL-unsafe character encoding
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/router-core/src/utils.ts`:
- Line 692: The single-line if using the regex /[\s<>"`{}]|[^\u0000-\u007F]/ on
variable path must use curly braces; replace the single-line form "if
(!/.../.test(path)) return path" with a block-style if that contains the return
statement so it conforms to the repo's control-statement brace rule (locate the
usage in utils.ts where path is tested against that regex).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fe1e0687-9d1f-417c-9110-c54e257d9c3b
📒 Files selected for processing (3)
.changeset/encode-url-unsafe-ascii-chars.mdpackages/router-core/src/utils.tspackages/router-core/tests/utils.test.ts
| // biome-ignore lint/suspicious/noControlCharactersInRegex: intentional ASCII range check | ||
| // eslint-disable-next-line no-control-regex | ||
| if (!/\s|[^\u0000-\u007F]/.test(path)) return path | ||
| if (!/[\s<>"`{}]|[^\u0000-\u007F]/.test(path)) return path |
There was a problem hiding this comment.
Add braces to the single-line if body to match repo style.
This changed line violates the TS/JS control-statement brace rule.
Suggested patch
- if (!/[\s<>"`{}]|[^\u0000-\u007F]/.test(path)) return path
+ if (!/[\s<>"`{}]|[^\u0000-\u007F]/.test(path)) {
+ return path
+ }As per coding guidelines, **/*.{ts,tsx,js,jsx} must “Always use curly braces for if, else, loops, and similar control statements.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!/[\s<>"`{}]|[^\u0000-\u007F]/.test(path)) return path | |
| if (!/[\s<>"`{}]|[^\u0000-\u007F]/.test(path)) { | |
| return path | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/router-core/src/utils.ts` at line 692, The single-line if using the
regex /[\s<>"`{}]|[^\u0000-\u007F]/ on variable path must use curly braces;
replace the single-line form "if (!/.../.test(path)) return path" with a
block-style if that contains the return statement so it conforms to the repo's
control-statement brace rule (locate the usage in utils.ts where path is tested
against that regex).
Source: Coding guidelines
1 participant
What
Requests whose pathname contains encoded URL-unsafe ASCII characters (e.g.
<,>,",`,{,}) crashed withERR_TOO_MANY_REDIRECTSbecause the router kept rewriting them between encoded and decoded forms. Visitinghttp://localhost:3000/<teston any fresh TanStack Start project would loop until the browser gave up. This PR makesencodePathLikeUrlpercent-encode the WHATWG URL "path percent-encode set" so its output is a fixed point ofdecodePath, and the SSR redirect comparator no longer fires on these inputs.Closes #7587.
Root cause
The SSR pipeline does three things to incoming URLs:
decodePath(inpackages/router-core/src/utils.ts) strips control characters and decodes percent sequences viadecodeURI. For/%3Ctestthis returns"/<test".parseLocation(router fast path) keepspublicHrefas the raw pathname ("/%3Ctest") and stores the decoded form inpathname("/<test").buildLocationrebuilds the URL from the decodedpathnameand runs the assembled path throughencodePathLikeUrl, which previously only re-encoded whitespace and non-ASCII characters. For<(an ASCII char in the WHATWG path percent-encode set) it returned the input verbatim, so the newpublicHrefwas"/<test".The redirect detector in
router.tsthen compared:"/%3Ctest" !== "/<test"so the router threw a redirect to/<test. The browser percent-encoded the response Location header back to/%3Ctestand re-requested, which produced the same diff. The loop terminated only when Chrome stopped atERR_TOO_MANY_REDIRECTS.The clean fix is to make
encodePathLikeUrlinvertible againstdecodePathfor the charactersdecodePathdecodes. The WHATWG URL spec defines the path percent-encode set as: C0 control + space +"+<+>+`+{+}. Whitespace and the control range were already covered; this PR adds<,>,",`,{,}.?and#are deliberately not added because the function is called on the combinedpathname + search + hashstring where those characters are the component separators.Tests added
A new
describe('URL-unsafe ASCII chars (WHATWG path percent-encode set)')block inpackages/router-core/tests/utils.test.ts. Eachittargets a specific contract:encodes < > "{ } in path segments** — happy path for every character in the added encode set. Without the fix, all five assertions fail withexpected "/<test" to be "/%3Ctest"` etc.round-trips with decodePath for unsafe ASCII chars— guards the actual invariant used by the SSR comparator (encodePathLikeUrl(decodePath(raw).path) === raw) across three inputs.does not double-encode already-percent-encoded sequences— negative regression so the new regex does not match%literals.preserves URL component separators (?, #, /)— confirms?,#, and/still pass through, so search/hash survive whenencodePathLikeUrlis called on a combined path string.The existing
encodePathLikeUrlhappy-path tests (unicode, spaces, emoji,[1]brackets) still pass unchanged, so the fix does not widen the encode set beyond what the WHATWG spec mandates.Verified the failing-test -> fix -> passing-test loop locally:
packages/router-core/tests/utils.test.tspass (113 passed | 3 expected fail, no regressions in any existing case).Repro
Before the fix, on any TanStack Start project (the issue's reproducer):
npx @tanstack/cli@latest new start-basic && cd start-basicpnpm devhttp://localhost:3000/<testERR_TOO_MANY_REDIRECTSwithin seconds.Direct repro at the utility layer (no Start app required):
The
reencoded === rawinvariant is exactly what the SSR redirect comparator relies on - the boolean flip is what stops the redirect loop.Validation
Pre-existing import resolution errors in unrelated test files (
build-location.test.ts,callbacks.test.tsetc. fail to resolve@tanstack/historyregardless of this branch — confirmed by running them onmainwith the change stashed).Changeset added as a
patchbump on@tanstack/router-core.