diff --git a/.changeset/tame-islands-play.md b/.changeset/tame-islands-play.md new file mode 100644 index 00000000000..40588c17eee --- /dev/null +++ b/.changeset/tame-islands-play.md @@ -0,0 +1,6 @@ +--- +'@tanstack/start-client-core': patch +'@tanstack/start-server-core': patch +--- + +Ensure request middleware context wins over colliding client-provided context in server function execution paths, including SSR, GET, and FormData requests. diff --git a/e2e/react-start/server-functions-global-middleware/src/routeTree.gen.ts b/e2e/react-start/server-functions-global-middleware/src/routeTree.gen.ts index 425fc58a24c..6fb74e28a27 100644 --- a/e2e/react-start/server-functions-global-middleware/src/routeTree.gen.ts +++ b/e2e/react-start/server-functions-global-middleware/src/routeTree.gen.ts @@ -12,6 +12,7 @@ import { Route as rootRouteImport } from './routes/__root' import { Route as SimpleRouteImport } from './routes/simple' import { Route as PathnameMiddlewareRouteImport } from './routes/pathname-middleware' import { Route as MultipleServerFunctionsRouteImport } from './routes/multiple-server-functions' +import { Route as ContextCollisionRouteImport } from './routes/context-collision' import { Route as IndexRouteImport } from './routes/index' const SimpleRoute = SimpleRouteImport.update({ @@ -29,6 +30,11 @@ const MultipleServerFunctionsRoute = MultipleServerFunctionsRouteImport.update({ path: '/multiple-server-functions', getParentRoute: () => rootRouteImport, } as any) +const ContextCollisionRoute = ContextCollisionRouteImport.update({ + id: '/context-collision', + path: '/context-collision', + getParentRoute: () => rootRouteImport, +} as any) const IndexRoute = IndexRouteImport.update({ id: '/', path: '/', @@ -37,12 +43,14 @@ const IndexRoute = IndexRouteImport.update({ export interface FileRoutesByFullPath { '/': typeof IndexRoute + '/context-collision': typeof ContextCollisionRoute '/multiple-server-functions': typeof MultipleServerFunctionsRoute '/pathname-middleware': typeof PathnameMiddlewareRoute '/simple': typeof SimpleRoute } export interface FileRoutesByTo { '/': typeof IndexRoute + '/context-collision': typeof ContextCollisionRoute '/multiple-server-functions': typeof MultipleServerFunctionsRoute '/pathname-middleware': typeof PathnameMiddlewareRoute '/simple': typeof SimpleRoute @@ -50,6 +58,7 @@ export interface FileRoutesByTo { export interface FileRoutesById { __root__: typeof rootRouteImport '/': typeof IndexRoute + '/context-collision': typeof ContextCollisionRoute '/multiple-server-functions': typeof MultipleServerFunctionsRoute '/pathname-middleware': typeof PathnameMiddlewareRoute '/simple': typeof SimpleRoute @@ -58,14 +67,21 @@ export interface FileRouteTypes { fileRoutesByFullPath: FileRoutesByFullPath fullPaths: | '/' + | '/context-collision' | '/multiple-server-functions' | '/pathname-middleware' | '/simple' fileRoutesByTo: FileRoutesByTo - to: '/' | '/multiple-server-functions' | '/pathname-middleware' | '/simple' + to: + | '/' + | '/context-collision' + | '/multiple-server-functions' + | '/pathname-middleware' + | '/simple' id: | '__root__' | '/' + | '/context-collision' | '/multiple-server-functions' | '/pathname-middleware' | '/simple' @@ -73,6 +89,7 @@ export interface FileRouteTypes { } export interface RootRouteChildren { IndexRoute: typeof IndexRoute + ContextCollisionRoute: typeof ContextCollisionRoute MultipleServerFunctionsRoute: typeof MultipleServerFunctionsRoute PathnameMiddlewareRoute: typeof PathnameMiddlewareRoute SimpleRoute: typeof SimpleRoute @@ -101,6 +118,13 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof MultipleServerFunctionsRouteImport parentRoute: typeof rootRouteImport } + '/context-collision': { + id: '/context-collision' + path: '/context-collision' + fullPath: '/context-collision' + preLoaderRoute: typeof ContextCollisionRouteImport + parentRoute: typeof rootRouteImport + } '/': { id: '/' path: '/' @@ -113,6 +137,7 @@ declare module '@tanstack/react-router' { const rootRouteChildren: RootRouteChildren = { IndexRoute: IndexRoute, + ContextCollisionRoute: ContextCollisionRoute, MultipleServerFunctionsRoute: MultipleServerFunctionsRoute, PathnameMiddlewareRoute: PathnameMiddlewareRoute, SimpleRoute: SimpleRoute, diff --git a/e2e/react-start/server-functions-global-middleware/src/routes/context-collision.tsx b/e2e/react-start/server-functions-global-middleware/src/routes/context-collision.tsx new file mode 100644 index 00000000000..e744e15d8a0 --- /dev/null +++ b/e2e/react-start/server-functions-global-middleware/src/routes/context-collision.tsx @@ -0,0 +1,155 @@ +import { createFileRoute } from '@tanstack/react-router' +import { createMiddleware, createServerFn } from '@tanstack/react-start' +import { useState } from 'react' + +const collisionMiddleware = createMiddleware({ type: 'function' }).client( + async ({ next }) => { + return next({ + sendContext: { + trustedUser: 'client-user', + clientNonce: 'client-nonce', + }, + }) + }, +) + +const getCollisionContext = createServerFn() + .middleware([collisionMiddleware]) + .handler(({ context }) => { + return { + trustedUser: context.trustedUser, + clientNonce: context.clientNonce, + } + }) + +const postCollisionContext = createServerFn({ method: 'POST' }) + .middleware([collisionMiddleware]) + .inputValidator((data: unknown) => { + if (!(data instanceof FormData)) { + throw new Error('Expected FormData') + } + + return { + attempt: String(data.get('attempt') ?? ''), + } + }) + .handler(({ context, data }) => { + return { + trustedUser: context.trustedUser, + clientNonce: context.clientNonce, + attempt: data.attempt, + } + }) + +type CollisionResult = { + trustedUser: string | undefined + clientNonce: string | undefined +} + +export const Route = createFileRoute('/context-collision')({ + loader: async () => { + return { + loaderResult: await getCollisionContext(), + } + }, + component: ContextCollisionComponent, +}) + +function ContextCollisionResult({ + label, + result, + testIdPrefix, +}: { + label: string + result: CollisionResult | null + testIdPrefix: string +}) { + const status = + result && + result.trustedUser === 'server-user' && + result.clientNonce === 'client-nonce' + ? 'PASS' + : 'FAIL' + + return ( +
+

{label}

+
+ trustedUser:{' '} + + {result?.trustedUser ?? 'pending'} + +
+
+ clientNonce:{' '} + + {result?.clientNonce ?? 'pending'} + +
+
{status}
+
+ ) +} + +function ContextCollisionComponent() { + const { loaderResult } = Route.useLoaderData() + const [getResult, setGetResult] = useState(null) + const [postResult, setPostResult] = useState(null) + + return ( +
+

+ Client Context Collision Regression +

+ +

+ Trusted request middleware context should win over colliding client + sendContext keys while preserving non-colliding client context. +

+ + + +
+ + + +
+ + + + +
+ ) +} diff --git a/e2e/react-start/server-functions-global-middleware/src/start.ts b/e2e/react-start/server-functions-global-middleware/src/start.ts index 7d22827605a..a6b89d7f7d1 100644 --- a/e2e/react-start/server-functions-global-middleware/src/start.ts +++ b/e2e/react-start/server-functions-global-middleware/src/start.ts @@ -61,6 +61,16 @@ export const pathnameMiddleware = createMiddleware().server( }, ) +export const trustedUserMiddleware = createMiddleware().server( + async ({ next }) => { + return next({ + context: { + trustedUser: 'server-user', + }, + }) + }, +) + // Global function middleware that should be deduped across server functions export const globalFunctionMiddleware = createMiddleware({ type: 'function', @@ -91,5 +101,9 @@ export const startInstance = createStart(() => ({ functionMiddleware: [globalFunctionMiddleware, globalFunctionMiddleware2], // Request middleware - includes loggingMiddleware (issue #5239 scenario) // AND the same loggingMiddleware is also attached to server functions - requestMiddleware: [loggingMiddleware, pathnameMiddleware], + requestMiddleware: [ + trustedUserMiddleware, + loggingMiddleware, + pathnameMiddleware, + ], })) diff --git a/e2e/react-start/server-functions-global-middleware/tests/global-middleware.spec.ts b/e2e/react-start/server-functions-global-middleware/tests/global-middleware.spec.ts index 211d7f59c79..3b1eac10813 100644 --- a/e2e/react-start/server-functions-global-middleware/tests/global-middleware.spec.ts +++ b/e2e/react-start/server-functions-global-middleware/tests/global-middleware.spec.ts @@ -153,3 +153,36 @@ test.describe('Request middleware pathname (issue #6647)', () => { expect(pathnameText!.startsWith('/_serverFn/')).toBe(true) }) }) + +test.describe('Client sendContext collisions (TSR-003)', () => { + test('trusted server middleware context wins for SSR, GET, and FormData calls', async ({ + page, + }) => { + await page.goto('/context-collision') + await page.waitForLoadState('networkidle') + + await expect(page.getByTestId('loader-status')).toHaveText('PASS') + await expect(page.getByTestId('loader-trusted-user')).toHaveText( + 'server-user', + ) + await expect(page.getByTestId('loader-client-nonce')).toHaveText( + 'client-nonce', + ) + + await page.getByTestId('invoke-get-collision').click() + await expect(page.getByTestId('get-status')).toHaveText('PASS') + await expect(page.getByTestId('get-trusted-user')).toHaveText('server-user') + await expect(page.getByTestId('get-client-nonce')).toHaveText( + 'client-nonce', + ) + + await page.getByTestId('invoke-post-collision').click() + await expect(page.getByTestId('post-status')).toHaveText('PASS') + await expect(page.getByTestId('post-trusted-user')).toHaveText( + 'server-user', + ) + await expect(page.getByTestId('post-client-nonce')).toHaveText( + 'client-nonce', + ) + }) +}) diff --git a/packages/start-client-core/src/createServerFn.ts b/packages/start-client-core/src/createServerFn.ts index 42eef35aec2..a4d0c71b829 100644 --- a/packages/start-client-core/src/createServerFn.ts +++ b/packages/start-client-core/src/createServerFn.ts @@ -152,7 +152,6 @@ export const createServerFn: CreateServerFn = (options, __opts) => { const startContext = getStartContextServerOnly() const serverContextAfterGlobalMiddlewares = startContext.contextAfterGlobalMiddlewares - // Use safeObjectMerge for opts.context which comes from client const ctx = { ...extractedFn, ...opts, @@ -160,10 +159,10 @@ export const createServerFn: CreateServerFn = (options, __opts) => { // (which has id, name, filename) rather than the partial one from SSR/client // callers (which only has id) serverFnMeta: extractedFn.serverFnMeta, - // Use safeObjectMerge for opts.context which comes from client + // Merge client context first so trusted server middleware context wins. context: safeObjectMerge( - serverContextAfterGlobalMiddlewares, opts.context, + serverContextAfterGlobalMiddlewares, ), request: startContext.request, } diff --git a/packages/start-server-core/src/server-functions-handler.ts b/packages/start-server-core/src/server-functions-handler.ts index 7b3b34585be..ff047ec0397 100644 --- a/packages/start-server-core/src/server-functions-handler.ts +++ b/packages/start-server-core/src/server-functions-handler.ts @@ -117,8 +117,8 @@ export const handleServerAction = async ({ deserializedContext ) { params.context = safeObjectMerge( - context, deserializedContext as Record, + context, ) } } catch (e) { @@ -144,7 +144,7 @@ export const handleServerAction = async ({ const payload: any = payloadParam ? parsePayload(JSON.parse(payloadParam)) : {} - payload.context = safeObjectMerge(context, payload.context) + payload.context = safeObjectMerge(payload.context, context) payload.method = methodUpper // Send it through! return await action(payload)