diff --git a/packages/router-core/src/path.ts b/packages/router-core/src/path.ts index da410e9c7d..26b55b4b41 100644 --- a/packages/router-core/src/path.ts +++ b/packages/router-core/src/path.ts @@ -112,13 +112,25 @@ export function resolvePath({ trailingSlash = 'never', cache, }: ResolvePathOptions) { - const isBase = to === '.' - const isAbsolute = to.startsWith('/') + if (to.includes('//')) { + to = cleanPath(to) + } + + if (to.startsWith('/')) { + if (to.length === 1 || trailingSlash === 'preserve') { + return to + } + if (trailingSlash === 'always') { + return to.endsWith('/') ? to : `${to}/` + } + return to.endsWith('/') ? to.slice(0, -1) : to + } + const isBase = to === '.' let key if (cache) { // `trailingSlash` is static per router, so it doesn't need to be part of the cache key - key = isAbsolute ? to : isBase ? base : base + '\0' + to + key = isBase ? base : base + '\0' + to const cached = cache.get(key) if (cached) return cached } @@ -126,9 +138,10 @@ export function resolvePath({ let baseSegments: Array if (isBase) { baseSegments = base.split('/') - } else if (isAbsolute) { - baseSegments = to.split('/') } else { + if (base.includes('//')) { + base = cleanPath(base) + } baseSegments = base.split('/') while (baseSegments.length > 1 && last(baseSegments) === '') { baseSegments.pop() @@ -171,7 +184,8 @@ export function resolvePath({ } } - const result = cleanPath(baseSegments.join('/')) || '/' + const joined = baseSegments.join('/') + const result = (isBase ? cleanPath(joined) : joined) || '/' if (key && cache) cache.set(key, result) return result } diff --git a/packages/router-core/src/router.ts b/packages/router-core/src/router.ts index b473339c70..372f87a22b 100644 --- a/packages/router-core/src/router.ts +++ b/packages/router-core/src/router.ts @@ -22,7 +22,6 @@ import { processRouteTree, } from './new-process-route-tree' import { - cleanPath, compileDecodeCharMap, interpolatePath, resolvePath, @@ -1469,7 +1468,7 @@ export class RouterCore< resolvePathWithBase = (from: string, path: string) => { return resolvePath({ base: from, - to: path.includes('//') ? cleanPath(path) : path, + to: path, trailingSlash: this.options.trailingSlash, cache: this.resolvePathCache, }) @@ -1892,28 +1891,19 @@ export class RouterCore< dest.unsafeRelative === 'path' ? currentLocation.pathname : (dest.from ?? lightweightResult[1 /* fullPath */]) - const destTo = dest.to ? `${dest.to}` : undefined // From search should always use the current location const fromSearch = lightweightResult[2 /* search */] // Same with params. It can't hurt to provide as many as possible - const fromParams = Object.assign( - Object.create(null), - lightweightResult[3 /* params */], - ) + const fromParams = lightweightResult[3 /* params */] - const isAbsoluteTo = destTo?.charCodeAt(0) === 47 - const sourcePath = isAbsoluteTo - ? '/' - : this.resolvePathWithBase(defaultedFromPath, '.') - - // Resolve the destination. Absolute destinations don't need the source path. - const nextTo = destTo - ? this.resolvePathWithBase(sourcePath, destTo) - : sourcePath + const nextTo = this.resolvePathWithBase( + defaultedFromPath, + dest.to ? `${dest.to}` : '.', + ) // Resolve the next params - const nextParams = resolveNextParams(dest.params, fromParams) + let nextParams = resolveNextParams(dest.params, fromParams) const destRoute = this.routesByPath[ trimPathRight(nextTo) as keyof typeof this.routesByPath @@ -1945,6 +1935,9 @@ export class RouterCore< const fn = route.options.params?.stringify ?? route.options.stringifyParams if (fn) { + if (nextParams === fromParams) { + nextParams = Object.assign(Object.create(null), nextParams) + } try { Object.assign(nextParams, fn(nextParams)) } catch { @@ -2864,11 +2857,14 @@ function resolveNextParams( spec: unknown, base: Record, ): Record { - return spec === false || spec === null - ? Object.create(null) - : (spec ?? true) === true - ? base - : Object.assign(base, functionalUpdate(spec as any, base)) + if (spec === false || spec === null) { + return Object.create(null) + } + if ((spec ?? true) === true) { + return base + } + const next = Object.assign(Object.create(null), base) + return Object.assign(next, functionalUpdate(spec as any, next)) } function extractStrictParams( diff --git a/packages/router-core/tests/build-location.test.ts b/packages/router-core/tests/build-location.test.ts index 14ab661ff6..c6fa108ef8 100644 --- a/packages/router-core/tests/build-location.test.ts +++ b/packages/router-core/tests/build-location.test.ts @@ -1300,6 +1300,39 @@ describe('buildLocation - relative paths', () => { expect(location.pathname).toBe('/a/d') }) + test('unsafe path-relative navigation ignores repeated slash segments', async () => { + const rootRoute = new BaseRootRoute({}) + const aRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/a', + }) + const bRoute = new BaseRoute({ + getParentRoute: () => aRoute, + path: '/b', + }) + const cRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/c', + }) + const routeTree = rootRoute.addChildren([ + aRoute.addChildren([bRoute]), + cRoute, + ]) + const router = createTestRouter({ + routeTree, + history: createMemoryHistory({ initialEntries: ['/a//b'] }), + }) + + await router.load() + + expect( + router.buildLocation({ + to: '../../c', + unsafeRelative: 'path', + }).pathname, + ).toBe('/c') + }) + test('over-root traversal stays rooted for javascript-like segments', async () => { const rootRoute = new BaseRootRoute({}) const indexRoute = new BaseRoute({ @@ -1613,6 +1646,37 @@ describe('buildLocation - params edge cases', () => { expect(location.pathname).toBe('/users/000042') }) + test('params.stringify should not mutate current params', async () => { + const rootRoute = new BaseRootRoute({}) + const userRoute = new BaseRoute({ + getParentRoute: () => rootRoute, + path: '/users/$userId', + params: { + parse: ({ userId }: { userId: string }) => ({ + userId: parseInt(userId, 10), + }), + stringify: (params: { userId: number }) => { + const userId = params.userId + params.userId = 999 + return { userId: String(userId).padStart(6, '0') } + }, + }, + }) + + const routeTree = rootRoute.addChildren([userRoute]) + const router = createTestRouter({ + routeTree, + history: createMemoryHistory({ initialEntries: ['/users/000123'] }), + }) + + await router.load() + + expect(router.buildLocation({ to: '/users/$userId' }).pathname).toBe( + '/users/000123', + ) + expect(router.state.matches.at(-1)?.params).toEqual({ userId: 123 }) + }) + test('params.stringify should run for params.parse route templates', async () => { const rootRoute = new BaseRootRoute({}) const languageRoute = new BaseRoute({ diff --git a/packages/router-core/tests/path.test.ts b/packages/router-core/tests/path.test.ts index 658bbcaa38..49c8f022e7 100644 --- a/packages/router-core/tests/path.test.ts +++ b/packages/router-core/tests/path.test.ts @@ -109,6 +109,8 @@ describe('resolvePath', () => { ['/a/b/c', '../..', '/a'], ['/a/b/c', '../../..', '/'], ['/a/b/c/', '../../..', '/'], + ['/a//b', '../../c', '/c'], + ['/a///b', '../c', '/a/c'], ['/', '../javascript:alert(1)', '/javascript:alert(1)'], ['/posts', '../../data:text/html,test', '/data:text/html,test'], ])('resolves correctly', (a, b, eq) => { @@ -123,6 +125,10 @@ describe('resolvePath', () => { }) }) + it('normalizes repeated slashes when resolving the base path', () => { + expect(resolvePath({ base: '/a//b', to: '.' })).toBe('/a/b') + }) + describe('trailingSlash', () => { describe(`'always'`, () => { it('keeps trailing slash', () => { @@ -184,6 +190,17 @@ describe('resolvePath', () => { ).toBe('/a/b/c/d') }) }) + + it.each([ + ['always', '/a//b', '/a/b/'], + ['never', '/a//b///', '/a/b'], + ['preserve', '/a//b///', '/a/b/'], + ] as const)( + "normalizes repeated slashes with trailingSlash '%s'", + (trailingSlash, to, expected) => { + expect(resolvePath({ base: '/', to, trailingSlash })).toBe(expected) + }, + ) }) describe.each([{ base: '/' }, { base: '/nested' }])(