From c6735f61afa46bc17a9170142d805b22c331a721 Mon Sep 17 00:00:00 2001 From: Dinh Le Date: Sat, 8 Aug 2026 14:45:09 +0700 Subject: [PATCH] fix(nest): keep non-contract params and harden path param handling --- packages/cloudflare/worker-configuration.d.ts | 2 +- packages/nest/src/implement.test.ts | 159 ++++++++++++++++++ packages/nest/src/implement.ts | 42 ++--- .../cloudflare/worker-configuration.d.ts | 2 +- 4 files changed, 183 insertions(+), 22 deletions(-) diff --git a/packages/cloudflare/worker-configuration.d.ts b/packages/cloudflare/worker-configuration.d.ts index af0c3f2af..64fabae79 100644 --- a/packages/cloudflare/worker-configuration.d.ts +++ b/packages/cloudflare/worker-configuration.d.ts @@ -1,6 +1,6 @@ /* eslint-disable */ // Generated by Wrangler by running `wrangler types` (hash: e624d76b8500cfee2091bb6c84ee404f) -// Runtime types generated with workerd@1.20260722.1 2026-07-01 +// Runtime types generated with workerd@1.20260730.1 2026-07-01 interface __BaseEnv_Env { RATELIMIT_3_10S: RateLimit; PUBLISHER_DON: DurableObjectNamespace /* PublisherDO */; diff --git a/packages/nest/src/implement.test.ts b/packages/nest/src/implement.test.ts index 02142e6aa..b369ebee4 100644 --- a/packages/nest/src/implement.test.ts +++ b/packages/nest/src/implement.test.ts @@ -278,6 +278,127 @@ describe('routing', () => { }) }) }) + + describe.each([ + ['express adapter', undefined], + ['fastify adapter', new FastifyAdapter()], + ] as const)('params edge cases with %s', async (_, adapter) => { + const contract = { + staticPath: oc.meta(openapi({ + path: '/static', + method: 'GET', + })).input(z.object({ tenant: z.string() })), + + dynamicPath: oc.meta(openapi({ + path: '/dynamic/{id}', + method: 'GET', + })).input(z.object({ tenant: z.string(), id: z.string() })), + + restPath: oc.meta(openapi({ + path: '/rest/{+rest}', + method: 'GET', + })).input(z.object({ tenant: z.string(), rest: z.string() })), + + pathNamedParam: oc.meta(openapi({ + path: '/files/{path}', + method: 'GET', + })).input(z.object({ tenant: z.string(), path: z.string() })), + } + + @Controller('/:tenant') + class TenantController { + @Implement(contract.staticPath) + staticPath() { + return implement(contract.staticPath).handler(({ input }) => input) + } + + @Implement(contract.dynamicPath) + dynamicPath() { + return implement(contract.dynamicPath).handler(({ input }) => input) + } + + @Implement(contract.restPath) + restPath() { + return implement(contract.restPath).handler(({ input }) => input) + } + + @Implement(contract.pathNamedParam) + pathNamedParam() { + return implement(contract.pathNamedParam).handler(({ input }) => input) + } + } + + const protoContract = oc.meta(openapi({ + path: '/proto/{+__proto__}', + inputStructure: 'detailed', + })) + + @Controller() + class ProtoController { + @Implement(protoContract) + proto() { + return implement(protoContract).handler(({ input }) => { + const params = (input as any).params + + return { + entries: Object.entries(params), + constructor: typeof params.constructor, + } + }) + } + } + + const moduleRef = await Test.createTestingModule({ + controllers: [TenantController, ProtoController], + }).compile() + + const app = moduleRef.createNestApplication(adapter as any) + await app.init() + + if (adapter) { + await app.getHttpAdapter().getInstance().ready() + } + + const httpServer = app.getHttpServer() + + it('should keep dynamic controller prefix params when the contract path has none', async () => { + const res = await supertest(httpServer).get('/acme/static') + + expect(res.statusCode).toEqual(200) + expect(res.body).toEqual({ tenant: 'acme' }) + }) + + it('should keep dynamic controller prefix params alongside contract params', async () => { + const res = await supertest(httpServer).get('/acme/dynamic/123') + + expect(res.statusCode).toEqual(200) + expect(res.body).toEqual({ tenant: 'acme', id: '123' }) + }) + + it('should keep dynamic controller prefix params alongside contract rest params', async () => { + const res = await supertest(httpServer).get('/acme/rest/some/long/path') + + expect(res.statusCode).toEqual(200) + expect(res.body).toEqual({ tenant: 'acme', rest: 'some/long/path' }) + }) + + it('should keep a non-rest param literally named `path`', async () => { + const res = await supertest(httpServer).get('/acme/files/xxx') + + expect(res.statusCode).toEqual(200) + expect(res.body).toEqual({ tenant: 'acme', path: 'xxx' }) + }) + + it('should treat params named like `__proto__` as own properties without polluting the prototype', async () => { + const res = await supertest(httpServer).post('/proto/some/value') + + expect(res.statusCode).toEqual(200) + expect(res.body).toEqual({ + entries: [['__proto__', 'some/value']], + constructor: 'undefined', + }) + }) + }) }) describe('response status, headers and body should follow standardserver', () => { @@ -1209,6 +1330,44 @@ describe('compatibility', () => { ) }) + it('ignores contract rest params when custom request parser provides no wildcard param', async () => { + const contract = oc.meta(openapi({ + path: '/parser-rest/{+rest}', + inputStructure: 'detailed', + })) + + @Controller() + class ImplController { + @Implement(contract) + parserRest() { + return implement(contract).handler(({ input }) => (input as any).params) + } + } + + const moduleRef = await Test.createTestingModule({ + controllers: [ImplController], + imports: [ + ORPCModule.forRoot({ + toNestStandardLazyRequest: () => ({ + url: '/parser-rest/value', + method: 'POST', + headers: {}, + resolveBody: async () => undefined, + params: { other: '__OTHER__' }, + } satisfies NestStandardLazyRequest), + }), + ], + }).compile() + + const app = moduleRef.createNestApplication() + await app.init() + + const res = await supertest(app.getHttpServer()).post('/parser-rest/some/value') + + expect(res.statusCode).toEqual(200) + expect(res.body).toEqual({ other: '__OTHER__' }) + }) + it('procedure path[] should use meta.path or fall back to empty', async () => { const contract = { without: oc.meta(openapi({ diff --git a/packages/nest/src/implement.ts b/packages/nest/src/implement.ts index 8af8ac8bb..a7a9eb437 100644 --- a/packages/nest/src/implement.ts +++ b/packages/nest/src/implement.ts @@ -15,7 +15,7 @@ import { DEFAULT_OPENAPI_METHOD, getDynamicPathParams, getOpenAPIMeta } from '@o import { OpenAPIHandlerCodecCore } from '@orpc/openapi/standard' import { DEFAULT_SUCCESS_STATUS, getRouter, Procedure, unlazy } from '@orpc/server' import { StandardHandler } from '@orpc/server/standard' -import { isAsyncIteratorObject, mergeHttpPath, stringifyJSON, value } from '@orpc/shared' +import { isAsyncIteratorObject, mergeHttpPath, NullProtoObj, stringifyJSON, value } from '@orpc/shared' import { flattenStandardHeader, generateContentDisposition } from '@standardserver/core' import { toEventStream, toStandardLazyRequest } from '@standardserver/node' import { mergeMap } from 'rxjs' @@ -293,39 +293,41 @@ export class ImplementInterceptor implements NestInterceptor { } } -function flattenParamValue(value: undefined | string | string[]): undefined | string { +function flattenParamValue(value: string | string[]): string { return Array.isArray(value) ? value.join('/') : value } function toORPCOpenAPIParams(contract: AnyProcedureContract, params: NestStandardLazyRequest['params']): undefined | Record { const meta = getOpenAPIMeta(contract) - /* c8 ignore start - there cases almost never happen only for type guard purpose */ - if (!params || meta?.path === undefined) { + if (!params || meta?.path === undefined || Object.keys(params).length === 0) { return undefined } - /* c8 ignore stop */ - const dynamicParams = getDynamicPathParams(meta.prefix ? mergeHttpPath(meta.prefix, meta.path) : meta.path) - if (!dynamicParams) { - return undefined - } + // NullProtoObj prevents prototype injection when a param is named like `__proto__` + const orpcParams: Record = new NullProtoObj() + // express use `path` while fastify use `*` for rest matching + const restKey = Object.hasOwn(params, '*') ? '*' : 'path' + + for (const [key, value] of Object.entries(params)) { + if (key === restKey) { + const restParams = getDynamicPathParams( + meta.prefix ? mergeHttpPath(meta.prefix, meta.path) : meta.path, + )?.filter(c => c.allowsSlash) - return dynamicParams.reduce((acc: Record, config) => { - const value = config.allowsSlash - ? flattenParamValue(params?.['*'] ?? params?.path) // express use `path` while fastify use `*` for rest matching - : flattenParamValue(params?.[config.parameterName]) + if (restParams?.length) { + for (const c of restParams) { + orpcParams[c.parameterName] = flattenParamValue(value) + } - /* c8 ignore start - this case almost never happen only for type guard purpose */ - if (value === undefined) { - return acc + continue + } } - /* c8 ignore stop */ - acc[config.parameterName] = value + orpcParams[key] = flattenParamValue(value) + } - return acc - }, {}) + return orpcParams } function toNestPattern(path: `/${string}`): `/${string}` { diff --git a/playgrounds/cloudflare/worker-configuration.d.ts b/playgrounds/cloudflare/worker-configuration.d.ts index e5a912fa6..12f783d88 100644 --- a/playgrounds/cloudflare/worker-configuration.d.ts +++ b/playgrounds/cloudflare/worker-configuration.d.ts @@ -1,6 +1,6 @@ /* eslint-disable */ // Generated by Wrangler by running `wrangler types` (hash: ddedf26e1bdb86a42eff01a24b02f0d4) -// Runtime types generated with workerd@1.20260722.1 2026-07-01 nodejs_compat +// Runtime types generated with workerd@1.20260730.1 2026-07-01 nodejs_compat interface __BaseEnv_Env { PUBLISHER_DON: DurableObjectNamespace; CHAT_ROOM_DON: DurableObjectNamespace;