fix: fastify form-data type #6974
Conversation
|
@wszgrcy is attempting to deploy a commit to the trpc Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughIntroduces multipart/form-data passthrough in the Fastify tRPC plugin, adds IncomingMessage body passthrough in the Node HTTP adapter, and updates tests to include a FormData-based procedure with client-side link splitting between httpLink (for FormData) and httpBatchStreamLink. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant Split as splitLink
participant HL as httpLink (multipart)
participant HB as httpBatchStreamLink
participant F as Fastify TRPC Plugin
participant R as tRPC Router
Note over C,Split: Client calls mutation
C->>Split: mutate(input)
alt input is FormData
Split->>HL: forward request (multipart/form-data)
HL->>F: HTTP request with FormData body
Note over F: Multipart parser is no-op (raw passthrough)
F->>R: pass raw body to procedure
R-->>F: result ({ id })
F-->>HL: HTTP response
HL-->>C: result
else non-FormData
Split->>HB: batch/stream request
HB->>F: batched request
F->>R: handle as usual
R-->>F: result
F-->>HB: response
HB-->>C: result
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@trpc/client
@trpc/next
@trpc/react-query
@trpc/server
@trpc/tanstack-react-query
@trpc/upgrade
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/server/src/adapters/fastify/fastifyTRPCPlugin.ts (1)
49-56: Global multipart parser override: verify side effects and add idempotency guardThis replaces the instance-wide multipart/form-data parser with a passthrough stream, which can impact non‑tRPC routes or apps using @fastify/multipart. Confirm ordering and scope are acceptable, or add an idempotency check.
Consider:
- fastify.removeContentTypeParser('multipart/form-data'); + if ('hasContentTypeParser' in fastify && fastify.hasContentTypeParser('multipart/form-data')) { + fastify.removeContentTypeParser('multipart/form-data'); + }Operational note: with a passthrough stream, Fastify’s bodyLimit on parser aggregation won’t apply; ensure size guarding is enforced downstream (e.g., in incomingMessageToRequest).
packages/tests/server/adapters/fastify.test.ts (2)
125-137: Harden FormData validation and dropas anyUse a type‑predicate with Symbol.toStringTag/Object.prototype.toString; then access
input.getwithout casting.Apply this diff:
- multipartForm: publicProcedure - .input( - z.custom<FormData>((input) => { - // input instanceof FormData return false but is a FormData type - // maybe undici version - return input.toString() === '[object FormData]'; - }), - ) - .mutation(async ({ input }) => { - const result = {} as Record<string, any>; - result['id'] = (input as any).get('id'); - return result; - }), + multipartForm: publicProcedure + .input( + z.custom<FormData>((input): input is FormData => { + // Robust cross‑impl detection; avoids undici version mismatches + const tag = (input as any)?.[Symbol.toStringTag]; + return tag === 'FormData' || Object.prototype.toString.call(input) === '[object FormData]'; + }), + ) + .mutation(async ({ input }) => { + return { id: input.get('id') as string }; + }),
246-258: Make FormData detection in splitLink resilient
instanceof FormDatacan fail across undici/globals. Use a tag/name fallback to ensure FormData routes to httpLink.Apply this diff:
- false: splitLink({ - condition(op) { - return op.input instanceof FormData; - }, + false: splitLink({ + condition(op) { + const v = op.input as any; + return ( + (typeof FormData !== 'undefined' && v instanceof FormData) || + v?.[Symbol.toStringTag] === 'FormData' || + Object.prototype.toString.call(v) === '[object FormData]' + ); + }, true: httpLink({ url: `http://${host}`, headers: opts.headers, }), false: httpBatchStreamLink({ url: `http://${host}`, headers: opts.headers, }), }),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/server/src/adapters/fastify/fastifyTRPCPlugin.ts(1 hunks)packages/server/src/adapters/node-http/incomingMessageToRequest.ts(2 hunks)packages/tests/server/adapters/fastify.test.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
**/*.{ts,tsx}: Always useimport typefor type-only imports
Separate type imports from value imports
Avoid overzealous object destructuring; prefer direct property access
Object destructuring is acceptable when a variable is used 3+ times
Prefer array destructuring
Avoid sparse array destructuring
Never destructure in function parameter declarations
Avoid destructuring potentially nullish nested properties
Maximum 3 parameters per function; use options objects when more
Type parameter names must match /^(T|$)(A-Z)?[0-9]*$/
Prefix unused variables, parameters, and caught errors with_
Prefer namespace imports for validation libraries and large modules (e.g.,import * as z from 'zod',import * as React from 'react')
Follow import order: test helpers, tRPC test imports, third-party, then relative
Never import from@trpc/*/src; import from the package root instead
Do not useSymbol.disposeorSymbol.asyncDispose; usemakeResource()/makeAsyncResource()
Always useawait usingfor resource cleanup
PrefermakeResource()/makeAsyncResource()over manual disposal logic
Avoid non-null assertions (!)
Use proper type guards and optional chaining instead of non-null assertions
Switch statements must be exhaustive for union types
Rely on TypeScript inference; avoid unnecessary explicit return/output types
Use explicit types at public API boundaries, for complex generic constraints, or when inference is insufficient/ambiguous
Use thesatisfiesoperator to retain inference while enforcing shapes
Useas constfor literal type inference when appropriate
Prefer explicit typing overany
Use type assertions sparingly
Leverage TypeScript strict mode features
Files:
packages/server/src/adapters/fastify/fastifyTRPCPlugin.tspackages/server/src/adapters/node-http/incomingMessageToRequest.tspackages/tests/server/adapters/fastify.test.ts
**/*.{ts,tsx,md,mdx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
Use camelCase for file names (with exceptions like TRPC/RPC/HTTP/JSON acronyms, .config.js, .d.ts, and tests)
Files:
packages/server/src/adapters/fastify/fastifyTRPCPlugin.tspackages/server/src/adapters/node-http/incomingMessageToRequest.tspackages/tests/server/adapters/fastify.test.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
No
console.login packages; use proper logging instead
Files:
packages/server/src/adapters/fastify/fastifyTRPCPlugin.tspackages/server/src/adapters/node-http/incomingMessageToRequest.tspackages/tests/server/adapters/fastify.test.ts
**/*.test.ts
📄 CodeRabbit inference engine (.cursor/rules/test-patterns.mdc)
**/*.test.ts: ALWAYS useawait using ctx = testServerAndClientResource(...)in tests that need both server and client setup
NEVER use the deprecatedrouterToServerAndClientNew()in tests
ImporttestServerAndClientResourcefrom@trpc/client/__tests__/testClientResource
Create fresh mock instances per test using a factory (e.g.,getMockFetch())
Do not use global mocks that persist across tests
Configure mocks via theclientcallback intestServerAndClientResourceoptions
Usectx.clientfrom the test resource for making tRPC calls
Access server URLs viactx.httpUrlandctx.wssUrl
Configure server options via theserverproperty intestServerAndClientResourceoptions
Configure client options via theclientcallback intestServerAndClientResourceoptions
Use descriptive test names that explain the behavior under test
Focus test names on what the test validates, not just what it does
Use proper TypeScript typing for mocks
Clear mocks between tests when needed
Use factory functions for mock creation to ensure isolation
Files:
packages/tests/server/adapters/fastify.test.ts
{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}}
📄 CodeRabbit inference engine (.cursor/rules/coding-guidelines.mdc)
{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}}: Do not use Testing LibrarywaitFor; usevi.waitForinstead
Tests may use non-null assertions
Tests may have unused variables
Tests may allow floating promises
Files:
packages/tests/server/adapters/fastify.test.ts
🧬 Code graph analysis (1)
packages/tests/server/adapters/fastify.test.ts (3)
packages/client/src/links/splitLink.ts (1)
splitLink(9-30)packages/client/src/links/httpLink.ts (1)
httpLink(75-142)packages/client/src/links/httpBatchStreamLink.ts (1)
httpBatchStreamLink(23-186)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: E2E-tests (next-formdata)
- GitHub Check: E2E-tests (soa)
- GitHub Check: E2E-tests (cloudflare-workers)
- GitHub Check: e2e-legacy-node (next-prisma-websockets-starter, 20.x)
- GitHub Check: E2E-tests (.test/ssg)
- GitHub Check: E2E-tests (express-minimal)
- GitHub Check: E2E-tests (.experimental/next-app-dir)
- GitHub Check: E2E-tests (.test/diagnostics-big-router)
- GitHub Check: e2e-legacy-node (next-prisma-starter, 18.x)
- GitHub Check: e2e-legacy-node (next-prisma-todomvc, 20.x)
- GitHub Check: e2e-legacy-node (next-prisma-todomvc, 18.x)
- GitHub Check: E2E-tests (.test/internal-types-export)
- GitHub Check: e2e-legacy-node (next-prisma-websockets-starter, 18.x)
- GitHub Check: e2e-legacy-node (next-prisma-starter, 20.x)
- GitHub Check: Test a monorepo using built declaration files
- GitHub Check: Release using pkg.pr.new
- GitHub Check: E2E-tests (Deno) (deno-deploy)
- GitHub Check: typecheck-www
- GitHub Check: Lint and auto-fix
- GitHub Check: test
🔇 Additional comments (3)
packages/server/src/adapters/node-http/incomingMessageToRequest.ts (1)
2-2: LGTM on runtime IncomingMessage importEnables a reliable instanceof check for body streams.
packages/tests/server/adapters/fastify.test.ts (2)
10-10: LGTM: httpLink importRequired for routing FormData requests outside batching.
474-483: LGTM: new multipart/form-data e2e testValidates the new path and client link routing.
| // formData use | ||
| if (req.body instanceof IncomingMessage) { | ||
| return req.body as any; | ||
| } |
There was a problem hiding this comment.
Do not bypass maxBodySize for IncomingMessage bodies
Returning the raw Node stream skips the existing size check, weakening request size guarantees and risking DoS. Wrap it in a ReadableStream with the same maxBodySize guard as the fallback path.
Apply this diff:
- // formData use
- if (req.body instanceof IncomingMessage) {
- return req.body as any;
- }
+ // formData/raw stream passthrough with size guard
+ if (req.body instanceof IncomingMessage) {
+ const nodeStream = req.body;
+ let size = 0;
+ let hasClosed = false;
+ return new ReadableStream({
+ start(controller) {
+ const onData = (chunk: Buffer) => {
+ size += chunk.length;
+ if (opts.maxBodySize != null && size > opts.maxBodySize) {
+ hasClosed = true;
+ controller.error(new TRPCError({ code: 'PAYLOAD_TOO_LARGE' }));
+ nodeStream.off('data', onData);
+ nodeStream.off('end', onEnd);
+ nodeStream.off('error', onError);
+ try { nodeStream.destroy(); } catch {}
+ return;
+ }
+ controller.enqueue(
+ new Uint8Array(chunk.buffer, chunk.byteOffset, chunk.byteLength),
+ );
+ };
+ const onEnd = () => {
+ if (hasClosed) return;
+ hasClosed = true;
+ nodeStream.off('data', onData);
+ nodeStream.off('end', onEnd);
+ nodeStream.off('error', onError);
+ controller.close();
+ };
+ const onError = (err: unknown) => {
+ if (hasClosed) return;
+ hasClosed = true;
+ nodeStream.off('data', onData);
+ nodeStream.off('end', onEnd);
+ nodeStream.off('error', onError);
+ controller.error(err);
+ };
+ nodeStream.on('data', onData);
+ nodeStream.on('end', onEnd);
+ nodeStream.on('error', onError);
+ },
+ cancel() {
+ try { nodeStream.destroy(); } catch {}
+ },
+ });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // formData use | |
| if (req.body instanceof IncomingMessage) { | |
| return req.body as any; | |
| } | |
| // formData/raw stream passthrough with size guard | |
| if (req.body instanceof IncomingMessage) { | |
| const nodeStream = req.body; | |
| let size = 0; | |
| let hasClosed = false; | |
| return new ReadableStream({ | |
| start(controller) { | |
| const onData = (chunk: Buffer) => { | |
| size += chunk.length; | |
| if (opts.maxBodySize != null && size > opts.maxBodySize) { | |
| hasClosed = true; | |
| controller.error(new TRPCError({ code: 'PAYLOAD_TOO_LARGE' })); | |
| nodeStream.off('data', onData); | |
| nodeStream.off('end', onEnd); | |
| nodeStream.off('error', onError); | |
| try { nodeStream.destroy(); } catch {} | |
| return; | |
| } | |
| controller.enqueue( | |
| new Uint8Array(chunk.buffer, chunk.byteOffset, chunk.byteLength), | |
| ); | |
| }; | |
| const onEnd = () => { | |
| if (hasClosed) return; | |
| hasClosed = true; | |
| nodeStream.off('data', onData); | |
| nodeStream.off('end', onEnd); | |
| nodeStream.off('error', onError); | |
| controller.close(); | |
| }; | |
| const onError = (err: unknown) => { | |
| if (hasClosed) return; | |
| hasClosed = true; | |
| nodeStream.off('data', onData); | |
| nodeStream.off('end', onEnd); | |
| nodeStream.off('error', onError); | |
| controller.error(err); | |
| }; | |
| nodeStream.on('data', onData); | |
| nodeStream.on('end', onEnd); | |
| nodeStream.on('error', onError); | |
| }, | |
| cancel() { | |
| try { nodeStream.destroy(); } catch {} | |
| }, | |
| }); | |
| } |
🤖 Prompt for AI Agents
In packages/server/src/adapters/node-http/incomingMessageToRequest.ts around
lines 25 to 28, returning the raw IncomingMessage bypasses the maxBodySize
check; instead wrap the IncomingMessage in a ReadableStream that enforces the
same maxBodySize guard used by the fallback path: create a ReadableStream that
reads chunks from the IncomingMessage, track total bytes read, if total exceeds
maxBodySize abort/close the stream and throw the same MaxBodySize error used
elsewhere, propagate and handle stream errors, and ensure the resulting
ReadableStream is returned in place of the raw req.body so all request bodies go
through the same size enforcement.
1 participant
Closes #
🎯 Changes
What changes are made in this PR? Is it a feature or a bug fix?
fastify form-data type
https://trpc.io/docs/server/non-json-content-types#formdata-input
✅ Checklist
I found that due to the version issue of undici, FormData validation may fail So should we update the official version before merging?
Or after updating, submit another patch to fix the test?
Summary by CodeRabbit
New Features
Tests