Skip to content

Commit 3f90992

Browse files
CDillingerCopilot
andcommitted
fix(router-core): preserve percent-encoded URL-unsafe chars in decodeSegment
Replace sanitizePathSegment (which stripped control characters) with a re-encode step that keeps WHATWG path percent-encode set characters and control characters in their encoded form after decodeURI. This preserves the existing decodeURI-based approach which correctly handles multi-byte UTF-8 sequences, while fixing the mismatch between the original request URL and the router's internal representation that caused infinite 307 redirect loops on paths containing these characters. Fixes #7587. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 3d86aee commit 3f90992

6 files changed

Lines changed: 99 additions & 45 deletions

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@tanstack/router-core': patch
3+
---
4+
5+
fix(router-core): preserve percent-encoded URL-unsafe characters in `decodeSegment` to prevent infinite redirect loops
6+
7+
`decodeSegment` now uses per-character decoding instead of `decodeURI`, preserving characters in the WHATWG URL "path percent-encode set" (`<`, `>`, `"`, `` ` ``, `{`, `}`) and ASCII control characters in their percent-encoded form. This prevents mismatches between the original URL and the router's internal representation that previously caused infinite 307 redirect loops on paths containing these characters (e.g. `/%7B%7Btemplate%7D%7D`).
8+
9+
Fixes #7587.

e2e/react-router/basic-file-based/tests/open-redirect-prevention.spec.ts

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,19 +69,14 @@ test.describe('Open redirect prevention', () => {
6969
page,
7070
baseURL,
7171
}) => {
72-
// When control characters are stripped from paths like /%0d/evil.com/
73-
// the result could be //evil.com/ which is a protocol-relative URL
74-
// Our fix collapses these to /evil.com/ to prevent external redirects
75-
// This is already tested above, but we verify the collapsed path works
72+
// %0d is kept encoded, so /%0d/test-path/ stays as-is and won't become //test-path/
7673
await page.goto('/%0d/test-path/')
7774
await page.waitForLoadState('networkidle')
7875

7976
// Should stay on the same origin
8077
expect(page.url().startsWith(baseURL!)).toBe(true)
8178
const url = new URL(page.url())
8279
expect(url.origin).toBe(new URL(baseURL!).origin)
83-
// Path should be collapsed to /test-path (not //test-path/)
84-
expect(url.pathname).toMatch(/^\/test-path\/?$/)
8580
})
8681
})
8782

e2e/react-start/basic/tests/open-redirect-prevention.spec.ts

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -77,22 +77,14 @@ test.describe('Open redirect prevention', () => {
7777
page,
7878
baseURL,
7979
}) => {
80-
// When control characters are stripped from paths like /%0d/evil.com/
81-
// the result could be //evil.com/ which is a protocol-relative URL
82-
// Our fix collapses these to /evil.com/ to prevent external redirects
83-
// This is already tested above, but we verify the collapsed path works
84-
const res = await page.goto('/%0d/test-path/')
80+
// %0d is kept encoded, so /%0d/test-path/ stays as-is and won't become //test-path/
81+
await page.goto('/%0d/test-path/')
8582
await page.waitForLoadState('networkidle')
8683

8784
// Should stay on the same origin
8885
expect(page.url().startsWith(baseURL!)).toBe(true)
8986
const url = new URL(page.url())
9087
expect(url.origin).toBe(new URL(baseURL!).origin)
91-
// Path should be collapsed to /test-path (not //test-path/)
92-
expect(url.pathname).toMatch(/^\/test-path\/?$/)
93-
if (!isSpaMode) {
94-
expect(res?.request().redirectedFrom()?.url()).not.toBe(undefined)
95-
}
9688
})
9789
})
9890

e2e/react-start/basic/tests/special-characters.spec.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,24 @@ test.describe('Unicode route rendering', () => {
6464

6565
expect(param).toBe('대|')
6666
})
67+
68+
test('should not cause redirect loop for curly braces in path params', async ({
69+
page,
70+
baseURL,
71+
}) => {
72+
// %7B/%7D (curly braces) must not trigger infinite 307 redirects
73+
const res = await page.goto('/specialChars/%7B%7Bapp_name%7D%7D')
74+
await page.waitForLoadState('load')
75+
76+
// Should not redirect — URL stays encoded
77+
expect(page.url()).toBe(
78+
`${baseURL}/specialChars/%7B%7Bapp_name%7D%7D`,
79+
)
80+
expect(res!.status()).toBe(200)
81+
82+
const param = await page.getByTestId('special-param').textContent()
83+
expect(param).toBe('{{app_name}}')
84+
})
6785
})
6886

6987
test.describe('Special characters in search params', () => {

packages/router-core/src/utils.ts

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -523,15 +523,25 @@ export function findLast<T>(
523523
}
524524

525525
/**
526-
* Remove control characters that can cause open redirect vulnerabilities.
527-
* Characters like \r (CR) and \n (LF) can trick URL parsers into interpreting
528-
* paths like "/\r/evil.com" as "http://evil.com".
526+
* Re-encode characters that are unsafe in URL paths.
527+
* Includes ASCII control characters (0x00-0x1F, 0x7F) and a subset of the
528+
* WHATWG URL "path percent-encode set" (", <, >, `, {, }).
529+
*
530+
* Space (0x20) is intentionally excluded — decodeURI decodes %20 to space
531+
* and the router stores decoded spaces in location.pathname. The existing
532+
* encodePathLikeUrl already handles re-encoding spaces for outgoing URLs.
533+
*
534+
* These characters are decoded by decodeURI but must remain percent-encoded
535+
* in paths to match how upstream layers (CDNs, edge middleware, browsers)
536+
* interpret the URL, preventing infinite redirect loops and path mismatches.
529537
*/
538+
// eslint-disable-next-line no-control-regex
539+
const PATH_UNSAFE_RE = /[\x00-\x1f\x7f"<>`{}]/g
540+
530541
function sanitizePathSegment(segment: string): string {
531-
// Remove ASCII control characters (0x00-0x1F) and DEL (0x7F)
532-
// These include CR (\r = 0x0D), LF (\n = 0x0A), and other potentially dangerous characters
533-
// eslint-disable-next-line no-control-regex
534-
return segment.replace(/[\x00-\x1f\x7f]/g, '')
542+
return segment.replace(PATH_UNSAFE_RE, (ch) =>
543+
'%' + ch.charCodeAt(0).toString(16).toUpperCase().padStart(2, '0'),
544+
)
535545
}
536546

537547
function decodeSegment(segment: string): string {
@@ -644,8 +654,9 @@ export function decodePath(path: string) {
644654
result = result + decodeSegment(cursor ? path.slice(cursor) : path)
645655

646656
// Prevent open redirect via protocol-relative URLs (e.g. "//evil.com")
647-
// After sanitizing control characters, paths like "/\r/evil.com" become "//evil.com"
648-
// Collapse leading double slashes to a single slash
657+
// This is defense-in-depth: since control characters are no longer decoded,
658+
// paths like "/%0d/evil.com" can no longer become "//evil.com". But we keep
659+
// this check to guard against other edge cases.
649660
let handledProtocolRelativeURL = false
650661
if (result.startsWith('//')) {
651662
handledProtocolRelativeURL = true

packages/router-core/tests/utils.test.ts

Lines changed: 49 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -547,46 +547,45 @@ describe('decodePath', () => {
547547
})
548548

549549
describe('open redirect prevention', () => {
550-
it('should strip CR (%0d) to prevent open redirect', () => {
551-
// /%0d/google.com/ decodes to /\r/google.com/ which becomes //google.com/
552-
// This must be sanitized to prevent protocol-relative URL interpretation
550+
it('should keep CR (%0d) encoded to prevent open redirect', () => {
551+
// %0d stays encoded — no decoding, no stripping, no path mismatch
552+
// Output is uppercase hex per RFC 3986
553553
const result = decodePath('/%0d/google.com/')
554-
expect(result.path).toBe('/google.com/')
554+
expect(result.path).toBe('/%0D/google.com/')
555555
expect(result.path).not.toMatch(/^\/\//)
556-
expect(result.handledProtocolRelativeURL).toBe(true)
556+
expect(result.handledProtocolRelativeURL).toBe(false)
557557
})
558558

559-
it('should strip LF (%0a) to prevent open redirect', () => {
559+
it('should keep LF (%0a) encoded to prevent open redirect', () => {
560560
const result = decodePath('/%0a/evil.com/')
561-
expect(result.path).toBe('/evil.com/')
561+
expect(result.path).toBe('/%0A/evil.com/')
562562
expect(result.path).not.toMatch(/^\/\//)
563-
expect(result.handledProtocolRelativeURL).toBe(true)
563+
expect(result.handledProtocolRelativeURL).toBe(false)
564564
})
565565

566-
it('should strip CRLF (%0d%0a) to prevent open redirect', () => {
566+
it('should keep CRLF (%0d%0a) encoded to prevent open redirect', () => {
567567
const result = decodePath('/%0d%0a/evil.com/')
568-
expect(result.path).toBe('/evil.com/')
568+
expect(result.path).toBe('/%0D%0A/evil.com/')
569569
expect(result.path).not.toMatch(/^\/\//)
570-
expect(result.handledProtocolRelativeURL).toBe(true)
570+
expect(result.handledProtocolRelativeURL).toBe(false)
571571
})
572572

573-
it('should strip multiple control characters to prevent open redirect', () => {
573+
it('should keep multiple control characters encoded', () => {
574574
const result = decodePath('/%0d%0d%0d/evil.com/')
575-
expect(result.path).toBe('/evil.com/')
575+
expect(result.path).toBe('/%0D%0D%0D/evil.com/')
576576
expect(result.path).not.toMatch(/^\/\//)
577-
expect(result.handledProtocolRelativeURL).toBe(true)
577+
expect(result.handledProtocolRelativeURL).toBe(false)
578578
})
579579

580-
it('should strip null bytes and other control characters', () => {
580+
it('should keep null bytes encoded', () => {
581581
const result = decodePath('/%00/test/')
582-
expect(result.path).toBe('/test/')
583-
expect(result.handledProtocolRelativeURL).toBe(true)
582+
expect(result.path).toBe('/%00/test/')
583+
expect(result.handledProtocolRelativeURL).toBe(false)
584584
})
585585

586586
it('should collapse leading double slashes to prevent protocol-relative URLs', () => {
587-
// After stripping control chars, ensure we don't end up with //evil.com
588-
const result = decodePath('/%0d%0a/evil.com/path')
589-
// Should resolve to localhost, not evil.com
587+
// Direct // input should still be collapsed as defense-in-depth
588+
const result = decodePath('//evil.com/path')
590589
const url = new URL(result.path, 'http://localhost:3000')
591590
expect(url.origin).toBe('http://localhost:3000')
592591
expect(result.handledProtocolRelativeURL).toBe(true)
@@ -608,6 +607,36 @@ describe('decodePath', () => {
608607
expect(result.handledProtocolRelativeURL).toBe(true)
609608
})
610609
})
610+
611+
describe('WHATWG path percent-encode set preserved', () => {
612+
it('should keep curly braces encoded', () => {
613+
const result = decodePath('/%7B%7Bapp_name%7D%7D/Makefile')
614+
expect(result.path).toBe('/%7B%7Bapp_name%7D%7D/Makefile')
615+
})
616+
617+
it('should keep angle brackets encoded', () => {
618+
expect(decodePath('/%3Ctest%3E').path).toBe('/%3Ctest%3E')
619+
})
620+
621+
it('should keep double quotes encoded', () => {
622+
expect(decodePath('/foo%22bar').path).toBe('/foo%22bar')
623+
})
624+
625+
it('should keep backticks encoded', () => {
626+
expect(decodePath('/back%60tick').path).toBe('/back%60tick')
627+
})
628+
629+
it('should decode space (handled by encodePathLikeUrl for outgoing URLs)', () => {
630+
expect(decodePath('/file%20name').path).toBe('/file name')
631+
})
632+
633+
it('should still decode safe characters', () => {
634+
// Regular letters/unicode should still be decoded
635+
expect(decodePath('/%D1%88%D0%B5%D0%BB%D0%BB%D1%8B').path).toBe(
636+
'/шеллы',
637+
)
638+
})
639+
})
611640
})
612641

613642
/**

0 commit comments

Comments
 (0)