Skip to content

Commit 7123395

Browse files
CDillingerCopilot
andcommitted
fix(router-core): preserve percent-encoded URL-unsafe chars in decodeSegment
Replace decodeURI()-based decoding with per-character decoding that preserves the WHATWG URL path percent-encode set ({, }, <, >, ", `) and ASCII control characters (0x00-0x1F, 0x7F) in their encoded form. Previously, decodeSegment decoded these characters and sanitizePathSegment stripped the control chars. This created mismatches between the original request URL and the router's internal representation, causing infinite 307 redirect loops on paths like /%7B%7Btemplate%7D%7D. By keeping these characters encoded, the router's path representation matches what browsers and upstream layers send, eliminating the mismatch that triggered spurious redirects. Fixes #7587. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 3d86aee commit 7123395

5 files changed

Lines changed: 94 additions & 42 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-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: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -523,16 +523,16 @@ 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+
* Characters that must remain percent-encoded in URL paths.
527+
* Includes the WHATWG URL "path percent-encode set" (space, ", <, >, `, {, })
528+
* and ASCII control characters (0x00-0x1F, 0x7F).
529+
*
530+
* Decoding these creates mismatches between how upstream layers (CDNs, edge
531+
* middleware, browsers) interpret the URL and how the router interprets it,
532+
* leading to infinite redirect loops and access control bypasses.
529533
*/
530-
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, '')
535-
}
534+
// eslint-disable-next-line no-control-regex
535+
const PATH_UNSAFE_RE = /[\x00-\x1f\x7f\x20"<>`{}]/g
536536

537537
function decodeSegment(segment: string): string {
538538
let decoded: string
@@ -548,7 +548,10 @@ function decodeSegment(segment: string): string {
548548
}
549549
})
550550
}
551-
return sanitizePathSegment(decoded)
551+
// Re-encode characters that are unsafe in URL paths instead of stripping them
552+
return decoded.replace(PATH_UNSAFE_RE, (ch) =>
553+
'%' + ch.charCodeAt(0).toString(16).toUpperCase().padStart(2, '0'),
554+
)
552555
}
553556

554557
/**
@@ -644,8 +647,9 @@ export function decodePath(path: string) {
644647
result = result + decodeSegment(cursor ? path.slice(cursor) : path)
645648

646649
// 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
650+
// This is defense-in-depth: since control characters are no longer decoded,
651+
// paths like "/%0d/evil.com" can no longer become "//evil.com". But we keep
652+
// this check to guard against other edge cases.
649653
let handledProtocolRelativeURL = false
650654
if (result.startsWith('//')) {
651655
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 keep space encoded', () => {
630+
expect(decodePath('/file%20name').path).toBe('/file%20name')
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)