diff --git a/.changeset/opaque-ssr-route-ids.md b/.changeset/opaque-ssr-route-ids.md new file mode 100644 index 00000000000..09af2c7177a --- /dev/null +++ b/.changeset/opaque-ssr-route-ids.md @@ -0,0 +1,5 @@ +--- +'@tanstack/router-core': patch +--- + +Encode dehydrated SSR route identifiers with an opaque URL-safe token so crawlers cannot interpret internal route IDs, manifest route keys, or not-found route IDs as paths. diff --git a/packages/router-core/src/ssr/ssr-client.ts b/packages/router-core/src/ssr/ssr-client.ts index 6b5623eae8e..ddbabe9f06c 100644 --- a/packages/router-core/src/ssr/ssr-client.ts +++ b/packages/router-core/src/ssr/ssr-client.ts @@ -27,6 +27,9 @@ function hydrateMatch( match.ssr = deyhydratedMatch.ssr match.updatedAt = deyhydratedMatch.u match.error = deyhydratedMatch.e + if (isNotFound(match.error) && typeof match.error.routeId === 'string') { + match.error.routeId = hydrateSsrMatchId(match.error.routeId) + } // Only hydrate global-not-found when a defined value is present in the // dehydrated payload. If omitted, preserve the value computed from the // current client location (important for SPA fallback HTML served at unknown @@ -81,6 +84,16 @@ export async function hydrate(router: AnyRouter): Promise { dehydratedRouter.lastMatchId, ) } + if (dehydratedRouter.manifest) { + dehydratedRouter.manifest.routes = Object.fromEntries( + Object.entries(dehydratedRouter.manifest.routes).map( + ([routeId, routeManifest]) => [ + hydrateSsrMatchId(routeId), + routeManifest, + ], + ), + ) + } const { manifest, dehydratedData, lastMatchId } = dehydratedRouter router.ssr = { diff --git a/packages/router-core/src/ssr/ssr-match-id.ts b/packages/router-core/src/ssr/ssr-match-id.ts index 498c3a0c7e4..f90f14aa590 100644 --- a/packages/router-core/src/ssr/ssr-match-id.ts +++ b/packages/router-core/src/ssr/ssr-match-id.ts @@ -1,7 +1,37 @@ +const encodedSsrMatchIdPrefix = '__TSR__' + export function dehydrateSsrMatchId(id: string): string { - return id.replaceAll('/', '\0') + if ( + !id.includes('/') && + !id.includes('\0') && + !id.includes('\uFFFD') && + !id.startsWith(encodedSsrMatchIdPrefix) + ) { + return id + } + + return `${encodedSsrMatchIdPrefix}${btoa(encodeURIComponent(id)) + .replaceAll('+', '-') + .replaceAll('/', '_') + .replaceAll('=', '')}` } export function hydrateSsrMatchId(id: string): string { + if (id.startsWith(encodedSsrMatchIdPrefix)) { + try { + const base64 = id + .slice(encodedSsrMatchIdPrefix.length) + .replaceAll('-', '+') + .replaceAll('_', '/') + const paddedBase64 = base64.padEnd( + base64.length + ((4 - (base64.length % 4)) % 4), + '=', + ) + return decodeURIComponent(atob(paddedBase64)) + } catch { + return id + } + } + return id.replaceAll('\0', '/').replaceAll('\uFFFD', '/') } diff --git a/packages/router-core/src/ssr/ssr-server.ts b/packages/router-core/src/ssr/ssr-server.ts index 2bca7009d9e..1f9d58f8c02 100644 --- a/packages/router-core/src/ssr/ssr-server.ts +++ b/packages/router-core/src/ssr/ssr-server.ts @@ -6,6 +6,7 @@ import { getStylesheetHref, isInlinableStylesheet, } from '../manifest' +import { isNotFound } from '../not-found' import { decodePath } from '../utils' import { createLRUCache } from '../lru-cache' import { rootRouteId } from '../root' @@ -58,7 +59,18 @@ export function dehydrateMatch(match: AnyRouteMatch): DehydratedMatch { for (const [key, shorthand] of properties) { if (match[key] !== undefined) { - dehydratedMatch[shorthand] = match[key] + let value = match[key] + if ( + key === 'error' && + isNotFound(value) && + typeof value.routeId === 'string' + ) { + value = { + ...value, + routeId: dehydrateSsrMatchId(value.routeId), + } + } + dehydratedMatch[shorthand] = value } } if (match.globalNotFound) { @@ -279,6 +291,15 @@ function stripInlinedStylesheetAssets( return nextRoutes } +function dehydrateManifestRouteKeys(routes: FilteredRoutes): FilteredRoutes { + return Object.fromEntries( + Object.entries(routes).map(([routeId, routeManifest]) => [ + dehydrateSsrMatchId(routeId), + routeManifest, + ]), + ) +} + export function attachRouterServerSsrUtils({ router, manifest, @@ -399,13 +420,14 @@ export function attachRouterServerSsrUtils({ } manifestToDehydrate = { - routes: { ...filteredRoutes }, + routes: dehydrateManifestRouteKeys(filteredRoutes), } // Merge request-scoped assets into root route (without mutating cached manifest) if (opts?.requestAssets?.length) { - const existingRoot = manifestToDehydrate.routes[rootRouteId] - manifestToDehydrate.routes[rootRouteId] = { + const dehydratedRootRouteId = dehydrateSsrMatchId(rootRouteId) + const existingRoot = manifestToDehydrate.routes[dehydratedRootRouteId] + manifestToDehydrate.routes[dehydratedRootRouteId] = { ...existingRoot, assets: [...opts.requestAssets, ...(existingRoot?.assets ?? [])], } diff --git a/packages/router-core/tests/hydrate.test.ts b/packages/router-core/tests/hydrate.test.ts index e9bb82018a1..d5104e25116 100644 --- a/packages/router-core/tests/hydrate.test.ts +++ b/packages/router-core/tests/hydrate.test.ts @@ -141,7 +141,13 @@ describe('hydrate', () => { }) it('should set manifest in router.ssr', async () => { - const testManifest = { routes: {} } + const testManifest = { + routes: { + [dehydrateSsrMatchId('/_layout/posts/$postId')]: { + preloads: ['/assets/post.js'], + }, + }, + } mockWindow.$_TSR = { router: { manifest: testManifest, @@ -160,7 +166,13 @@ describe('hydrate', () => { await hydrate(mockRouter) expect(mockRouter.ssr).toEqual({ - manifest: testManifest, + manifest: { + routes: { + '/_layout/posts/$postId': { + preloads: ['/assets/post.js'], + }, + }, + }, }) }) @@ -395,6 +407,56 @@ describe('hydrate', () => { expect((mockRouter.state.matches[0] as AnyRouteMatch).id).toBe('/') }) + it('should decode notFound error route ids during hydration', async () => { + const rawRouteId = '/other' + const mockMatches = [ + { + id: rawRouteId, + routeId: rawRouteId, + index: 0, + ssr: undefined, + _nonReactive: {}, + }, + ] + + mockRouter.matchRoutes = vi.fn().mockReturnValue(mockMatches) + mockRouter.state.matches = mockMatches + + mockWindow.$_TSR = { + router: { + manifest: { routes: {} }, + dehydratedData: {}, + lastMatchId: dehydrateSsrMatchId(rawRouteId), + matches: [ + { + i: dehydrateSsrMatchId(rawRouteId), + e: { + isNotFound: true, + routeId: dehydrateSsrMatchId(rawRouteId), + }, + s: 'notFound', + ssr: true, + u: Date.now(), + }, + ], + }, + h: vi.fn(), + e: vi.fn(), + c: vi.fn(), + p: vi.fn(), + buffer: [], + initialized: false, + } + + await hydrate(mockRouter) + + const match = mockRouter.state.matches[0] as AnyRouteMatch + expect(match.error).toEqual({ + isNotFound: true, + routeId: rawRouteId, + }) + }) + it('should handle errors during route context hydration', async () => { const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) mockHead.mockImplementation(() => { diff --git a/packages/router-core/tests/ssr-match-id.test.ts b/packages/router-core/tests/ssr-match-id.test.ts index 77399edb5aa..c2697f8ecea 100644 --- a/packages/router-core/tests/ssr-match-id.test.ts +++ b/packages/router-core/tests/ssr-match-id.test.ts @@ -2,12 +2,18 @@ import { describe, expect, it } from 'vitest' import { dehydrateSsrMatchId, hydrateSsrMatchId } from '../src/ssr/ssr-match-id' describe('ssr match id codec', () => { - it('removes forward slashes in dehydrated ids', () => { + it('removes crawler-normalizable path separators in dehydrated ids', () => { const dehydratedId = dehydrateSsrMatchId( '/$orgId/projects/$projectId//acme/projects/dashboard/{}', ) + const crawlerNormalized = dehydratedId + .replaceAll('\0', '/') + .replaceAll('\uFFFD', '/') expect(dehydratedId).not.toContain('/') + expect(dehydratedId).not.toContain('\0') + expect(crawlerNormalized).not.toContain('/$orgId') + expect(crawlerNormalized).not.toContain('$projectId') expect(hydrateSsrMatchId(dehydratedId)).toBe( '/$orgId/projects/$projectId//acme/projects/dashboard/{}', ) @@ -23,4 +29,18 @@ describe('ssr match id codec', () => { it('decodes browser-normalized replacement chars back to slashes', () => { expect(hydrateSsrMatchId('\uFFFDposts\uFFFD1')).toBe('/posts/1') }) + + it('round trips ids that start with the encoded prefix', () => { + const id = '__TSR__route-id' + const dehydratedId = dehydrateSsrMatchId(id) + + expect(dehydratedId).not.toBe(id) + expect(hydrateSsrMatchId(dehydratedId)).toBe(id) + }) + + it('leaves invalid encoded-prefix ids unchanged for backwards compatibility', () => { + expect(hydrateSsrMatchId('__TSR__not-valid-base64%')).toBe( + '__TSR__not-valid-base64%', + ) + }) }) diff --git a/packages/router-core/tests/ssr-route-id-dehydration.test.ts b/packages/router-core/tests/ssr-route-id-dehydration.test.ts new file mode 100644 index 00000000000..8a3e303a356 --- /dev/null +++ b/packages/router-core/tests/ssr-route-id-dehydration.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, test } from 'vitest' +import { isNotFound, notFound } from '../src' +import { dehydrateMatch } from '../src/ssr/ssr-server' +import { hydrateSsrMatchId } from '../src/ssr/ssr-match-id' +import type { AnyRouteMatch } from '../src' + +function normalizeCrawlerCandidate(id: string) { + return id.replaceAll('\0', '/').replaceAll('\uFFFD', '/') +} + +describe('SSR route id dehydration', () => { + test('hides internal match ids and notFound route ids from slash-normalizing crawlers', () => { + const routeId = '/_layout/posts/$postId' + const matchId = `${routeId}/posts/1` + const dehydratedMatch = dehydrateMatch({ + id: matchId, + routeId, + updatedAt: 1, + status: 'notFound', + error: notFound({ routeId }), + ssr: true, + _nonReactive: {}, + } as AnyRouteMatch) + + const normalizedMatchId = normalizeCrawlerCandidate(dehydratedMatch.i) + const dehydratedError = dehydratedMatch.e + if ( + !isNotFound(dehydratedError) || + typeof dehydratedError.routeId !== 'string' + ) { + throw new Error('Expected a dehydrated notFound error with a routeId') + } + const normalizedErrorRouteId = normalizeCrawlerCandidate( + dehydratedError.routeId, + ) + + expect(normalizedMatchId).not.toContain('/_layout') + expect(normalizedMatchId).not.toContain('$postId') + expect(normalizedErrorRouteId).not.toContain('/_layout') + expect(normalizedErrorRouteId).not.toContain('$postId') + expect(hydrateSsrMatchId(dehydratedMatch.i)).toBe(matchId) + expect(hydrateSsrMatchId(dehydratedError.routeId)).toBe(routeId) + }) +}) diff --git a/packages/router-core/tests/ssr-server-manifest.test.ts b/packages/router-core/tests/ssr-server-manifest.test.ts index b566f13dc8f..eee8dd78bee 100644 --- a/packages/router-core/tests/ssr-server-manifest.test.ts +++ b/packages/router-core/tests/ssr-server-manifest.test.ts @@ -3,6 +3,7 @@ import { runInNewContext } from 'node:vm' import { BaseRootRoute, BaseRoute } from '../src' import { attachRouterServerSsrUtils } from '../src/ssr/ssr-server' import { GLOBAL_TSR } from '../src/ssr/constants' +import { dehydrateSsrMatchId, hydrateSsrMatchId } from '../src/ssr/ssr-match-id' import { createTestRouter } from './routerTestUtils' import { describe, expect, test } from 'vitest' import type { Manifest } from '../src/manifest' @@ -17,7 +18,7 @@ function buildRouter() { }) const postsRoute = new BaseRoute({ getParentRoute: () => rootRoute, - path: '/posts', + path: '/_layout/posts/$postId', component: () => null, }) @@ -54,6 +55,10 @@ function buildManifest(): Manifest { assets: [sharedAsset], preloads: ['/assets/posts.js'], }, + '/_layout/posts/$postId': { + assets: [sharedAsset], + preloads: ['/assets/post.js'], + }, }, } } @@ -72,6 +77,7 @@ function buildInlineManifest(): Manifest { async function dehydrateManifest(options?: { includeUnmatchedRouteAssets?: boolean + decodeRouteKeys?: boolean }) { const router = buildRouter() const manifest = buildManifest() @@ -89,7 +95,10 @@ async function dehydrateManifest(options?: { expect(script?.tag).toBe('script') expect(script?.children).toBeTruthy() - return parseSerializedRouter(script!.children!).manifest! + const dehydratedManifest = parseSerializedRouter(script!.children!).manifest! + return options?.decodeRouteKeys === false + ? dehydratedManifest + : decodeManifestRouteKeys(dehydratedManifest) } function parseSerializedRouter(serialized: string): DehydratedRouter { @@ -109,6 +118,22 @@ function parseSerializedRouter(serialized: string): DehydratedRouter { return router } +function decodeManifestRouteKeys(manifest: Manifest): Manifest { + return { + ...manifest, + routes: Object.fromEntries( + Object.entries(manifest.routes).map(([routeId, routeManifest]) => [ + hydrateSsrMatchId(routeId), + routeManifest, + ]), + ), + } +} + +function normalizeCrawlerCandidate(id: string) { + return id.replaceAll('\0', '/').replaceAll('\uFFFD', '/') +} + describe('attachRouterServerSsrUtils manifest dehydration', () => { test('includes unmatched route assets by default', async () => { const manifest = await dehydrateManifest() @@ -136,6 +161,25 @@ describe('attachRouterServerSsrUtils manifest dehydration', () => { expect(manifest.routes['/']?.preloads).toEqual(['/assets/index.js']) }) + test('dehydrates manifest route keys without crawler-visible route paths', async () => { + const manifest = await dehydrateManifest({ + decodeRouteKeys: false, + }) + const routeKeys = Object.keys(manifest.routes) + const crawlerNormalizedRouteKeys = routeKeys.map(normalizeCrawlerCandidate) + + expect(manifest.routes['/_layout/posts/$postId']).toBeUndefined() + expect( + manifest.routes[dehydrateSsrMatchId('/_layout/posts/$postId')], + ).toBeDefined() + expect( + crawlerNormalizedRouteKeys.some( + (routeId) => + routeId.includes('/_layout') || routeId.includes('$postId'), + ), + ).toBe(false) + }) + test('inlines stylesheet assets for SSR and strips stylesheet links from dehydration', async () => { const router = buildRouter() const manifest = buildInlineManifest() @@ -163,7 +207,9 @@ describe('attachRouterServerSsrUtils manifest dehydration', () => { const script = router.serverSsr!.takeBufferedScripts() expect(script?.children).toBeTruthy() const dehydratedRouter = parseSerializedRouter(script!.children!) - const dehydratedManifest = dehydratedRouter.manifest! + const dehydratedManifest = decodeManifestRouteKeys( + dehydratedRouter.manifest!, + ) const rootAssets = dehydratedManifest.routes.__root__?.assets ?? [] const allAssets = Object.values(dehydratedManifest.routes).flatMap( (route) => route.assets ?? [],