From ec00287b15853a85f9af7e4692f99251e84f5d70 Mon Sep 17 00:00:00 2001 From: Markus Blomqvist Date: Wed, 27 Mar 2024 01:04:10 +0200 Subject: [PATCH] Validation & response serialization improvements (#152) * Fix request body & path param parsing This fixes a bug where the Zod-parsed request body accessed via `req.json()` had potentially incorrect types due to JSON serialization. Now the `json()` method is overridden so that the parsed data is returned as is without the JSON serialization. Additionally, the request path parameters are now also validated when using app router when previously they were used only for providing types for the used path parameters. * Add 10s timeout for form parsing * Improve RPC route response serialization This change serializes Blob data for API routes and creates a JSON object from form responses, although these are rare cases. --- .../v2/route-with-path-params/[slug]/route.ts | 30 +++++++++ .../api/v2/route-with-query-params/route.ts | 8 +-- .../v1/route-with-path-params/[slug]/index.ts | 26 +++++++ .../api/v1/route-with-query-params/index.ts | 4 +- .../src/pages/api/v1/rpc/[operationId].ts | 8 +++ docs/docs/api-reference.md | 2 +- packages/next-rest-framework/README.md | 2 +- .../src/app-router/route.ts | 67 +++++++++++++++---- .../src/app-router/rpc-route.ts | 20 +++--- packages/next-rest-framework/src/constants.ts | 3 +- .../src/pages-router/rpc-api-route.ts | 32 ++++++++- .../src/shared/form-data.ts | 4 ++ .../next-rest-framework/src/shared/utils.ts | 14 ++++ .../tests/app-router/route.test.ts | 49 ++++++++++++-- .../tests/pages-router/api-route.test.ts | 49 ++++++++++++-- 15 files changed, 266 insertions(+), 52 deletions(-) create mode 100644 apps/example/src/app/api/v2/route-with-path-params/[slug]/route.ts create mode 100644 apps/example/src/pages/api/v1/route-with-path-params/[slug]/index.ts diff --git a/apps/example/src/app/api/v2/route-with-path-params/[slug]/route.ts b/apps/example/src/app/api/v2/route-with-path-params/[slug]/route.ts new file mode 100644 index 0000000..faa748d --- /dev/null +++ b/apps/example/src/app/api/v2/route-with-path-params/[slug]/route.ts @@ -0,0 +1,30 @@ +import { TypedNextResponse, route, routeOperation } from 'next-rest-framework'; +import { z } from 'zod'; + +const paramsSchema = z.object({ + slug: z.enum(['foo', 'bar', 'baz']) +}); + +export const runtime = 'edge'; + +export const { GET } = route({ + getPathParams: routeOperation({ + method: 'GET' + }) + .input({ + contentType: 'application/json', + params: paramsSchema + }) + .outputs([ + { + status: 200, + contentType: 'application/json', + body: paramsSchema + } + ]) + .handler((_req, { params: { slug } }) => { + return TypedNextResponse.json({ + slug + }); + }) +}); diff --git a/apps/example/src/app/api/v2/route-with-query-params/route.ts b/apps/example/src/app/api/v2/route-with-query-params/route.ts index f3248d5..084d326 100644 --- a/apps/example/src/app/api/v2/route-with-query-params/route.ts +++ b/apps/example/src/app/api/v2/route-with-query-params/route.ts @@ -2,9 +2,7 @@ import { TypedNextResponse, route, routeOperation } from 'next-rest-framework'; import { z } from 'zod'; const querySchema = z.object({ - foo: z.string().uuid(), - bar: z.string().optional(), - baz: z.string() + total: z.string() }); export const runtime = 'edge'; @@ -28,9 +26,7 @@ export const { GET } = route({ const query = req.nextUrl.searchParams; return TypedNextResponse.json({ - foo: query.get('foo') ?? '', - bar: query.get('bar') ?? '', - baz: query.get('baz') ?? '' + total: query.get('total') ?? '' }); }) }); diff --git a/apps/example/src/pages/api/v1/route-with-path-params/[slug]/index.ts b/apps/example/src/pages/api/v1/route-with-path-params/[slug]/index.ts new file mode 100644 index 0000000..72a483e --- /dev/null +++ b/apps/example/src/pages/api/v1/route-with-path-params/[slug]/index.ts @@ -0,0 +1,26 @@ +import { apiRoute, apiRouteOperation } from 'next-rest-framework'; +import { z } from 'zod'; + +const paramsSchema = z.object({ + slug: z.enum(['foo', 'bar', 'baz']) +}); + +export default apiRoute({ + getQueryParams: apiRouteOperation({ + method: 'GET' + }) + .input({ + contentType: 'application/json', + query: paramsSchema + }) + .outputs([ + { + status: 200, + contentType: 'application/json', + body: paramsSchema + } + ]) + .handler((req, res) => { + res.json(req.query); + }) +}); diff --git a/apps/example/src/pages/api/v1/route-with-query-params/index.ts b/apps/example/src/pages/api/v1/route-with-query-params/index.ts index f122c4d..ed278e5 100644 --- a/apps/example/src/pages/api/v1/route-with-query-params/index.ts +++ b/apps/example/src/pages/api/v1/route-with-query-params/index.ts @@ -2,9 +2,7 @@ import { apiRoute, apiRouteOperation } from 'next-rest-framework'; import { z } from 'zod'; const querySchema = z.object({ - foo: z.string().uuid(), - bar: z.string().optional(), - baz: z.string() + total: z.string() }); export default apiRoute({ diff --git a/apps/example/src/pages/api/v1/rpc/[operationId].ts b/apps/example/src/pages/api/v1/rpc/[operationId].ts index 43078ae..006df1d 100644 --- a/apps/example/src/pages/api/v1/rpc/[operationId].ts +++ b/apps/example/src/pages/api/v1/rpc/[operationId].ts @@ -8,6 +8,14 @@ import { } from '@/actions'; import { rpcApiRoute } from 'next-rest-framework'; +// Body parser must be disabled when parsing multipart/form-data requests with pages router. +// A recommended way is to create a separate RPC API route for multipart/form-data requests. +// export const config = { +// api: { +// bodyParser: false +// } +// }; + const handler = rpcApiRoute({ getTodos, getTodoById, diff --git a/docs/docs/api-reference.md b/docs/docs/api-reference.md index 4e0fbcf..15eb742 100644 --- a/docs/docs/api-reference.md +++ b/docs/docs/api-reference.md @@ -57,7 +57,7 @@ The route operation input function is used for type-checking, validation and doc | `bodySchema` | A JSON schema that you can provide in case the conversion of the `body` Zod schema fails or produces an incorrect result in your OpenAPI spec. | `false` | | `query` | A [Zod](https://github.com/colinhacks/zod) schema describing the format of the query parameters. When the query schema is defined, a request with invalid query parameters will get an error response. Query parameters are parsed using this schema and updated to the request if valid, so the query parameters from the request should always match the schema. | `false` | | `querySchema` | A JSON schema that you can provide in case the conversion of the `query` Zod schema fails or produces an incorrect result in your OpenAPI spec. | `false` | -| `params` | A [Zod](https://github.com/colinhacks/zod) schema describing the format of the path parameters for strong typing when using them in your route handler. | `false` | +| `params` | A [Zod](https://github.com/colinhacks/zod) schema describing the format of the path parameters. When the params schema is defined, a request with invalid path parameters will get an error response. Path parameters are parsed using this schema and updated to the request if valid, so the path parameters from the request should always match the schema. | `false` | Calling the route operation input function allows you to chain your API handler logic with the [Route operation outputs](#route-operation-outputs), [Route operation middleware](#route-operation-middleware) and [Route operation handler](#route-operation-handler) functions. diff --git a/packages/next-rest-framework/README.md b/packages/next-rest-framework/README.md index 0205223..5c0d730 100644 --- a/packages/next-rest-framework/README.md +++ b/packages/next-rest-framework/README.md @@ -736,7 +736,7 @@ The route operation input function is used for type-checking, validation and doc | `bodySchema` | A JSON schema that you can provide in case the conversion of the `body` Zod schema fails or produces an incorrect result in your OpenAPI spec. | `false` | | `query` | A [Zod](https://github.com/colinhacks/zod) schema describing the format of the query parameters. When the query schema is defined, a request with invalid query parameters will get an error response. Query parameters are parsed using this schema and updated to the request if valid, so the query parameters from the request should always match the schema. | `false` | | `querySchema` | A JSON schema that you can provide in case the conversion of the `query` Zod schema fails or produces an incorrect result in your OpenAPI spec. | `false` | -| `params` | A [Zod](https://github.com/colinhacks/zod) schema describing the format of the path parameters for strong typing when using them in your route handler. | `false` | +| `params` | A [Zod](https://github.com/colinhacks/zod) schema describing the format of the path parameters. When the params schema is defined, a request with invalid path parameters will get an error response. Path parameters are parsed using this schema and updated to the request if valid, so the path parameters from the request should always match the schema. | `false` | Calling the route operation input function allows you to chain your API handler logic with the [Route operation outputs](#route-operation-outputs), [Route operation middleware](#route-operation-middleware) and [Route operation handler](#route-operation-handler) functions. diff --git a/packages/next-rest-framework/src/app-router/route.ts b/packages/next-rest-framework/src/app-router/route.ts index fc9bd9a..77339fc 100644 --- a/packages/next-rest-framework/src/app-router/route.ts +++ b/packages/next-rest-framework/src/app-router/route.ts @@ -26,10 +26,13 @@ export const route = >( openApiPath?: OpenApiPathItem; } ) => { - const handler = async (req: NextRequest, context: { params: BaseParams }) => { + const handler = async ( + _req: NextRequest, + context: { params: BaseParams } + ) => { try { const operation = Object.entries(operations).find( - ([_operationId, operation]) => operation.method === req.method + ([_operationId, operation]) => operation.method === _req.method )?.[1]; if (!operation) { @@ -49,7 +52,15 @@ export const route = >( const { input, handler, middleware1, middleware2, middleware3 } = operation; - let reqClone = req.clone() as NextRequest; + const _reqClone = _req.clone() as NextRequest; + + let reqClone = new NextRequest(_reqClone.url, { + method: _reqClone.method, + headers: _reqClone.headers + }); + + reqClone.json = async () => await _req.clone().json(); + reqClone.formData = async () => await _req.clone().formData(); let middlewareOptions: BaseOptions = {}; @@ -94,10 +105,11 @@ export const route = >( const { body: bodySchema, query: querySchema, - contentType: contentTypeSchema + contentType: contentTypeSchema, + params: paramsSchema } = input; - const contentType = req.headers.get('content-type')?.split(';')[0]; + const contentType = reqClone.headers.get('content-type')?.split(';')[0]; if (contentTypeSchema && contentType !== contentTypeSchema) { return NextResponse.json( @@ -109,7 +121,7 @@ export const route = >( if (bodySchema) { if (contentType === 'application/json') { try { - const json = await req.clone().json(); + const json = await reqClone.json(); const { valid, errors, data } = validateSchema({ schema: bodySchema, @@ -129,11 +141,12 @@ export const route = >( } reqClone = new NextRequest(reqClone.url, { - ...reqClone, method: reqClone.method, headers: reqClone.headers, body: JSON.stringify(data) }); + + reqClone.json = async () => data; } catch { return NextResponse.json( { @@ -150,7 +163,7 @@ export const route = >( FORM_DATA_CONTENT_TYPES.includes(contentType as FormDataContentType) ) { try { - const formData = await req.clone().formData(); + const formData = await reqClone.formData(); const { valid, errors, data } = validateSchema({ schema: bodySchema, @@ -171,7 +184,6 @@ export const route = >( // Inject parsed for data to JSON body. reqClone = new NextRequest(reqClone.url, { - ...reqClone, method: reqClone.method, headers: reqClone.headers, body: JSON.stringify(data) @@ -203,7 +215,9 @@ export const route = >( if (querySchema) { const { valid, errors, data } = validateSchema({ schema: querySchema, - obj: qs.parse(req.nextUrl.search, { ignoreQueryPrefix: true }) + obj: qs.parse(reqClone.nextUrl.search, { + ignoreQueryPrefix: true + }) }); if (!valid) { @@ -230,9 +244,38 @@ export const route = >( }); reqClone = new NextRequest(url, { - ...reqClone, method: reqClone.method, - headers: reqClone.headers + headers: reqClone.headers, + body: reqClone.body + }); + } + + if (paramsSchema) { + const { valid, errors, data } = validateSchema({ + schema: paramsSchema, + obj: context.params + }); + + if (!valid) { + return NextResponse.json( + { + message: DEFAULT_ERRORS.invalidPathParameters, + errors + }, + { + status: 400 + } + ); + } + + context.params = data; + const url = new URL(reqClone.url); + url.search = new URLSearchParams(context.params).toString(); + + reqClone = new NextRequest(url, { + method: reqClone.method, + headers: reqClone.headers, + body: reqClone.body }); } } diff --git a/packages/next-rest-framework/src/app-router/rpc-route.ts b/packages/next-rest-framework/src/app-router/rpc-route.ts index 92ab62d..1bdf871 100644 --- a/packages/next-rest-framework/src/app-router/rpc-route.ts +++ b/packages/next-rest-framework/src/app-router/rpc-route.ts @@ -7,7 +7,8 @@ import { import { validateSchema, logNextRestFrameworkError, - type RpcOperationDefinition + type RpcOperationDefinition, + parseRpcOperationResponseJson } from '../shared'; import { type FormDataContentType, @@ -194,21 +195,18 @@ export const rpcRoute = < ); } - const parseRes = (res: unknown): BodyInit => { - if ( - res instanceof ReadableStream || - res instanceof ArrayBuffer || - res instanceof Blob || - res instanceof FormData || - res instanceof URLSearchParams - ) { + const parseRes = async (res: unknown): Promise => { + if (res instanceof ReadableStream || res instanceof Blob) { return res; } - return JSON.stringify(res); + const parsed = await parseRpcOperationResponseJson(res); + return JSON.stringify(parsed); }; - return new NextResponse(parseRes(res), { + const json = await parseRes(res); + + return new NextResponse(json, { status: 200, headers: { 'Content-Type': 'application/json' diff --git a/packages/next-rest-framework/src/constants.ts b/packages/next-rest-framework/src/constants.ts index 7baf818..cb75f85 100644 --- a/packages/next-rest-framework/src/constants.ts +++ b/packages/next-rest-framework/src/constants.ts @@ -9,7 +9,8 @@ export const DEFAULT_ERRORS = { operationNotAllowed: 'Operation not allowed.', invalidRequestBody: 'Invalid request body.', missingRequestBody: 'Missing request body.', - invalidQueryParameters: 'Invalid query parameters.' + invalidQueryParameters: 'Invalid query parameters.', + invalidPathParameters: 'Invalid path parameters.' }; export enum ValidMethod { diff --git a/packages/next-rest-framework/src/pages-router/rpc-api-route.ts b/packages/next-rest-framework/src/pages-router/rpc-api-route.ts index f64389f..b9e3bf0 100644 --- a/packages/next-rest-framework/src/pages-router/rpc-api-route.ts +++ b/packages/next-rest-framework/src/pages-router/rpc-api-route.ts @@ -7,7 +7,8 @@ import { validateSchema, logNextRestFrameworkError, type RpcOperationDefinition, - logPagesEdgeRuntimeErrorForRoute + logPagesEdgeRuntimeErrorForRoute, + parseRpcOperationResponseJson } from '../shared'; import { type NextApiRequest, type NextApiResponse } from 'next/types'; import { @@ -117,7 +118,7 @@ export const rpcApiRoute = < // Parse multipart/form-data into a FormData object. try { req.body = await parseMultiPartFormData(req); - } catch (e) { + } catch { res.status(400).json({ message: `${DEFAULT_ERRORS.invalidRequestBody} Failed to parse form data.` }); @@ -166,7 +167,32 @@ export const rpcApiRoute = < return; } - res.status(200).json(_res); + if (_res instanceof Blob) { + const reader = _res.stream().getReader(); + res.setHeader('Content-Type', 'application/octet-stream'); + + res.setHeader( + 'Content-Disposition', + `attachment; filename="${_res.name}"` + ); + + const pump = async () => { + await reader.read().then(async ({ done, value }) => { + if (done) { + res.end(); + return; + } + + res.write(value); + await pump(); + }); + }; + + await pump(); + } + + const json = await parseRpcOperationResponseJson(_res); + res.status(200).json(json); } catch (error) { logNextRestFrameworkError(error); res.status(400).json({ message: DEFAULT_ERRORS.unexpectedError }); diff --git a/packages/next-rest-framework/src/shared/form-data.ts b/packages/next-rest-framework/src/shared/form-data.ts index a7682c9..96f4310 100644 --- a/packages/next-rest-framework/src/shared/form-data.ts +++ b/packages/next-rest-framework/src/shared/form-data.ts @@ -6,6 +6,10 @@ export const parseMultiPartFormData = async (req: NextApiRequest) => await new Promise((resolve, reject) => { const form = new Formidable(); + setTimeout(() => { + reject(new Error('Form parsing timeout.')); + }, 10000); + form.parse(req, (err, fields, files) => { if (err) { reject(err); diff --git a/packages/next-rest-framework/src/shared/utils.ts b/packages/next-rest-framework/src/shared/utils.ts index b0fa235..ba24dbc 100644 --- a/packages/next-rest-framework/src/shared/utils.ts +++ b/packages/next-rest-framework/src/shared/utils.ts @@ -5,3 +5,17 @@ export const isValidMethod = (x: unknown): x is ValidMethod => export const capitalizeFirstLetter = (str: string) => str[0]?.toUpperCase() + str.slice(1); + +export const parseRpcOperationResponseJson = async (res: unknown) => { + if (res instanceof FormData || res instanceof URLSearchParams) { + const body: Record = {}; + + for (const [key, value] of res.entries()) { + body[key] = value; + } + + return body; + } + + return res; +}; diff --git a/packages/next-rest-framework/tests/app-router/route.test.ts b/packages/next-rest-framework/tests/app-router/route.test.ts index 41f56ad..0e63e03 100644 --- a/packages/next-rest-framework/tests/app-router/route.test.ts +++ b/packages/next-rest-framework/tests/app-router/route.test.ts @@ -493,13 +493,22 @@ describe('route', () => { .input({ body: z.object({ foo: z.string() }) }) .middleware(async (req) => { const body = await req.json(); - console.log(body); + console.log({ middleware: 1, ...body }); + req.headers.set('x-foo', 'custom-header'); return { bar: 'baz' }; }) + .middleware(async (req, _ctx, options) => { + const body = await req.json(); + console.log({ middleware: 2, ...body }); + req.headers.set('x-bar', 'custom-header-2'); + return { ...options, baz: 'qux' }; + }) .handler(async (req, _ctx, options) => { const body = await req.json(); - console.log(body); - console.log(options); + console.log({ handler: true, ...body }); + console.log({ options: true, ...options }); + console.log({ 'x-foo': req.headers.get('x-foo') }); + console.log({ 'x-bar': req.headers.get('x-bar') }); return NextResponse.json(options); }) }).POST(req, context); @@ -508,12 +517,38 @@ describe('route', () => { expect(res?.status).toEqual(200); expect(json).toEqual({ - bar: 'baz' + bar: 'baz', + baz: 'qux' + }); + + expect(console.log).toHaveBeenNthCalledWith(1, { + middleware: 1, + foo: 'bar' + }); + + expect(console.log).toHaveBeenNthCalledWith(2, { + middleware: 2, + foo: 'bar' }); - expect(console.log).toHaveBeenNthCalledWith(1, { foo: 'bar' }); - expect(console.log).toHaveBeenNthCalledWith(2, { foo: 'bar' }); - expect(console.log).toHaveBeenNthCalledWith(3, { bar: 'baz' }); + expect(console.log).toHaveBeenNthCalledWith(3, { + handler: true, + foo: 'bar' + }); + + expect(console.log).toHaveBeenNthCalledWith(4, { + options: true, + bar: 'baz', + baz: 'qux' + }); + + expect(console.log).toHaveBeenNthCalledWith(5, { + 'x-foo': 'custom-header' + }); + + expect(console.log).toHaveBeenNthCalledWith(6, { + 'x-bar': 'custom-header-2' + }); }); it('allows chaining three middlewares', async () => { diff --git a/packages/next-rest-framework/tests/pages-router/api-route.test.ts b/packages/next-rest-framework/tests/pages-router/api-route.test.ts index d686041..94b2eb6 100644 --- a/packages/next-rest-framework/tests/pages-router/api-route.test.ts +++ b/packages/next-rest-framework/tests/pages-router/api-route.test.ts @@ -476,13 +476,22 @@ describe('apiRoute', () => { .input({ body: z.object({ foo: z.string() }) }) .middleware((req) => { const body = req.body; - console.log(body); + console.log({ middleware: 1, ...body }); + req.headers = { ...req.headers, 'x-foo': 'custom-header' }; return { bar: 'baz' }; }) + .middleware((req, _res, options) => { + const body = req.body; + console.log({ middleware: 2, ...body }); + req.headers = { ...req.headers, 'x-bar': 'custom-header-2' }; + return { ...options, baz: 'qux' }; + }) .handler(async (req, res, options) => { const body = req.body; - console.log(body); - console.log(options); + console.log({ handler: true, ...body }); + console.log({ options: true, ...options }); + console.log({ 'x-foo': req.headers['x-foo'] }); + console.log({ 'x-bar': req.headers['x-bar'] }); res.json(options); }) })(req, res); @@ -490,12 +499,38 @@ describe('apiRoute', () => { expect(res.statusCode).toEqual(200); expect(res._getJSONData()).toEqual({ - bar: 'baz' + bar: 'baz', + baz: 'qux' + }); + + expect(console.log).toHaveBeenNthCalledWith(1, { + middleware: 1, + foo: 'bar' + }); + + expect(console.log).toHaveBeenNthCalledWith(2, { + middleware: 2, + foo: 'bar' }); - expect(console.log).toHaveBeenNthCalledWith(1, { foo: 'bar' }); - expect(console.log).toHaveBeenNthCalledWith(2, { foo: 'bar' }); - expect(console.log).toHaveBeenNthCalledWith(3, { bar: 'baz' }); + expect(console.log).toHaveBeenNthCalledWith(3, { + handler: true, + foo: 'bar' + }); + + expect(console.log).toHaveBeenNthCalledWith(4, { + options: true, + bar: 'baz', + baz: 'qux' + }); + + expect(console.log).toHaveBeenNthCalledWith(5, { + 'x-foo': 'custom-header' + }); + + expect(console.log).toHaveBeenNthCalledWith(6, { + 'x-bar': 'custom-header-2' + }); }); it('allows chaining three middlewares', async () => {