From e215420edf4889d116b01ca0f52109e7167e7b63 Mon Sep 17 00:00:00 2001 From: "Marc J. Schmidt" Date: Fri, 9 Feb 2024 02:29:16 +0100 Subject: [PATCH] fix(http): make sure all path parameters are available in HttpRequestParser Also, make sure that cache is correctly placed. --- packages/http/src/module.ts | 11 +-- packages/http/src/request-parser.ts | 16 +++-- packages/http/tests/router.spec.ts | 101 +++++++++++++++++++++++----- 3 files changed, 102 insertions(+), 26 deletions(-) diff --git a/packages/http/src/module.ts b/packages/http/src/module.ts index 25a547059..309a3cd4a 100644 --- a/packages/http/src/module.ts +++ b/packages/http/src/module.ts @@ -80,7 +80,8 @@ export class HttpModule extends createModule({ throw new Error(`Listener ${stringifyListener(listener)} requires async HttpBody. This is not yet supported. You have to parse the request manually by injecting HttpRequest.`); } - let build: Function; + const parserCache = new Map(); + for (let index = 0; index < params.length; index++) { const parameter = params[index]; if (!parameterRequiresRequest(parameter)) continue; @@ -93,14 +94,16 @@ export class HttpModule extends createModule({ this.addProvider({ provide: uniqueType, useFactory: (httpConfig: HttpConfig, request: HttpRequest, injector: InjectorContext, config?: RouteConfig) => { + let build = config && parserCache.get(config); if (!build) { const params = listener.reflection.getParameters().slice(1); - build = buildRequestParser(httpConfig.parser, params, config?.getFullPath()); + build = buildRequestParser(httpConfig.parser, params, config); + if (config) parserCache.set(config, build); } const parser = build(request); - const params = parser(injector); - return params.arguments[i]; + const result = parser(injector); + return result.arguments[i]; }, scope: 'http' }); this.addExport(uniqueType); diff --git a/packages/http/src/request-parser.ts b/packages/http/src/request-parser.ts index f9a721e54..34320f692 100644 --- a/packages/http/src/request-parser.ts +++ b/packages/http/src/request-parser.ts @@ -170,22 +170,28 @@ function isTypeUnknown(type: Type): boolean { || type.kind === ReflectionKind.never; } -export function buildRequestParser(parseOptions: HttpParserOptions, parameters: ReflectionParameter[], path?: string): (request: HttpRequest) => any[] { +export function buildRequestParser(parseOptions: HttpParserOptions, parameters: ReflectionParameter[], routeConfig?: RouteConfig): (request: HttpRequest) => any[] { const compiler = new CompilerContext(); const params = parameters.map(v => new ParameterForRequestParser(v)); //todo: parse path let pathRegex = ''; - if (path) { - const parsedPath = parseRoutePathToRegex(path, parameters); + let pathParameterNames: { [name: string]: number } = {}; + + if (routeConfig) { + const parsedPath = parseRoutePathToRegex(routeConfig.getFullPath(), parameters); pathRegex = parsedPath.regex; + pathParameterNames = parsedPath.parameterNames; for (const param of params) { param.regexPosition = parsedPath.parameterNames[param.parameter.name]; } } - const code = getRequestParserCodeForParameters(compiler, parseOptions, params, {}); + const code = getRequestParserCodeForParameters(compiler, parseOptions, params, { + pathParameterNames, + routeConfig + }); compiler.context.set('ValidationError', ValidationError); compiler.context.set('qs', qs); @@ -254,7 +260,7 @@ export function getRequestParserCodeForParameters( setParameters.push(`parameters.${parameter.parameter.name} = async (options = {}) => { let res = {}; if (options.withPath !== false) { - ${assignPathNames.join('\n')}; + ${assignPathNames.join('\n')} } if (options.withHeader !== false) { Object.assign(res, _headers); diff --git a/packages/http/tests/router.spec.ts b/packages/http/tests/router.spec.ts index bcc1833dc..4d467d7c4 100644 --- a/packages/http/tests/router.spec.ts +++ b/packages/http/tests/router.spec.ts @@ -1141,7 +1141,7 @@ test('parameter from header', async () => { } @http.GET('second/:userId') - handle2(userId: number, groupId: HttpHeader) { + handle2(userId: number, groupId: HttpHeader) { return [userId, groupId]; } } @@ -1307,7 +1307,7 @@ test('body and queries in listener', async () => { } @http.POST('/3') - handle3({ userId }: HttpBody<{userId: number}>, session: HttpSession) { + handle3({ userId }: HttpBody<{ userId: number }>, session: HttpSession) { return [userId, session.userId, session.auth]; } @@ -1316,16 +1316,28 @@ test('body and queries in listener', async () => { const data = await parser(); return [data.auth, data.userId]; } + @http.GET('/5/:userId') async handle5(userId: number, parser: HttpRequestParser) { const data = await parser(); return [userId, data.auth, data.userId]; } + @http.GET('/6/:userId') async handle6(parser: HttpRequestParser) { const data = await parser(); return [data.auth, data.userId]; } + + @http.GET('/7/:userId') + async handle7(session: HttpSession) { + return [session.auth, session.userId]; + } + + @http.GET('/8/:id/:userId') + async handle8(id: string, session: HttpSession) { + return [id, session.auth, session.userId]; + } } type AuthData = { @@ -1343,14 +1355,16 @@ test('body and queries in listener', async () => { } const httpKernel = createHttpKernel([Controller], [{ provide: HttpSession, scope: 'http' }], [Listener]); - expect((await httpKernel.request(HttpRequest.POST('/1?userId=1').json({auth: 'secretToken1'}))).json).toEqual([1, 1, 'secretToken1']); + expect((await httpKernel.request(HttpRequest.POST('/1?userId=1').json({ auth: 'secretToken1' }))).json).toEqual([1, 1, 'secretToken1']); expect((await httpKernel.request(HttpRequest.GET('/2?auth=secretToken1&userId=1'))).json).toEqual([1, 1, 'secretToken1']); expect((await httpKernel.request(HttpRequest.GET('/2?userId=1'))).json.message).toEqual('Validation error:\nauth(type): Not a string'); - expect((await httpKernel.request(HttpRequest.POST('/3?auth=secretToken1').json({userId: '24'}))).json).toEqual([24, 24, 'secretToken1']); + expect((await httpKernel.request(HttpRequest.POST('/3?auth=secretToken1').json({ userId: '24' }))).json).toEqual([24, 24, 'secretToken1']); expect((await httpKernel.request(HttpRequest.GET('/4?auth=secretToken1&userId=1'))).json).toEqual(['secretToken1', '1']); expect((await httpKernel.request(HttpRequest.GET('/4?userId=1').header('auth', 'secretToken1'))).json).toEqual(['secretToken1', '1']); expect((await httpKernel.request(HttpRequest.GET('/5/1?auth=secretToken1'))).json).toEqual([1, 'secretToken1', '1']); - expect((await httpKernel.request(HttpRequest.GET('/6/1?auth=secretToken1'))).json).toEqual(['secretToken1', '1']); + expect((await httpKernel.request(HttpRequest.GET('/6/2?auth=secretToken1'))).json).toEqual(['secretToken1', '2']); + expect((await httpKernel.request(HttpRequest.GET('/7/3?auth=secretToken1'))).json).toEqual(['secretToken1', 3]); + expect((await httpKernel.request(HttpRequest.GET('/8/1/3?auth=secretToken1'))).json).toEqual(['1', 'secretToken1', 3]); }); test('stream', async () => { @@ -1442,8 +1456,8 @@ test('upload security', async () => { path: '/etc/secure-file', name: 'fakefile', type: 'image/jpeg', - lastModifiedDate: null - } + lastModifiedDate: null, + }, }))).json.message).toContain('Not an uploaded file caused by value'); // ensure type deserialization doesn't set the invalid 'fake value' value to UploadedFileSymbol @@ -1454,16 +1468,16 @@ test('upload security', async () => { path: '/etc/secure-file', name: 'fakefile', type: 'image/jpeg', - lastModifiedDate: null - } + lastModifiedDate: null, + }, }))).json.message).toContain('Not an uploaded file caused by value'); expect((await httpKernel.request(HttpRequest.POST('/upload').multiPart([ { name: 'someFile', file: Buffer.from('testing a text file'), - fileName: 'test.txt' - } + fileName: 'test.txt', + }, ]))).json).toMatchObject({ uploadedSize: 19 }); }); @@ -1622,8 +1636,8 @@ test('dependency injection unknown', async () => { }); test('disabled reflection', async () => { - type User = {username: string}; - type Doc = {id: number}; + type User = { username: string }; + type Doc = { id: number }; /** @reflection never */ type PopulateUser = T & { user: User }; @@ -1633,9 +1647,9 @@ test('disabled reflection', async () => { const user: DocUser = { id: 1, user: { - username: 'peter2' - } - } + username: 'peter2', + }, + }; class Controller { @http.GET('user/:name') @@ -1644,6 +1658,59 @@ test('disabled reflection', async () => { } } - const httpKernel = createHttpKernel([Controller], [provide({useValue: user})]); + const httpKernel = createHttpKernel([Controller], [provide({ useValue: user })]); expect((await httpKernel.request(HttpRequest.GET('/user/peter'))).json).toEqual(['peter', 1, 'peter2']); +}); + +test('throw when invalid data returned', async () => { + // todo: auto serialization of route return value should throw if invalid data was given +}); + +test('http optional query', async () => { + class Controller { + @http.GET('user') + async anyReq(name: HttpQuery) { + return [name]; + } + } + + const httpKernel = createHttpKernel([Controller]); + expect((await httpKernel.request(HttpRequest.GET('/user'))).json).toEqual([null]); + expect((await httpKernel.request(HttpRequest.GET('/user?name=Peter'))).json).toEqual(['Peter']); +}); + +test('required query should be required', async () => { + class Controller { + @http.GET('') + async anyReq(entityId: HttpQuery) { + return [entityId]; + } + } + + const httpKernel = createHttpKernel([Controller]); + expect((await httpKernel.request(HttpRequest.GET('/'))).json).toMatchObject({ + message: 'Validation error:\nentityId(type): No value given', + }); + expect((await httpKernel.request(HttpRequest.GET('/?entityId=asd'))).json).toEqual(['asd']); +}); + +test('required fields in body should be required', async () => { + interface Body { + a: string | null; + b: string; + } + + class Controller { + @http.POST('') + async anyReq(body: HttpBody) { + return [body.a, body.b]; + } + } + + const httpKernel = createHttpKernel([Controller]); + expect((await httpKernel.request(HttpRequest.POST('/'))).json).toMatchObject({ + message: 'Validation error:\nb(type): Not a string', + }); + expect((await httpKernel.request(HttpRequest.POST('/').json({ b: 'asd' }))).json).toEqual([null, 'asd']); + expect((await httpKernel.request(HttpRequest.POST('/').json({ a: 'a', b: 'asd' }))).json).toEqual(['a', 'asd']); })