Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/fix-decode-path-preserve-unsafe.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -69,19 +69,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
// %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\/?$/)
})
})

Expand Down
12 changes: 2 additions & 10 deletions e2e/react-start/basic/tests/open-redirect-prevention.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
})

Expand Down
16 changes: 16 additions & 0 deletions e2e/react-start/basic/tests/special-characters.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,22 @@ 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', () => {
Expand Down
30 changes: 21 additions & 9 deletions packages/router-core/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -523,15 +523,26 @@ export function findLast<T>(
}

/**
* 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'),
)
Comment on lines 541 to +545

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.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Mixed malformed + valid UTF-8 sequences are still broken.

This re-encode step only helps after decodeURI(segment) succeeds. If one malformed escape makes that throw, the fallback still decodes %XX byte-by-byte, so a later valid multibyte run in the same segment stays incorrectly encoded instead of being decoded and preserved. That misses the “decode contiguous runs and continue” requirement from the review thread and can still change route matching/param extraction on mixed-validity paths.

🤖 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` around lines 541 - 544, The fallback in
sanitizePathSegment still fails on mixed malformed and valid UTF-8 because it
decodes %XX byte-by-byte after decodeURI throws, leaving later multibyte runs
incorrectly encoded. Update the decoding logic in sanitizePathSegment (and its
PATH_UNSAFE_RE-based fallback path) to process contiguous valid percent-encoded
runs as a unit, preserve valid decoded bytes, and continue past malformed bytes
instead of falling back to per-byte behavior.

}

function decodeSegment(segment: string): string {
Expand Down Expand Up @@ -644,8 +655,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
Expand Down
67 changes: 47 additions & 20 deletions packages/router-core/tests/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -608,6 +607,34 @@ 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('/шеллы')
})
})
})

/**
Expand Down
Loading