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 4267de8b645..e4971098893 100644 --- a/e2e/react-start/basic/tests/open-redirect-prevention.spec.ts +++ b/e2e/react-start/basic/tests/open-redirect-prevention.spec.ts @@ -1,5 +1,6 @@ import { expect } from '@playwright/test' import { test } from '@tanstack/router-e2e-utils' +import { isSpaMode } from './utils/isSpaMode' test.use({ whitelistErrors: [ @@ -80,7 +81,7 @@ test.describe('Open redirect prevention', () => { // 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 - await page.goto('/%0d/test-path/') + const res = await page.goto('/%0d/test-path/') await page.waitForLoadState('networkidle') // Should stay on the same origin @@ -89,6 +90,9 @@ test.describe('Open redirect prevention', () => { 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/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index 6520f307994..d677f5530ea 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -1268,14 +1268,14 @@ export class RouterCore< return { href: pathname + searchStr + hash, publicHref: href, - pathname: decodePath(pathname), + pathname: decodePath(pathname).path, external: false, searchStr, search: replaceEqualDeep( previousLocation?.search, parsedSearch, ) as any, - hash: decodePath(hash.slice(1)), + hash: decodePath(hash.slice(1)).path, state: replaceEqualDeep(previousLocation?.state, state), } } @@ -1297,11 +1297,11 @@ export class RouterCore< return { href: fullPath, publicHref: href, - pathname: decodePath(url.pathname), + pathname: decodePath(url.pathname).path, external: !!this.rewrite && url.origin !== this.origin, searchStr, search: replaceEqualDeep(previousLocation?.search, parsedSearch) as any, - hash: decodePath(url.hash.slice(1)), + hash: decodePath(url.hash.slice(1)).path, state: replaceEqualDeep(previousLocation?.state, state), } } @@ -1879,7 +1879,7 @@ export class RouterCore< decoder: this.pathParamsDecoder, server: this.isServer, }).interpolatedPath, - ) + ).path // Resolve the next search let nextSearch = fromSearch diff --git a/packages/router-core/src/ssr/createRequestHandler.ts b/packages/router-core/src/ssr/createRequestHandler.ts index fe1b2d71c2c..6fe0d68d4ac 100644 --- a/packages/router-core/src/ssr/createRequestHandler.ts +++ b/packages/router-core/src/ssr/createRequestHandler.ts @@ -35,7 +35,7 @@ export function createRequestHandler({ }) // normalizing and sanitizing the pathname here for server, so we always deal with the same format during SSR. - const url = getNormalizedURL(request.url, 'http://localhost') + const { url } = getNormalizedURL(request.url, 'http://localhost') const origin = getOrigin(request) const href = url.href.replace(url.origin, '') diff --git a/packages/router-core/src/ssr/ssr-server.ts b/packages/router-core/src/ssr/ssr-server.ts index b98516e9d51..2c531de8ab5 100644 --- a/packages/router-core/src/ssr/ssr-server.ts +++ b/packages/router-core/src/ssr/ssr-server.ts @@ -398,7 +398,9 @@ export function getNormalizedURL(url: string | URL, base?: string | URL) { if (typeof url === 'string') url = url.replace('\\', '%5C') const rawUrl = new URL(url, base) - const decodedPathname = decodePath(rawUrl.pathname) + const { path: decodedPathname, handledProtocolRelativeURL } = decodePath( + rawUrl.pathname, + ) const searchParams = new URLSearchParams(rawUrl.search) const normalizedHref = decodedPathname + @@ -406,5 +408,8 @@ export function getNormalizedURL(url: string | URL, base?: string | URL) { searchParams.toString() + rawUrl.hash - return new URL(normalizedHref, rawUrl.origin) + return { + url: new URL(normalizedHref, rawUrl.origin), + handledProtocolRelativeURL, + } } diff --git a/packages/router-core/src/utils.ts b/packages/router-core/src/utils.ts index eac67cce034..8495477df09 100644 --- a/packages/router-core/src/utils.ts +++ b/packages/router-core/src/utils.ts @@ -593,8 +593,8 @@ export function escapeHtml(str: string): string { return str.replace(HTML_ESCAPE_REGEX, (match) => HTML_ESCAPE_LOOKUP[match]!) } -export function decodePath(path: string, decodeIgnore?: Array): string { - if (!path) return path +export function decodePath(path: string) { + if (!path) return { path, handledProtocolRelativeURL: false } // Fast path: most paths are already decoded and safe. // Only fall back to the slower scan/regex path when we see a '%' (encoded), @@ -602,12 +602,10 @@ export function decodePath(path: string, decodeIgnore?: Array): string { // prefix which needs collapsing. // eslint-disable-next-line no-control-regex if (!/[%\\\x00-\x1f\x7f]/.test(path) && !path.startsWith('//')) { - return path + return { path, handledProtocolRelativeURL: false } } - const re = decodeIgnore - ? new RegExp(`${decodeIgnore.join('|')}`, 'gi') - : /%25|%5C/gi + const re = /%25|%5C/gi let cursor = 0 let result = '' let match @@ -620,11 +618,13 @@ export function decodePath(path: string, decodeIgnore?: Array): string { // 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 + let handledProtocolRelativeURL = false if (result.startsWith('//')) { + handledProtocolRelativeURL = true result = '/' + result.replace(/^\/+/, '') } - return result + return { path: result, handledProtocolRelativeURL } } /** diff --git a/packages/router-core/tests/getNormalizedURL.test.ts b/packages/router-core/tests/getNormalizedURL.test.ts index 6af6b2736a6..d459c8b4185 100644 --- a/packages/router-core/tests/getNormalizedURL.test.ts +++ b/packages/router-core/tests/getNormalizedURL.test.ts @@ -9,12 +9,12 @@ describe('getNormalizedURL', () => { const normalizedUrl1 = getNormalizedURL(url1) const normalizedUrl2 = getNormalizedURL(url2) - expect(normalizedUrl1.pathname).toBe('/%EB%8C%80|/path') - expect(normalizedUrl1.pathname).toBe(normalizedUrl2.pathname) + expect(normalizedUrl1.url.pathname).toBe('/%EB%8C%80|/path') + expect(normalizedUrl1.url.pathname).toBe(normalizedUrl2.url.pathname) expect(new URL(url1).pathname).not.toBe(new URL(url2).pathname) - expect(normalizedUrl1.search).toBe(`?query=%EB%8C%80%7C`) - expect(normalizedUrl1.search).toBe(normalizedUrl2.search) + expect(normalizedUrl1.url.search).toBe(`?query=%EB%8C%80%7C`) + expect(normalizedUrl1.url.search).toBe(normalizedUrl2.url.search) expect(new URL(url1).search).not.toBe(new URL(url2).search) }) @@ -120,9 +120,9 @@ describe('getNormalizedURL', () => { 'should treat encoded URL specific characters correctly', ({ url, expectedPathName, expectedHash, expectedSearchParams }) => { const normalizedUrl = getNormalizedURL(url) - expect(normalizedUrl.pathname).toBe(expectedPathName) - expect(normalizedUrl.search).toBe(expectedSearchParams) - expect(normalizedUrl.hash).toBe(expectedHash) + expect(normalizedUrl.url.pathname).toBe(expectedPathName) + expect(normalizedUrl.url.search).toBe(expectedSearchParams) + expect(normalizedUrl.url.hash).toBe(expectedHash) }, ) }) diff --git a/packages/router-core/tests/utils.test.ts b/packages/router-core/tests/utils.test.ts index ea466a0ec43..95874f230e3 100644 --- a/packages/router-core/tests/utils.test.ts +++ b/packages/router-core/tests/utils.test.ts @@ -501,81 +501,12 @@ describe('deepEqual', () => { }) describe('decodePath', () => { - it('should decode a path segment with no ignored items existing', () => { - const itemsToCheck = ['%25', '%5C'] - const stringToCheck = - 'https://mozilla.org/?x=%D1%88%D0%B5%D0%BB%D0%BB%D1%8B' - const expectedResult = 'https://mozilla.org/?x=шеллы' - - const result = decodePath(stringToCheck, itemsToCheck) - - expect(result).toBe(expectedResult) - }) - - it('should decode a path segment with one ignored character and one ignored item existing', () => { - const itemsToCheck = ['%25'] - const stringToCheck = - 'https://mozilla.org/?x=%25%D1%88%D0%B5%5C%D0%BB%D0%BB%D1%8B' - const expectedResult = 'https://mozilla.org/?x=%25ше\\ллы' - - const result = decodePath(stringToCheck, itemsToCheck) - - expect(result).toBe(expectedResult) - }) - - it('should decode a path segment with multiple ignored characters and first ignored item existing', () => { - const itemsToCheck = ['%25', '%5C'] - let stringToCheck = - 'https://mozilla.org/?x=%25%D1%88%D0%B5%D0%BB%D0%BB%D1%8B' - let expectedResult = 'https://mozilla.org/?x=%25шеллы' - - let result = decodePath(stringToCheck, itemsToCheck) - - expect(result).toBe(expectedResult) - - stringToCheck = 'https://mozilla.org/?x=%D1%88%D0%B5%5C%D0%BB%D0%BB%D1%8B' - expectedResult = 'https://mozilla.org/?x=ше%5Cллы' - - result = decodePath(stringToCheck, itemsToCheck) - - expect(result).toBe(expectedResult) - }) - - it('should decode a path segment with multiple ignored characters and other ignored item existing', () => { - const itemsToCheck = ['%25', '%5C'] - let stringToCheck = - 'https://mozilla.org/?x=%5C%D1%88%D0%B5%D0%BB%D0%BB%D1%8B' - let expectedResult = 'https://mozilla.org/?x=%5Cшеллы' - - let result = decodePath(stringToCheck, itemsToCheck) - - expect(result).toBe(expectedResult) - - stringToCheck = 'https://mozilla.org/?x=%D1%88%D0%B5%5C%D0%BB%D0%BB%D1%8B' - expectedResult = 'https://mozilla.org/?x=ше%5Cллы' - - result = decodePath(stringToCheck, itemsToCheck) - - expect(result).toBe(expectedResult) - }) - - it('should decode a path segment with multiple ignored characters and multiple ignored items existing', () => { - const itemsToCheck = ['%25', '%5C'] - const stringToCheck = - 'https://mozilla.org/?x=%25%D1%88%D0%B5%5C%D0%BB%D0%BB%D1%8B' - const expectedResult = 'https://mozilla.org/?x=%25ше%5Cллы' - - const result = decodePath(stringToCheck, itemsToCheck) - - expect(result).toBe(expectedResult) - }) - it('should decode a path segment, ignoring `%` and `\\` by default, with multiple ignored items existing', () => { const stringToCheck = 'https://mozilla.org/?x=%25%D1%88%D0%B5%5C%D0%BB%D0%BB%D1%8B%2F' const expectedResult = 'https://mozilla.org/?x=%25ше%5Cллы%2F' - const result = decodePath(stringToCheck) + const result = decodePath(stringToCheck).path expect(result).toBe(expectedResult) }) @@ -583,34 +514,34 @@ describe('decodePath', () => { it('should handle malformed percent-encodings gracefully', () => { const stringToCheck = 'path%ZZ%D1%88test%5C%C3%A9' // Malformed sequences should remain as-is, valid ones decoded - const result = decodePath(stringToCheck) + const result = decodePath(stringToCheck).path expect(result).toBe(`path%ZZ%D1%88test%5Cé`) }) it('should return empty string unchanged', () => { - expect(decodePath('')).toBe('') + expect(decodePath('').path).toBe('') }) it('should return strings without encoding unchanged', () => { const stringToCheck = 'plain-text-path' - expect(decodePath(stringToCheck)).toBe(stringToCheck) + expect(decodePath(stringToCheck).path).toBe(stringToCheck) }) it('should handle consecutive ignored characters', () => { const stringToCheck = 'test%25%25end' const expectedResult = 'test%25%25end' - expect(decodePath(stringToCheck)).toBe(expectedResult) + expect(decodePath(stringToCheck).path).toBe(expectedResult) }) it('should handle multiple ignored items of the same type with varying case', () => { const stringToCheck = '/params-ps/named/foo%2Fabc/c%2Fh' const expectedResult = '/params-ps/named/foo%2Fabc/c%2Fh' - expect(decodePath(stringToCheck)).toBe(expectedResult) + expect(decodePath(stringToCheck).path).toBe(expectedResult) const stringToCheckWithLowerCase = '/params-ps/named/foo%2Fabc/c%5C%2f%5cAh' const expectedResultWithLowerCase = '/params-ps/named/foo%2Fabc/c%5C%2f%5cAh' - expect(decodePath(stringToCheckWithLowerCase)).toBe( + expect(decodePath(stringToCheckWithLowerCase).path).toBe( expectedResultWithLowerCase, ) }) @@ -620,49 +551,61 @@ describe('decodePath', () => { // /%0d/google.com/ decodes to /\r/google.com/ which becomes //google.com/ // This must be sanitized to prevent protocol-relative URL interpretation const result = decodePath('/%0d/google.com/') - expect(result).toBe('/google.com/') - expect(result).not.toMatch(/^\/\//) + expect(result.path).toBe('/google.com/') + expect(result.path).not.toMatch(/^\/\//) + expect(result.handledProtocolRelativeURL).toBe(true) }) it('should strip LF (%0a) to prevent open redirect', () => { const result = decodePath('/%0a/evil.com/') - expect(result).toBe('/evil.com/') - expect(result).not.toMatch(/^\/\//) + expect(result.path).toBe('/evil.com/') + expect(result.path).not.toMatch(/^\/\//) + expect(result.handledProtocolRelativeURL).toBe(true) }) it('should strip CRLF (%0d%0a) to prevent open redirect', () => { const result = decodePath('/%0d%0a/evil.com/') - expect(result).toBe('/evil.com/') - expect(result).not.toMatch(/^\/\//) + expect(result.path).toBe('/evil.com/') + expect(result.path).not.toMatch(/^\/\//) + expect(result.handledProtocolRelativeURL).toBe(true) }) it('should strip multiple control characters to prevent open redirect', () => { const result = decodePath('/%0d%0d%0d/evil.com/') - expect(result).toBe('/evil.com/') - expect(result).not.toMatch(/^\/\//) + expect(result.path).toBe('/evil.com/') + expect(result.path).not.toMatch(/^\/\//) + expect(result.handledProtocolRelativeURL).toBe(true) }) it('should strip null bytes and other control characters', () => { const result = decodePath('/%00/test/') - expect(result).toBe('/test/') + expect(result.path).toBe('/test/') + expect(result.handledProtocolRelativeURL).toBe(true) }) 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 - const url = new URL(result, 'http://localhost:3000') + const url = new URL(result.path, 'http://localhost:3000') expect(url.origin).toBe('http://localhost:3000') + expect(result.handledProtocolRelativeURL).toBe(true) }) it('should handle normal paths unchanged', () => { - expect(decodePath('/users/profile/')).toBe('/users/profile/') - expect(decodePath('/api/v1/data')).toBe('/api/v1/data') + expect(decodePath('/users/profile/').path).toBe('/users/profile/') + expect(decodePath('/users/profile/').handledProtocolRelativeURL).toBe( + false, + ) + expect(decodePath('/api/v1/data').path).toBe('/api/v1/data') + expect(decodePath('/api/v1/data').handledProtocolRelativeURL).toBe(false) }) it('should handle double slash only input', () => { // Direct // input should also be collapsed - expect(decodePath('//')).toBe('/') + const result = decodePath('//') + expect(result.path).toBe('/') + expect(result.handledProtocolRelativeURL).toBe(true) }) }) }) diff --git a/packages/start-server-core/src/createStartHandler.ts b/packages/start-server-core/src/createStartHandler.ts index 7fa0e4a4843..fabda334be0 100644 --- a/packages/start-server-core/src/createStartHandler.ts +++ b/packages/start-server-core/src/createStartHandler.ts @@ -218,10 +218,16 @@ export function createStartHandler( try { // normalizing and sanitizing the pathname here for server, so we always deal with the same format during SSR. - const url = getNormalizedURL(request.url) + // during normalization paths like '//posts' are flattened to '/posts'. + // in these cases we would prefer to redirect to the new path + const { url, handledProtocolRelativeURL } = getNormalizedURL(request.url) const href = url.pathname + url.search + url.hash const origin = getOrigin(request) + if (handledProtocolRelativeURL) { + return Response.redirect(url, 308) + } + const entries = await getEntries() const startOptions: AnyStartInstanceOptions = (await entries.startEntry.startInstance?.getOptions()) ||