From 9fa3d930db7236e2235e6c09cfd3ca98f0b27eb3 Mon Sep 17 00:00:00 2001 From: codytseng Date: Tue, 1 Aug 2023 21:33:00 +0800 Subject: [PATCH 1/6] fix: the root path middleware not applied when setting the global prefix --- .../global-prefix/e2e/global-prefix.spec.ts | 4 ++++ .../global-prefix/src/app.controller.ts | 5 +++++ packages/core/middleware/route-info-path-extractor.ts | 8 ++++++-- .../test/middleware/route-info-path-extractor.spec.ts | 10 +++++----- 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/integration/nest-application/global-prefix/e2e/global-prefix.spec.ts b/integration/nest-application/global-prefix/e2e/global-prefix.spec.ts index 74f8e9ad6fc..01973ed17fa 100644 --- a/integration/nest-application/global-prefix/e2e/global-prefix.spec.ts +++ b/integration/nest-application/global-prefix/e2e/global-prefix.spec.ts @@ -28,6 +28,10 @@ describe('Global prefix', () => { await request(server).get('/health').expect(404); await request(server).get('/api/v1/health').expect(200); + + await request(server) + .get('/api/v1') + .expect(200, 'Root: Data attached in middleware'); }); it(`should exclude the path as string`, async () => { diff --git a/integration/nest-application/global-prefix/src/app.controller.ts b/integration/nest-application/global-prefix/src/app.controller.ts index d826cd0dbc2..35444679426 100644 --- a/integration/nest-application/global-prefix/src/app.controller.ts +++ b/integration/nest-application/global-prefix/src/app.controller.ts @@ -2,6 +2,11 @@ import { Controller, Get, Post, Req } from '@nestjs/common'; @Controller() export class AppController { + @Get() + root(@Req() req): string { + return 'Root: ' + req.extras?.data; + } + @Get('hello/:name') getHello(@Req() req): string { return 'Hello: ' + req.extras?.data; diff --git a/packages/core/middleware/route-info-path-extractor.ts b/packages/core/middleware/route-info-path-extractor.ts index c3d2bf8bfb6..c75b9ca29e5 100644 --- a/packages/core/middleware/route-info-path-extractor.ts +++ b/packages/core/middleware/route-info-path-extractor.ts @@ -33,14 +33,18 @@ export class RouteInfoPathExtractor { const versionPath = this.extractVersionPathFrom(version); if (this.isAWildcard(path)) { + const prefix = addLeadingSlash(this.prefixPath + versionPath); + const basePaths = prefix + ? [prefix, prefix + addLeadingSlash(path)] + : [addLeadingSlash(path)]; return Array.isArray(this.excludedGlobalPrefixRoutes) ? [ - this.prefixPath + versionPath + addLeadingSlash(path), + ...basePaths, ...this.excludedGlobalPrefixRoutes.map( route => versionPath + addLeadingSlash(route.path), ), ] - : [this.prefixPath + versionPath + addLeadingSlash(path)]; + : basePaths; } return [this.extractNonWildcardPathFrom({ path, method, version })]; diff --git a/packages/core/test/middleware/route-info-path-extractor.spec.ts b/packages/core/test/middleware/route-info-path-extractor.spec.ts index 967b26243bd..e3f258bb2ec 100644 --- a/packages/core/test/middleware/route-info-path-extractor.spec.ts +++ b/packages/core/test/middleware/route-info-path-extractor.spec.ts @@ -31,7 +31,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/v1/*']); + ).to.eql(['/v1', '/v1/*']); }); it(`should return correct paths when set global prefix`, () => { @@ -42,7 +42,7 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['/api/*']); + ).to.eql(['/api', '/api/*']); expect( routeInfoPathExtractor.extractPathsFrom({ @@ -50,7 +50,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/api/v1/*']); + ).to.eql(['/api/v1', '/api/v1/*']); }); it(`should return correct paths when set global prefix and global prefix options`, () => { @@ -66,7 +66,7 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['/api/*', '/foo']); + ).to.eql(['/api', '/api/*', '/foo']); expect( routeInfoPathExtractor.extractPathsFrom({ @@ -74,7 +74,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/api/v1/*', '/v1/foo']); + ).to.eql(['/api/v1', '/api/v1/*', '/v1/foo']); expect( routeInfoPathExtractor.extractPathsFrom({ From edd5eaf7186c47ecfb2b7cb3d58d294c57a8c176 Mon Sep 17 00:00:00 2001 From: codytseng Date: Thu, 24 Aug 2023 22:17:40 +0800 Subject: [PATCH 2/6] fix: middleware applied twice --- .../core/middleware/route-info-path-extractor.ts | 7 ++----- .../middleware/route-info-path-extractor.spec.ts | 12 ++++++------ 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/core/middleware/route-info-path-extractor.ts b/packages/core/middleware/route-info-path-extractor.ts index c75b9ca29e5..2d24625b101 100644 --- a/packages/core/middleware/route-info-path-extractor.ts +++ b/packages/core/middleware/route-info-path-extractor.ts @@ -34,17 +34,14 @@ export class RouteInfoPathExtractor { if (this.isAWildcard(path)) { const prefix = addLeadingSlash(this.prefixPath + versionPath); - const basePaths = prefix - ? [prefix, prefix + addLeadingSlash(path)] - : [addLeadingSlash(path)]; return Array.isArray(this.excludedGlobalPrefixRoutes) ? [ - ...basePaths, + prefix, ...this.excludedGlobalPrefixRoutes.map( route => versionPath + addLeadingSlash(route.path), ), ] - : basePaths; + : [prefix]; } return [this.extractNonWildcardPathFrom({ path, method, version })]; diff --git a/packages/core/test/middleware/route-info-path-extractor.spec.ts b/packages/core/test/middleware/route-info-path-extractor.spec.ts index e3f258bb2ec..c4791e255c3 100644 --- a/packages/core/test/middleware/route-info-path-extractor.spec.ts +++ b/packages/core/test/middleware/route-info-path-extractor.spec.ts @@ -23,7 +23,7 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['/*']); + ).to.eql(['']); expect( routeInfoPathExtractor.extractPathsFrom({ @@ -31,7 +31,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/v1', '/v1/*']); + ).to.eql(['/v1']); }); it(`should return correct paths when set global prefix`, () => { @@ -42,7 +42,7 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['/api', '/api/*']); + ).to.eql(['/api']); expect( routeInfoPathExtractor.extractPathsFrom({ @@ -50,7 +50,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/api/v1', '/api/v1/*']); + ).to.eql(['/api/v1']); }); it(`should return correct paths when set global prefix and global prefix options`, () => { @@ -66,7 +66,7 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['/api', '/api/*', '/foo']); + ).to.eql(['/api', '/foo']); expect( routeInfoPathExtractor.extractPathsFrom({ @@ -74,7 +74,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/api/v1', '/api/v1/*', '/v1/foo']); + ).to.eql(['/api/v1', '/v1/foo']); expect( routeInfoPathExtractor.extractPathsFrom({ From 73ff3ac81dac8dc83f714492d8bb9e21fe2528af Mon Sep 17 00:00:00 2001 From: codytseng Date: Thu, 24 Aug 2023 23:30:35 +0800 Subject: [PATCH 3/6] Revert "fix: middleware applied twice" This reverts commit edd5eaf7186c47ecfb2b7cb3d58d294c57a8c176. --- .../core/middleware/route-info-path-extractor.ts | 7 +++++-- .../middleware/route-info-path-extractor.spec.ts | 12 ++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/core/middleware/route-info-path-extractor.ts b/packages/core/middleware/route-info-path-extractor.ts index 2d24625b101..c75b9ca29e5 100644 --- a/packages/core/middleware/route-info-path-extractor.ts +++ b/packages/core/middleware/route-info-path-extractor.ts @@ -34,14 +34,17 @@ export class RouteInfoPathExtractor { if (this.isAWildcard(path)) { const prefix = addLeadingSlash(this.prefixPath + versionPath); + const basePaths = prefix + ? [prefix, prefix + addLeadingSlash(path)] + : [addLeadingSlash(path)]; return Array.isArray(this.excludedGlobalPrefixRoutes) ? [ - prefix, + ...basePaths, ...this.excludedGlobalPrefixRoutes.map( route => versionPath + addLeadingSlash(route.path), ), ] - : [prefix]; + : basePaths; } return [this.extractNonWildcardPathFrom({ path, method, version })]; diff --git a/packages/core/test/middleware/route-info-path-extractor.spec.ts b/packages/core/test/middleware/route-info-path-extractor.spec.ts index c4791e255c3..e3f258bb2ec 100644 --- a/packages/core/test/middleware/route-info-path-extractor.spec.ts +++ b/packages/core/test/middleware/route-info-path-extractor.spec.ts @@ -23,7 +23,7 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['']); + ).to.eql(['/*']); expect( routeInfoPathExtractor.extractPathsFrom({ @@ -31,7 +31,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/v1']); + ).to.eql(['/v1', '/v1/*']); }); it(`should return correct paths when set global prefix`, () => { @@ -42,7 +42,7 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['/api']); + ).to.eql(['/api', '/api/*']); expect( routeInfoPathExtractor.extractPathsFrom({ @@ -50,7 +50,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/api/v1']); + ).to.eql(['/api/v1', '/api/v1/*']); }); it(`should return correct paths when set global prefix and global prefix options`, () => { @@ -66,7 +66,7 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['/api', '/foo']); + ).to.eql(['/api', '/api/*', '/foo']); expect( routeInfoPathExtractor.extractPathsFrom({ @@ -74,7 +74,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/api/v1', '/v1/foo']); + ).to.eql(['/api/v1', '/api/v1/*', '/v1/foo']); expect( routeInfoPathExtractor.extractPathsFrom({ From 3b100cc97142066b2f6f6cdbdb891099511486d5 Mon Sep 17 00:00:00 2001 From: codytseng Date: Thu, 24 Aug 2023 23:32:24 +0800 Subject: [PATCH 4/6] feat: change middleware default request method --- packages/core/middleware/routes-mapper.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core/middleware/routes-mapper.ts b/packages/core/middleware/routes-mapper.ts index 90193250a35..0bfa8ade8bb 100644 --- a/packages/core/middleware/routes-mapper.ts +++ b/packages/core/middleware/routes-mapper.ts @@ -20,6 +20,7 @@ import { Module } from '../injector/module'; import { MetadataScanner } from '../metadata-scanner'; import { PathsExplorer, RouteDefinition } from '../router/paths-explorer'; import { targetModulesByContainer } from '../router/router-module'; +import { RequestMethod } from '@nestjs/common'; export class RoutesMapper { private readonly pathsExplorer: PathsExplorer; @@ -46,7 +47,7 @@ export class RoutesMapper { } private getRouteInfoFromPath(routePath: string): RouteInfo[] { - const defaultRequestMethod = -1; + const defaultRequestMethod = RequestMethod.ALL; return [ { path: addLeadingSlash(routePath), From 16106e084c06656409c393fd897f222ce0c43670 Mon Sep 17 00:00:00 2001 From: codytseng Date: Thu, 24 Aug 2023 23:56:53 +0800 Subject: [PATCH 5/6] test: default request method --- packages/core/test/middleware/builder.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/test/middleware/builder.spec.ts b/packages/core/test/middleware/builder.spec.ts index fa73acae66a..f3854a5bf1d 100644 --- a/packages/core/test/middleware/builder.spec.ts +++ b/packages/core/test/middleware/builder.spec.ts @@ -193,7 +193,7 @@ describe('MiddlewareBuilder', () => { expect(proxy.getExcludedRoutes()).to.be.eql([ { path, - method: -1 as any, + method: RequestMethod.ALL, }, ]); }); From 3321f6ca20345c61d0b0718079edf7ac73a6c62d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20My=C5=9Bliwiec?= Date: Mon, 18 Mar 2024 11:35:49 +0100 Subject: [PATCH 6/6] fix(core): middleware is not executed for root route with prefix --- .../global-prefix/e2e/global-prefix.spec.ts | 8 +++----- .../global-prefix/src/app.controller.ts | 7 +------ packages/core/middleware/middleware-module.ts | 2 ++ .../core/middleware/route-info-path-extractor.ts | 4 ++-- packages/core/middleware/routes-mapper.ts | 3 +-- packages/core/test/middleware/builder.spec.ts | 2 +- .../middleware/route-info-path-extractor.spec.ts | 10 +++++----- .../platform-fastify/adapters/fastify-adapter.ts | 14 +++++++++----- 8 files changed, 24 insertions(+), 26 deletions(-) diff --git a/integration/nest-application/global-prefix/e2e/global-prefix.spec.ts b/integration/nest-application/global-prefix/e2e/global-prefix.spec.ts index af2404fde7e..76acf712137 100644 --- a/integration/nest-application/global-prefix/e2e/global-prefix.spec.ts +++ b/integration/nest-application/global-prefix/e2e/global-prefix.spec.ts @@ -28,10 +28,6 @@ describe('Global prefix', () => { await request(server).get('/health').expect(404); await request(server).get('/api/v1/health').expect(200); - - await request(server) - .get('/api/v1') - .expect(200, 'Root: Data attached in middleware'); }); it(`should exclude the path as string`, async () => { @@ -140,7 +136,9 @@ describe('Global prefix', () => { server = app.getHttpServer(); await app.init(); - await request(server).get('/').expect(200, '1'); + await request(server) + .get('/') + .expect(200, 'Extras: Data attached in middleware, Count: 1'); await request(server).get('/api/count').expect(200, '2'); }); diff --git a/integration/nest-application/global-prefix/src/app.controller.ts b/integration/nest-application/global-prefix/src/app.controller.ts index a3c3bea664c..416ca649e5e 100644 --- a/integration/nest-application/global-prefix/src/app.controller.ts +++ b/integration/nest-application/global-prefix/src/app.controller.ts @@ -2,11 +2,6 @@ import { Controller, Get, Post, Req } from '@nestjs/common'; @Controller() export class AppController { - @Get() - root(@Req() req): string { - return 'Root: ' + req.extras?.data; - } - @Get('hello/:name') getHello(@Req() req): string { return 'Hello: ' + req.extras?.data; @@ -34,7 +29,7 @@ export class AppController { @Get() getHome(@Req() req) { - return req.count; + return 'Extras: ' + req.extras?.data + ', Count: ' + req.count; } @Get('count') diff --git a/packages/core/middleware/middleware-module.ts b/packages/core/middleware/middleware-module.ts index b0bf92a3fca..b956a23850f 100644 --- a/packages/core/middleware/middleware-module.ts +++ b/packages/core/middleware/middleware-module.ts @@ -190,12 +190,14 @@ export class MiddlewareModule< for (const metatype of middlewareCollection) { const collection = middlewareContainer.getMiddlewareCollection(moduleKey); const instanceWrapper = collection.get(metatype); + if (isUndefined(instanceWrapper)) { throw new RuntimeException(); } if (instanceWrapper.isTransient) { return; } + this.graphInspector.insertClassNode( moduleRef, instanceWrapper, diff --git a/packages/core/middleware/route-info-path-extractor.ts b/packages/core/middleware/route-info-path-extractor.ts index caf3f0c6f42..263b12679db 100644 --- a/packages/core/middleware/route-info-path-extractor.ts +++ b/packages/core/middleware/route-info-path-extractor.ts @@ -37,12 +37,12 @@ export class RouteInfoPathExtractor { versionPaths.length > 0 ? versionPaths .map(versionPath => [ - this.prefixPath + versionPath, + this.prefixPath + versionPath + '$', this.prefixPath + versionPath + addLeadingSlash(path), ]) .flat() : this.prefixPath - ? [this.prefixPath, this.prefixPath + addLeadingSlash(path)] + ? [this.prefixPath + '$', this.prefixPath + addLeadingSlash(path)] : [addLeadingSlash(path)]; return Array.isArray(this.excludedGlobalPrefixRoutes) diff --git a/packages/core/middleware/routes-mapper.ts b/packages/core/middleware/routes-mapper.ts index ce8f3e9b0bf..bd491277d89 100644 --- a/packages/core/middleware/routes-mapper.ts +++ b/packages/core/middleware/routes-mapper.ts @@ -20,7 +20,6 @@ import { Module } from '../injector/module'; import { MetadataScanner } from '../metadata-scanner'; import { PathsExplorer, RouteDefinition } from '../router/paths-explorer'; import { targetModulesByContainer } from '../router/router-module'; -import { RequestMethod } from '@nestjs/common'; export class RoutesMapper { private readonly pathsExplorer: PathsExplorer; @@ -47,7 +46,7 @@ export class RoutesMapper { } private getRouteInfoFromPath(routePath: string): RouteInfo[] { - const defaultRequestMethod = RequestMethod.ALL; + const defaultRequestMethod = -1; return [ { path: addLeadingSlash(routePath), diff --git a/packages/core/test/middleware/builder.spec.ts b/packages/core/test/middleware/builder.spec.ts index f3854a5bf1d..0f625f71496 100644 --- a/packages/core/test/middleware/builder.spec.ts +++ b/packages/core/test/middleware/builder.spec.ts @@ -193,7 +193,7 @@ describe('MiddlewareBuilder', () => { expect(proxy.getExcludedRoutes()).to.be.eql([ { path, - method: RequestMethod.ALL, + method: -1, }, ]); }); diff --git a/packages/core/test/middleware/route-info-path-extractor.spec.ts b/packages/core/test/middleware/route-info-path-extractor.spec.ts index a836d4a0342..62bfb5fc8d7 100644 --- a/packages/core/test/middleware/route-info-path-extractor.spec.ts +++ b/packages/core/test/middleware/route-info-path-extractor.spec.ts @@ -31,7 +31,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/v1', '/v1/*']); + ).to.eql(['/v1$', '/v1/*']); }); it(`should return correct paths when set global prefix`, () => { @@ -42,7 +42,7 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['/api', '/api/*']); + ).to.eql(['/api$', '/api/*']); expect( routeInfoPathExtractor.extractPathsFrom({ @@ -50,7 +50,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/api/v1', '/api/v1/*']); + ).to.eql(['/api/v1$', '/api/v1/*']); }); it(`should return correct paths when set global prefix and global prefix options`, () => { @@ -66,7 +66,7 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['/api', '/api/*', '/foo']); + ).to.eql(['/api$', '/api/*', '/foo']); expect( routeInfoPathExtractor.extractPathsFrom({ @@ -74,7 +74,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/api/v1', '/api/v1/*', '/v1/foo']); + ).to.eql(['/api/v1$', '/api/v1/*', '/v1/foo']); expect( routeInfoPathExtractor.extractPathsFrom({ diff --git a/packages/platform-fastify/adapters/fastify-adapter.ts b/packages/platform-fastify/adapters/fastify-adapter.ts index 54e2df387e6..a2fb37046f8 100644 --- a/packages/platform-fastify/adapters/fastify-adapter.ts +++ b/packages/platform-fastify/adapters/fastify-adapter.ts @@ -47,15 +47,15 @@ import { import * as pathToRegexp from 'path-to-regexp'; // `querystring` is used internally in fastify for registering urlencoded body parser. import { parse as querystringParse } from 'querystring'; +import { + FASTIFY_ROUTE_CONFIG_METADATA, + FASTIFY_ROUTE_CONSTRAINTS_METADATA, +} from '../constants'; import { NestFastifyBodyParserOptions } from '../interfaces'; import { FastifyStaticOptions, FastifyViewOptions, } from '../interfaces/external'; -import { - FASTIFY_ROUTE_CONFIG_METADATA, - FASTIFY_ROUTE_CONSTRAINTS_METADATA, -} from '../constants'; type FastifyHttp2SecureOptions< Server extends http2.Http2SecureServer, @@ -554,6 +554,9 @@ export class FastifyAdapter< await this.registerMiddie(); } return (path: string, callback: Function) => { + const hasEndOfStringCharacter = path.endsWith('$'); + path = hasEndOfStringCharacter ? path.slice(0, -1) : path; + let normalizedPath = path.endsWith('/*') ? `${path.slice(0, -1)}(.*)` : path; @@ -561,7 +564,8 @@ export class FastifyAdapter< // Fallback to "(.*)" to support plugins like GraphQL normalizedPath = normalizedPath === '/(.*)' ? '(.*)' : normalizedPath; - const re = pathToRegexp(normalizedPath); + let re = pathToRegexp(normalizedPath); + re = hasEndOfStringCharacter ? new RegExp(re.source + '$', re.flags) : re; // The following type assertion is valid as we use import('@fastify/middie') rather than require('@fastify/middie') // ref https://github.com/fastify/middie/pull/55