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
141 changes: 61 additions & 80 deletions packages/start-client-core/src/client-rpc/serverFnFetcher.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
import {
encode,
isNotFound,
isPlainObject,
parseRedirect,
} from '@tanstack/router-core'
import { encode, isNotFound, parseRedirect } from '@tanstack/router-core'
import { fromCrossJSON, toJSONAsync } from 'seroval'
import invariant from 'tiny-invariant'
import { getDefaultSerovalPlugins } from '../getDefaultSerovalPlugins'
Expand All @@ -17,6 +12,20 @@ import type { Plugin as SerovalPlugin } from 'seroval'

let serovalPlugins: Array<SerovalPlugin<any, any>> | null = null

/**
* Checks if an object has at least one own enumerable property.
* More efficient than Object.keys(obj).length > 0 as it short-circuits on first property.
*/
const hop = Object.prototype.hasOwnProperty
function hasOwnProperties(obj: object): boolean {
for (const _ in obj) {
if (hop.call(obj, _)) {
return true
}
}
return false
}

export async function serverFnFetcher(
url: string,
args: Array<any>,
Expand All @@ -27,80 +36,52 @@ export async function serverFnFetcher(
}
const _first = args[0]

// If createServerFn was used to wrap the fetcher,
// We need to handle the arguments differently
if (isPlainObject(_first) && _first.method) {
const first = _first as FunctionMiddlewareClientFnOptions<any, any, any> & {
headers: HeadersInit
}
const type = first.data instanceof FormData ? 'formData' : 'payload'

// Arrange the headers
const headers = new Headers({
'x-tsr-redirect': 'manual',
...(first.headers instanceof Headers
? Object.fromEntries(first.headers.entries())
: first.headers),
})
const first = _first as FunctionMiddlewareClientFnOptions<any, any, any> & {
headers?: HeadersInit
}
const type = first.data instanceof FormData ? 'formData' : 'payload'

if (type === 'payload') {
headers.set('accept', 'application/x-ndjson, application/json')
}
// Arrange the headers
const headers = first.headers ? new Headers(first.headers) : new Headers()
headers.set('x-tsr-redirect', 'manual')

// If the method is GET, we need to move the payload to the query string
if (first.method === 'GET') {
if (type === 'formData') {
throw new Error('FormData is not supported with GET requests')
}
const serializedPayload = await serializePayload(first)
if (serializedPayload !== undefined) {
const encodedPayload = encode({
payload: await serializePayload(first),
})
if (url.includes('?')) {
url += `&${encodedPayload}`
} else {
url += `?${encodedPayload}`
}
}
}
if (type === 'payload') {
headers.set('accept', 'application/x-ndjson, application/json')
}

if (url.includes('?')) {
url += `&createServerFn`
} else {
url += `?createServerFn`
// If the method is GET, we need to move the payload to the query string
if (first.method === 'GET') {
if (type === 'formData') {
throw new Error('FormData is not supported with GET requests')
}

let body = undefined
if (first.method === 'POST') {
const fetchBody = await getFetchBody(first)
if (fetchBody?.contentType) {
headers.set('content-type', fetchBody.contentType)
const serializedPayload = await serializePayload(first)
if (serializedPayload !== undefined) {
const encodedPayload = encode({
payload: serializedPayload,
})
if (url.includes('?')) {
url += `&${encodedPayload}`
} else {
url += `?${encodedPayload}`
}
body = fetchBody?.body
}
}

return await getResponse(async () =>
handler(url, {
method: first.method,
headers,
signal: first.signal,
body,
}),
)
let body = undefined
if (first.method === 'POST') {
const fetchBody = await getFetchBody(first)
if (fetchBody?.contentType) {
headers.set('content-type', fetchBody.contentType)
}
body = fetchBody?.body
}

// If not a custom fetcher, it was probably
// a `use server` function, so just proxy the arguments
// through as a POST request
return await getResponse(() =>
return await getResponse(async () =>
handler(url, {
method: 'POST',
headers: {
Accept: 'application/json',
'Content-Type': 'application/json',
},
body: JSON.stringify(args),
method: first.method,
headers,
signal: first.signal,
body,
}),
)
}
Expand All @@ -116,7 +97,7 @@ async function serializePayload(
}

// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
if (opts.context && Object.keys(opts.context).length > 0) {
if (opts.context && hasOwnProperties(opts.context)) {
payloadAvailable = true
payloadToSerialize['context'] = opts.context
}
Expand All @@ -139,7 +120,7 @@ async function getFetchBody(
if (opts.data instanceof FormData) {
let serializedContext = undefined
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
if (opts.context && Object.keys(opts.context).length > 0) {
if (opts.context && hasOwnProperties(opts.context)) {
serializedContext = await serialize(opts.context)
}
if (serializedContext !== undefined) {
Expand All @@ -163,17 +144,17 @@ async function getFetchBody(
* @throws If the response is invalid or an error occurs during processing.
*/
async function getResponse(fn: () => Promise<Response>) {
const response = await (async () => {
try {
return await fn()
} catch (error) {
if (error instanceof Response) {
return error
}
let response: Response
try {
response = await fn()
} catch (error) {
if (error instanceof Response) {
response = error
} else {
console.log(error)
throw error
}
})()
}

if (response.headers.get(X_TSS_RAW_RESPONSE) === 'true') {
return response
Expand Down
95 changes: 28 additions & 67 deletions packages/start-server-core/src/server-functions-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,19 @@ import {
import { fromJSON, toCrossJSONAsync, toCrossJSONStream } from 'seroval'
import { getResponse } from './request-response'
import { getServerFnById } from './getServerFnById'
import type { Plugin as SerovalPlugin } from 'seroval'

let regex: RegExp | undefined = undefined

// Cache serovalPlugins at module level to avoid repeated calls
let serovalPlugins: Array<SerovalPlugin<any, any>> | undefined = undefined

// Known FormData 'Content-Type' header values - module-level constant
const FORM_DATA_CONTENT_TYPES = [
'multipart/form-data',
'application/x-www-form-urlencoded',
]
Comment on lines +12 to +23
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find the implementation of getDefaultSerovalPlugins
rg -n 'getDefaultSerovalPlugins' -t ts -t tsx -A 15

# Also check where seroval is imported from
rg -n 'from.*seroval|import.*seroval' -t ts -t tsx

# Check the file structure and surrounding code
cat -n packages/start-server-core/src/server-functions-handler.ts | head -50

Repository: TanStack/router

Length of output: 2064


🏁 Script executed:

#!/bin/bash
# Find where serovalPlugins is initialized/assigned
rg -n 'serovalPlugins\s*=' packages/start-server-core/src/server-functions-handler.ts -A 3 -B 1

# Look for more context in the file, particularly around lines 57-60 mentioned in review
sed -n '50,75p' packages/start-server-core/src/server-functions-handler.ts

# Search for getDefaultSerovalPlugins in the client-core package
find packages/start-client-core -name '*.ts' -o -name '*.tsx' | head -20

# Look for any documentation or type definitions for SerovalPlugin
rg -n 'SerovalPlugin' packages/start-client-core -t ts -A 2 -B 1 | head -40

Repository: TanStack/router

Length of output: 4979


🏁 Script executed:

#!/bin/bash
# Check the actual seroval Plugin type definition
rg -n 'Plugin' node_modules/seroval/package.json 2>/dev/null || echo "Checking alternative"

# Look for the actual type parameters being used in the parsePayload function
sed -n '65,75p' packages/start-server-core/src/server-functions-handler.ts

# Check if there are any other type safety issues in the file
rg -n ': any' packages/start-server-core/src/server-functions-handler.ts

# Look at how parsePayload uses the plugins
sed -n '65,95p' packages/start-server-core/src/server-functions-handler.ts

Repository: TanStack/router

Length of output: 1841


Module-level caching is correct, but TypeScript strict mode compliance is violated throughout the file.

The lazy initialization and caching of serovalPlugins is a safe optimization—the plugins are stateless serialization adapters created from configuration. However, the file extensively violates the coding guideline requiring "TypeScript strict mode with extensive type safety":

  • Line 17: SerovalPlugin<any, any> uses any type parameters
  • Lines 30, 64, 114, 148, 154, 156, 229, 281: Multiple function parameters and variables typed as any

Replace all any type annotations with proper types. For serovalPlugins, use explicit generic parameters from the seroval library. For function parameters like context, payload, and error, provide concrete types instead of any.

🤖 Prompt for AI Agents
In packages/start-server-core/src/server-functions-handler.ts (lines 12–23 and
referenced lines 30, 64, 114, 148, 154, 156, 229, 281), replace all uses of the
any type with concrete types: change serovalPlugins from
Array<SerovalPlugin<any, any>> to an explicit generic such as
Array<SerovalPlugin<unknown, unknown>> or the exact serialization input/output
types from the seroval lib (import and use those actual generics), and update
all function parameters and variables currently typed as any (context, payload,
error, etc.) to the precise project types (e.g., ServerContext,
FunctionInvokePayload, InvokeError or equivalent types exported by your
server/types module) — if those types don't yet exist, define narrow interfaces
for the shape of context, payload and error and import them into this file;
ensure all replacements remove any and use strict nullability/readonly
annotations as appropriate so the file compiles under TypeScript strict mode.


export const handleServerAction = async ({
request,
context,
Expand All @@ -29,34 +39,27 @@ export const handleServerAction = async ({
}

const method = request.method
const methodLower = method.toLowerCase()
const url = new URL(request.url, 'http://localhost:3000')
// extract the serverFnId from the url as host/_serverFn/:serverFnId
// Define a regex to match the path and extract the :thing part

// Execute the regex
const match = url.pathname.match(regex)
const serverFnId = match ? match[1] : null
const search = Object.fromEntries(url.searchParams.entries()) as {
payload?: any
createServerFn?: boolean
}

const isCreateServerFn = 'createServerFn' in search

if (typeof serverFnId !== 'string') {
throw new Error('Invalid server action param for serverFnId: ' + serverFnId)
}

const action = await getServerFnById(serverFnId, { fromClient: true })

// Known FormData 'Content-Type' header values
const formDataContentTypes = [
'multipart/form-data',
'application/x-www-form-urlencoded',
]
// Initialize serovalPlugins lazily (cached at module level)
if (!serovalPlugins) {
serovalPlugins = getDefaultSerovalPlugins()
}

const contentType = request.headers.get('Content-Type')
const serovalPlugins = getDefaultSerovalPlugins()

function parsePayload(payload: any) {
const parsedPayload = fromJSON(payload, { plugins: serovalPlugins })
Expand All @@ -65,16 +68,16 @@ export const handleServerAction = async ({

const response = await (async () => {
try {
let result = await (async () => {
const result = await (async () => {
// FormData
if (
formDataContentTypes.some(
FORM_DATA_CONTENT_TYPES.some(
(type) => contentType && contentType.includes(type),
)
) {
// We don't support GET requests with FormData payloads... that seems impossible
invariant(
method.toLowerCase() !== 'get',
methodLower !== 'get',
'GET requests with FormData payloads are not supported',
)
const formData = await request.formData()
Expand Down Expand Up @@ -104,21 +107,19 @@ export const handleServerAction = async ({
}

// Get requests use the query string
if (method.toLowerCase() === 'get') {
invariant(
isCreateServerFn,
'expected GET request to originate from createServerFn',
)
// By default the payload is the search params
let payload: any = search.payload
if (methodLower === 'get') {
// Get payload directly from searchParams
const payloadParam = url.searchParams.get('payload')
// If there's a payload, we should try to parse it
payload = payload ? parsePayload(JSON.parse(payload)) : {}
const payload: any = payloadParam
? parsePayload(JSON.parse(payloadParam))
: {}
payload.context = { ...context, ...payload.context }
// Send it through!
return await action(payload, signal)
}
Comment on lines +110 to 120
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Add error handling for JSON.parse to prevent unhandled exceptions.

Line 115 calls JSON.parse(payloadParam) without error handling. If the payload query parameter contains malformed JSON, this will throw an uncaught exception during request processing.

🔎 Proposed fix with error handling
 if (methodLower === 'get') {
   // Get payload directly from searchParams
   const payloadParam = url.searchParams.get('payload')
   // If there's a payload, we should try to parse it
-  const payload: any = payloadParam
-    ? parsePayload(JSON.parse(payloadParam))
-    : {}
+  let payload: any = {}
+  if (payloadParam) {
+    try {
+      payload = parsePayload(JSON.parse(payloadParam))
+    } catch (error) {
+      throw new Error('Invalid JSON payload in query parameter', { cause: error })
+    }
+  }
   payload.context = { ...context, ...payload.context }
   // Send it through!
   return await action(payload, signal)
 }
🤖 Prompt for AI Agents
In packages/start-server-core/src/server-functions-handler.ts around lines 110
to 120, the code calls JSON.parse(payloadParam) directly which can throw on
malformed JSON; wrap the JSON.parse call in a try/catch, and on parse failure
handle it gracefully (e.g., log the error and return a controlled client
error/response or throw a specific 400 error with a descriptive message like
"Invalid payload JSON"), only call parsePayload and merge context when parsing
succeeds, and ensure the original flow returns the action(payload, signal) on
success.


if (method.toLowerCase() !== 'post') {
if (methodLower !== 'post') {
throw new Error('expected POST method')
}

Expand All @@ -127,18 +128,9 @@ export const handleServerAction = async ({
jsonPayload = await request.json()
}

// If this POST request was created by createServerFn,
// its payload will be the only argument
if (isCreateServerFn) {
const payload = jsonPayload ? parsePayload(jsonPayload) : {}
payload.context = { ...payload.context, ...context }
return await action(payload, signal)
}

// Otherwise, we'll spread the payload. Need to
// support `use server` functions that take multiple
// arguments.
return await action(...jsonPayload)
const payload = jsonPayload ? parsePayload(jsonPayload) : {}
payload.context = { ...payload.context, ...context }
return await action(payload, signal)
})()

// Any time we get a Response back, we should just
Expand All @@ -148,37 +140,6 @@ export const handleServerAction = async ({
return result.result
}

// If this is a non createServerFn request, we need to
// pull out the result from the result object
if (!isCreateServerFn) {
result = result.result

// The result might again be a response,
// and if it is, return it.
if (result instanceof Response) {
return result
}
}

// TODO: RSCs Where are we getting this package?
// if (isValidElement(result)) {
// const { renderToPipeableStream } = await import(
// // @ts-expect-error
// 'react-server-dom/server'
// )

// const pipeableStream = renderToPipeableStream(result)

// setHeaders(event, {
// 'Content-Type': 'text/x-component',
// } as any)

// sendStream(event, response)
// event._handled = true

// return new Response(null, { status: 200 })
// }

if (isNotFound(result)) {
return isNotFoundResponse(result)
}
Expand Down
Loading