From 8b7eddb79984d4e1874e12a6755f2452c17face2 Mon Sep 17 00:00:00 2001 From: CDillinger Date: Thu, 25 Jun 2026 22:20:15 -0400 Subject: [PATCH 1/2] 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> --- .changeset/fix-decode-path-preserve-unsafe.md | 9 +++ .../tests/open-redirect-prevention.spec.ts | 7 +- .../tests/open-redirect-prevention.spec.ts | 12 +--- .../basic/tests/special-characters.spec.ts | 18 +++++ packages/router-core/src/utils.ts | 29 +++++--- packages/router-core/tests/utils.test.ts | 69 +++++++++++++------ 6 files changed, 99 insertions(+), 45 deletions(-) create mode 100644 .changeset/fix-decode-path-preserve-unsafe.md diff --git a/.changeset/fix-decode-path-preserve-unsafe.md b/.changeset/fix-decode-path-preserve-unsafe.md new file mode 100644 index 0000000000..a68ae9e3e1 --- /dev/null +++ b/.changeset/fix-decode-path-preserve-unsafe.md @@ -0,0 +1,9 @@ +--- +'@tanstack/router-core': patch +--- + +fix(router-core): re-encode URL-unsafe characters in `sanitizePathSegment` to prevent infinite redirect loops + +`sanitizePathSegment` now re-encodes characters in the WHATWG URL "path percent-encode set" (`<`, `>`, `"`, `` ` ``, `{`, `}`) and ASCII control characters back to their percent-encoded form, instead of stripping control characters. 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`). + +Fixes #7587. diff --git a/e2e/react-router/basic-file-based/tests/open-redirect-prevention.spec.ts b/e2e/react-router/basic-file-based/tests/open-redirect-prevention.spec.ts index 3ad83fb455..2f0fe256cd 100644 --- a/e2e/react-router/basic-file-based/tests/open-redirect-prevention.spec.ts +++ b/e2e/react-router/basic-file-based/tests/open-redirect-prevention.spec.ts @@ -69,10 +69,7 @@ test.describe('Open redirect prevention', () => { page, baseURL, }) => { - // When control characters are stripped from paths like /%0d/evil.com/ - // the result could be //evil.com/ which is a protocol-relative URL - // Our fix collapses these to /evil.com/ to prevent external redirects - // This is already tested above, but we verify the collapsed path works + // %0d is kept encoded, so /%0d/test-path/ stays as-is and won't become //test-path/ await page.goto('/%0d/test-path/') await page.waitForLoadState('networkidle') @@ -80,8 +77,6 @@ test.describe('Open redirect prevention', () => { expect(page.url().startsWith(baseURL!)).toBe(true) const url = new URL(page.url()) expect(url.origin).toBe(new URL(baseURL!).origin) - // Path should be collapsed to /test-path (not //test-path/) - expect(url.pathname).toMatch(/^\/test-path\/?$/) }) }) diff --git a/e2e/react-start/basic/tests/open-redirect-prevention.spec.ts b/e2e/react-start/basic/tests/open-redirect-prevention.spec.ts index e497109889..bc88c33605 100644 --- a/e2e/react-start/basic/tests/open-redirect-prevention.spec.ts +++ b/e2e/react-start/basic/tests/open-redirect-prevention.spec.ts @@ -77,22 +77,14 @@ test.describe('Open redirect prevention', () => { page, baseURL, }) => { - // When control characters are stripped from paths like /%0d/evil.com/ - // the result could be //evil.com/ which is a protocol-relative URL - // Our fix collapses these to /evil.com/ to prevent external redirects - // This is already tested above, but we verify the collapsed path works - const res = await page.goto('/%0d/test-path/') + // %0d is kept encoded, so /%0d/test-path/ stays as-is and won't become //test-path/ + await page.goto('/%0d/test-path/') await page.waitForLoadState('networkidle') // Should stay on the same origin expect(page.url().startsWith(baseURL!)).toBe(true) const url = new URL(page.url()) expect(url.origin).toBe(new URL(baseURL!).origin) - // Path should be collapsed to /test-path (not //test-path/) - expect(url.pathname).toMatch(/^\/test-path\/?$/) - if (!isSpaMode) { - expect(res?.request().redirectedFrom()?.url()).not.toBe(undefined) - } }) }) diff --git a/e2e/react-start/basic/tests/special-characters.spec.ts b/e2e/react-start/basic/tests/special-characters.spec.ts index e88280e2e1..164da2a7e2 100644 --- a/e2e/react-start/basic/tests/special-characters.spec.ts +++ b/e2e/react-start/basic/tests/special-characters.spec.ts @@ -64,6 +64,24 @@ test.describe('Unicode route rendering', () => { expect(param).toBe('대|') }) + + test('should not cause redirect loop for curly braces in path params', async ({ + page, + baseURL, + }) => { + // %7B/%7D (curly braces) must not trigger infinite 307 redirects + const res = await page.goto('/specialChars/%7B%7Bapp_name%7D%7D') + await page.waitForLoadState('load') + + // Should not redirect — URL stays encoded + expect(page.url()).toBe( + `${baseURL}/specialChars/%7B%7Bapp_name%7D%7D`, + ) + expect(res!.status()).toBe(200) + + const param = await page.getByTestId('special-param').textContent() + expect(param).toBe('{{app_name}}') + }) }) test.describe('Special characters in search params', () => { diff --git a/packages/router-core/src/utils.ts b/packages/router-core/src/utils.ts index f017215b5c..a0e4706cb5 100644 --- a/packages/router-core/src/utils.ts +++ b/packages/router-core/src/utils.ts @@ -523,15 +523,25 @@ export function findLast( } /** - * Remove control characters that can cause open redirect vulnerabilities. - * Characters like \r (CR) and \n (LF) can trick URL parsers into interpreting - * paths like "/\r/evil.com" as "http://evil.com". + * Re-encode characters that are unsafe in URL paths. + * Includes ASCII control characters (0x00-0x1F, 0x7F) and a subset of the + * WHATWG URL "path percent-encode set" (", <, >, `, {, }). + * + * Space (0x20) is intentionally excluded — decodeURI decodes %20 to space + * and the router stores decoded spaces in location.pathname. The existing + * encodePathLikeUrl already handles re-encoding spaces for outgoing URLs. + * + * These characters are decoded by decodeURI but must remain percent-encoded + * in paths to match how upstream layers (CDNs, edge middleware, browsers) + * interpret the URL, preventing infinite redirect loops and path mismatches. */ +// eslint-disable-next-line no-control-regex +const PATH_UNSAFE_RE = /[\x00-\x1f\x7f"<>`{}]/g + function sanitizePathSegment(segment: string): string { - // Remove ASCII control characters (0x00-0x1F) and DEL (0x7F) - // These include CR (\r = 0x0D), LF (\n = 0x0A), and other potentially dangerous characters - // eslint-disable-next-line no-control-regex - return segment.replace(/[\x00-\x1f\x7f]/g, '') + return segment.replace(PATH_UNSAFE_RE, (ch) => + '%' + ch.charCodeAt(0).toString(16).toUpperCase().padStart(2, '0'), + ) } function decodeSegment(segment: string): string { @@ -644,8 +654,9 @@ export function decodePath(path: string) { result = result + decodeSegment(cursor ? path.slice(cursor) : path) // Prevent open redirect via protocol-relative URLs (e.g. "//evil.com") - // After sanitizing control characters, paths like "/\r/evil.com" become "//evil.com" - // Collapse leading double slashes to a single slash + // This is defense-in-depth: since control characters are no longer decoded, + // paths like "/%0d/evil.com" can no longer become "//evil.com". But we keep + // this check to guard against other edge cases. let handledProtocolRelativeURL = false if (result.startsWith('//')) { handledProtocolRelativeURL = true diff --git a/packages/router-core/tests/utils.test.ts b/packages/router-core/tests/utils.test.ts index 95874f230e..375a9bffaf 100644 --- a/packages/router-core/tests/utils.test.ts +++ b/packages/router-core/tests/utils.test.ts @@ -547,46 +547,45 @@ describe('decodePath', () => { }) describe('open redirect prevention', () => { - it('should strip CR (%0d) to prevent open redirect', () => { - // /%0d/google.com/ decodes to /\r/google.com/ which becomes //google.com/ - // This must be sanitized to prevent protocol-relative URL interpretation + it('should keep CR (%0d) encoded to prevent open redirect', () => { + // %0d stays encoded — no decoding, no stripping, no path mismatch + // Output is uppercase hex per RFC 3986 const result = decodePath('/%0d/google.com/') - expect(result.path).toBe('/google.com/') + expect(result.path).toBe('/%0D/google.com/') expect(result.path).not.toMatch(/^\/\//) - expect(result.handledProtocolRelativeURL).toBe(true) + expect(result.handledProtocolRelativeURL).toBe(false) }) - it('should strip LF (%0a) to prevent open redirect', () => { + it('should keep LF (%0a) encoded to prevent open redirect', () => { const result = decodePath('/%0a/evil.com/') - expect(result.path).toBe('/evil.com/') + expect(result.path).toBe('/%0A/evil.com/') expect(result.path).not.toMatch(/^\/\//) - expect(result.handledProtocolRelativeURL).toBe(true) + expect(result.handledProtocolRelativeURL).toBe(false) }) - it('should strip CRLF (%0d%0a) to prevent open redirect', () => { + it('should keep CRLF (%0d%0a) encoded to prevent open redirect', () => { const result = decodePath('/%0d%0a/evil.com/') - expect(result.path).toBe('/evil.com/') + expect(result.path).toBe('/%0D%0A/evil.com/') expect(result.path).not.toMatch(/^\/\//) - expect(result.handledProtocolRelativeURL).toBe(true) + expect(result.handledProtocolRelativeURL).toBe(false) }) - it('should strip multiple control characters to prevent open redirect', () => { + it('should keep multiple control characters encoded', () => { const result = decodePath('/%0d%0d%0d/evil.com/') - expect(result.path).toBe('/evil.com/') + expect(result.path).toBe('/%0D%0D%0D/evil.com/') expect(result.path).not.toMatch(/^\/\//) - expect(result.handledProtocolRelativeURL).toBe(true) + expect(result.handledProtocolRelativeURL).toBe(false) }) - it('should strip null bytes and other control characters', () => { + it('should keep null bytes encoded', () => { const result = decodePath('/%00/test/') - expect(result.path).toBe('/test/') - expect(result.handledProtocolRelativeURL).toBe(true) + expect(result.path).toBe('/%00/test/') + expect(result.handledProtocolRelativeURL).toBe(false) }) it('should collapse leading double slashes to prevent protocol-relative URLs', () => { - // After stripping control chars, ensure we don't end up with //evil.com - const result = decodePath('/%0d%0a/evil.com/path') - // Should resolve to localhost, not evil.com + // Direct // input should still be collapsed as defense-in-depth + const result = decodePath('//evil.com/path') const url = new URL(result.path, 'http://localhost:3000') expect(url.origin).toBe('http://localhost:3000') expect(result.handledProtocolRelativeURL).toBe(true) @@ -608,6 +607,36 @@ describe('decodePath', () => { expect(result.handledProtocolRelativeURL).toBe(true) }) }) + + describe('WHATWG path percent-encode set preserved', () => { + it('should keep curly braces encoded', () => { + const result = decodePath('/%7B%7Bapp_name%7D%7D/Makefile') + expect(result.path).toBe('/%7B%7Bapp_name%7D%7D/Makefile') + }) + + it('should keep angle brackets encoded', () => { + expect(decodePath('/%3Ctest%3E').path).toBe('/%3Ctest%3E') + }) + + it('should keep double quotes encoded', () => { + expect(decodePath('/foo%22bar').path).toBe('/foo%22bar') + }) + + it('should keep backticks encoded', () => { + expect(decodePath('/back%60tick').path).toBe('/back%60tick') + }) + + it('should decode space (handled by encodePathLikeUrl for outgoing URLs)', () => { + expect(decodePath('/file%20name').path).toBe('/file name') + }) + + it('should still decode safe characters', () => { + // Regular letters/unicode should still be decoded + expect(decodePath('/%D1%88%D0%B5%D0%BB%D0%BB%D1%8B').path).toBe( + '/шеллы', + ) + }) + }) }) /** From a6398a2de968a2d040bf80a3474e419e20e443b0 Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Tue, 30 Jun 2026 20:17:41 +0000 Subject: [PATCH 2/2] ci: apply automated fixes --- e2e/react-start/basic/tests/special-characters.spec.ts | 4 +--- packages/router-core/src/utils.ts | 5 +++-- packages/router-core/tests/utils.test.ts | 4 +--- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/e2e/react-start/basic/tests/special-characters.spec.ts b/e2e/react-start/basic/tests/special-characters.spec.ts index 164da2a7e2..7cf50c39c8 100644 --- a/e2e/react-start/basic/tests/special-characters.spec.ts +++ b/e2e/react-start/basic/tests/special-characters.spec.ts @@ -74,9 +74,7 @@ test.describe('Unicode route rendering', () => { await page.waitForLoadState('load') // Should not redirect — URL stays encoded - expect(page.url()).toBe( - `${baseURL}/specialChars/%7B%7Bapp_name%7D%7D`, - ) + expect(page.url()).toBe(`${baseURL}/specialChars/%7B%7Bapp_name%7D%7D`) expect(res!.status()).toBe(200) const param = await page.getByTestId('special-param').textContent() diff --git a/packages/router-core/src/utils.ts b/packages/router-core/src/utils.ts index a0e4706cb5..91011bfa7f 100644 --- a/packages/router-core/src/utils.ts +++ b/packages/router-core/src/utils.ts @@ -539,8 +539,9 @@ export function findLast( const PATH_UNSAFE_RE = /[\x00-\x1f\x7f"<>`{}]/g function sanitizePathSegment(segment: string): string { - return segment.replace(PATH_UNSAFE_RE, (ch) => - '%' + ch.charCodeAt(0).toString(16).toUpperCase().padStart(2, '0'), + return segment.replace( + PATH_UNSAFE_RE, + (ch) => '%' + ch.charCodeAt(0).toString(16).toUpperCase().padStart(2, '0'), ) } diff --git a/packages/router-core/tests/utils.test.ts b/packages/router-core/tests/utils.test.ts index 375a9bffaf..fff911df37 100644 --- a/packages/router-core/tests/utils.test.ts +++ b/packages/router-core/tests/utils.test.ts @@ -632,9 +632,7 @@ describe('decodePath', () => { it('should still decode safe characters', () => { // Regular letters/unicode should still be decoded - expect(decodePath('/%D1%88%D0%B5%D0%BB%D0%BB%D1%8B').path).toBe( - '/шеллы', - ) + expect(decodePath('/%D1%88%D0%B5%D0%BB%D0%BB%D1%8B').path).toBe('/шеллы') }) }) })