mirror of
https://github.com/nestjs/nest.git
synced 2026-10-28 17:56:01 +08:00
fix(core): mount wildcard middleware on every path when "/" is excluded
When the root path is excluded from the global prefix, the paths of a
wildcard middleware include "/" next to the prefixed ones, and
registerHandler() then registered "/" alone. Only Express "use()" reads
"/" as every path: a wildcard bound to a method ({ path: '*', method })
went through all() or get() and ran on "/" only, so the whole prefixed
API was served without it. On Fastify, which rewrites a bare "/" to
"<prefix>/{*path}", the other routes excluded from the prefix lost it.
The middleware is now registered on each of its paths, the root path
first so that "use()" still runs it for every request with "req.url"
unchanged, and it runs for the first of them that matches a request
only. This also stops it from running twice for "/api/" on Fastify, and
replaces the Express adapter's own check for "/api/". FastifyAdapter
still rewrites a bare "/", so on Fastify the root path itself is still
served without the middleware.
Discovered by Anthropic's OSS Scanner, as vulnerability ANT-2026-P3SFKBU4.
This commit is contained in:
6 files changed
+162
-46
No files matched your search
@@ -821,6 +821,60 @@ describe('Middleware (FastifyAdapter)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('should run middleware once on routes excluded from the global prefix with the root path', () => {
|
||||
@Controller()
|
||||
class CountingController {
|
||||
@Get('metrics')
|
||||
metrics(@Req() req: FastifyRequest) {
|
||||
return { runs: req.raw['runs'] ?? 0 };
|
||||
}
|
||||
|
||||
@Get('data')
|
||||
data(@Req() req: FastifyRequest) {
|
||||
return { runs: req.raw['runs'] ?? 0 };
|
||||
}
|
||||
}
|
||||
|
||||
@Module({
|
||||
controllers: [CountingController],
|
||||
})
|
||||
class CountingModule implements NestModule {
|
||||
configure(consumer: MiddlewareConsumer) {
|
||||
consumer
|
||||
.apply((req, res, next) => {
|
||||
req.runs = (req.runs ?? 0) + 1;
|
||||
next();
|
||||
})
|
||||
.forRoutes('{*path}');
|
||||
}
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
app = (
|
||||
await Test.createTestingModule({
|
||||
imports: [CountingModule],
|
||||
}).compile()
|
||||
).createNestApplication<NestFastifyApplication>(new FastifyAdapter());
|
||||
});
|
||||
|
||||
it(`GET forRoutes('{*path}') with the root path and another route excluded from the global prefix`, async () => {
|
||||
app.setGlobalPrefix('/api', { exclude: ['/', 'metrics'] });
|
||||
await listenOnLoopback(app);
|
||||
await app.getHttpAdapter().getInstance().ready();
|
||||
|
||||
await request(app.getHttpServer())
|
||||
.get('/metrics')
|
||||
.expect(200, { runs: 1 });
|
||||
await request(app.getHttpServer())
|
||||
.get('/api/data')
|
||||
.expect(200, { runs: 1 });
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await app.close();
|
||||
});
|
||||
});
|
||||
|
||||
describe('should respect fastify routing options', () => {
|
||||
const MIDDLEWARE_RETURN_VALUE = 'middleware_return';
|
||||
|
||||
|
||||
@@ -143,6 +143,24 @@ describe('Global prefix', () => {
|
||||
await request(server).get('/api/count').expect(200, '2');
|
||||
});
|
||||
|
||||
it(`should run a wildcard middleware bound to a method under the prefix when the root path is excluded`, async () => {
|
||||
app.setGlobalPrefix('/api', { exclude: ['/', 'health'] });
|
||||
|
||||
server = app.getHttpServer();
|
||||
await listenOnLoopback(app);
|
||||
|
||||
await request(server)
|
||||
.get('/api/hello/foo')
|
||||
.expect(200, 'Hello: Data attached in middleware');
|
||||
await request(server)
|
||||
.get('/api/params')
|
||||
.expect(200, { path: ['params'] });
|
||||
await request(server)
|
||||
.get('/')
|
||||
.expect(200, 'Extras: Data attached in middleware, Count: 3');
|
||||
await request(server).get('/api/count').expect(200, '4');
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await app.close();
|
||||
});
|
||||
|
||||
@@ -343,11 +343,27 @@ export class MiddlewareModule<
|
||||
}
|
||||
return next();
|
||||
};
|
||||
const pathsToApplyMiddleware = [] as string[];
|
||||
paths.some(path => path.match(/^\/?$/))
|
||||
? pathsToApplyMiddleware.push('/')
|
||||
: pathsToApplyMiddleware.push(...paths);
|
||||
pathsToApplyMiddleware.forEach(path => router(path, middlewareFunction));
|
||||
// The paths may overlap (e.g. "/" and "/api/*" when the root path is
|
||||
// excluded from the global prefix): the middleware runs for the first of
|
||||
// them that matches a request, and the others skip that request
|
||||
const handledRequests = new WeakSet<object>();
|
||||
const handler = <TRequest, TResponse>(
|
||||
req: TRequest,
|
||||
res: TResponse,
|
||||
next: () => void,
|
||||
) => {
|
||||
if (handledRequests.has(req as object)) {
|
||||
return next();
|
||||
}
|
||||
handledRequests.add(req as object);
|
||||
return middlewareFunction(req, res, next);
|
||||
};
|
||||
// The root path goes first: mounted with Express "use()", it then runs
|
||||
// the middleware for every request, with "req.url" left as it is
|
||||
const pathsToApplyMiddleware = paths
|
||||
.map(path => path || '/')
|
||||
.sort((a, b) => Number(b === '/') - Number(a === '/'));
|
||||
pathsToApplyMiddleware.forEach(path => router(path, handler));
|
||||
}
|
||||
|
||||
private getContextId(request: unknown, isTreeDurable: boolean): ContextId {
|
||||
|
||||
@@ -15,6 +15,7 @@ import { GraphInspector } from '../../inspector/graph-inspector.js';
|
||||
import { MiddlewareBuilder } from '../../middleware/builder.js';
|
||||
import { MiddlewareContainer } from '../../middleware/container.js';
|
||||
import { MiddlewareModule } from '../../middleware/middleware-module.js';
|
||||
import { mapToExcludeRoute } from '../../middleware/utils.js';
|
||||
import { RouterExceptionFilters } from '../../router/router-exception-filters.js';
|
||||
import { NoopHttpAdapter } from '../utils/noop-adapter.js';
|
||||
|
||||
@@ -207,6 +208,64 @@ describe('MiddlewareModule', () => {
|
||||
expect(createMiddlewareFactoryStub).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it('should mount the middleware on each path, the root path first, and run it once for a request', async () => {
|
||||
const appConfig = new ApplicationConfig();
|
||||
appConfig.setGlobalPrefix('api');
|
||||
appConfig.setGlobalPrefixOptions({ exclude: mapToExcludeRoute(['/']) });
|
||||
middlewareModule['routeInfoPathExtractor'] = new RouteInfoPathExtractor(
|
||||
appConfig,
|
||||
);
|
||||
const configuration = {
|
||||
middleware: [TestMiddleware],
|
||||
forRoutes: ['*'],
|
||||
};
|
||||
|
||||
const router = vi.fn();
|
||||
const app = {
|
||||
createMiddlewareFactory: vi.fn().mockResolvedValue(router),
|
||||
};
|
||||
|
||||
const stubContainer = new NestContainer();
|
||||
const container = new MiddlewareContainer(stubContainer);
|
||||
const moduleKey = 'Test';
|
||||
stubContainer
|
||||
.getModules()
|
||||
.set(moduleKey, new Module(TestModule, stubContainer));
|
||||
container.insertConfig([configuration], moduleKey);
|
||||
|
||||
const instance = new TestMiddleware();
|
||||
const useSpy = vi.spyOn(instance, 'use');
|
||||
container.getMiddlewareCollection(moduleKey).set(
|
||||
TestMiddleware,
|
||||
new InstanceWrapper({
|
||||
metatype: TestMiddleware,
|
||||
instance,
|
||||
}),
|
||||
);
|
||||
middlewareModule['container'] = stubContainer;
|
||||
|
||||
await middlewareModule.registerRouteMiddleware(
|
||||
container,
|
||||
{ path: '*', method: RequestMethod.ALL },
|
||||
configuration,
|
||||
moduleKey,
|
||||
app,
|
||||
);
|
||||
expect(router.mock.calls.map(([path]) => path)).toEqual([
|
||||
'/',
|
||||
'/api$',
|
||||
'/api/*',
|
||||
]);
|
||||
|
||||
const req = {};
|
||||
const next = vi.fn();
|
||||
for (const [, handler] of router.mock.calls) {
|
||||
await handler(req, {}, next);
|
||||
}
|
||||
expect(useSpy).toHaveBeenCalledOnce();
|
||||
expect(next).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('should mount a transient middleware and the middleware listed after it', async () => {
|
||||
@Injectable({ scope: Scope.TRANSIENT })
|
||||
class TransientMiddleware implements NestMiddleware {
|
||||
|
||||
@@ -400,7 +400,6 @@ export class ExpressAdapter extends AbstractHttpAdapter<
|
||||
public createMiddlewareFactory(
|
||||
requestMethod: RequestMethod,
|
||||
): (path: string, callback: Function) => any {
|
||||
const handledRequests = new WeakMap<Function, WeakSet<object>>();
|
||||
return (path: string, callback: Function) => {
|
||||
try {
|
||||
// The core marks an exact-match path with a trailing "$" (e.g. "/api$").
|
||||
@@ -416,18 +415,9 @@ export class ExpressAdapter extends AbstractHttpAdapter<
|
||||
}
|
||||
// Express routes are not strict, so "/api" also matches "/api/", which
|
||||
// the wildcard path registered next to it may match too ("/api/{*path}"
|
||||
// does, "/api/*path" does not). The middleware runs for the first of
|
||||
// its paths that matches a request, and not again for the others.
|
||||
const requests = handledRequests.get(callback) ?? new WeakSet();
|
||||
handledRequests.set(callback, requests);
|
||||
const handler = (req: any, res: any, next: Function) => {
|
||||
if (requests.has(req)) {
|
||||
return next();
|
||||
}
|
||||
requests.add(req);
|
||||
return callback(req, res, next);
|
||||
};
|
||||
return router.call(this.instance, convertedPath, handler);
|
||||
// does, "/api/*path" does not). The core runs the middleware for the
|
||||
// first of its paths that matches a request only.
|
||||
return router.call(this.instance, convertedPath, callback);
|
||||
} catch (e) {
|
||||
if (e instanceof TypeError) {
|
||||
LegacyRouteConverter.printError(path);
|
||||
|
||||
@@ -198,38 +198,17 @@ describe('ExpressAdapter', () => {
|
||||
},
|
||||
);
|
||||
|
||||
it('should run the middleware once for a request matching more than one of its paths', () => {
|
||||
it('should register the middleware itself on the exact-match path', () => {
|
||||
const allSpy = vi.spyOn(expressAdapter.getInstance(), 'all');
|
||||
const useSpy = vi.spyOn(expressAdapter.getInstance(), 'use');
|
||||
const middleware = vi.fn();
|
||||
const next = vi.fn();
|
||||
|
||||
const registerMiddleware =
|
||||
expressAdapter.createMiddlewareFactory(NO_REQUEST_METHOD);
|
||||
registerMiddleware('/api$', middleware);
|
||||
registerMiddleware('/api/{*path}', middleware);
|
||||
const [, exactHandler] = allSpy.mock.calls[0] as unknown as [
|
||||
string,
|
||||
Function,
|
||||
];
|
||||
const [, wildcardHandler] = useSpy.mock.calls[0] as unknown as [
|
||||
string,
|
||||
Function,
|
||||
];
|
||||
expressAdapter.createMiddlewareFactory(NO_REQUEST_METHOD)(
|
||||
'/api$',
|
||||
middleware,
|
||||
);
|
||||
|
||||
// Both "/api" (not strict) and "/api/{*path}" match "/api/", while
|
||||
// "/api/*path" would not
|
||||
const req = { path: '/api/' };
|
||||
exactHandler(req, {}, next);
|
||||
expect(middleware).toHaveBeenCalledExactlyOnceWith(req, {}, next);
|
||||
|
||||
wildcardHandler(req, {}, next);
|
||||
expect(middleware).toHaveBeenCalledOnce();
|
||||
expect(next).toHaveBeenCalledOnce();
|
||||
|
||||
const anotherReq = { path: '/api/' };
|
||||
wildcardHandler(anotherReq, {}, next);
|
||||
expect(middleware).toHaveBeenLastCalledWith(anotherReq, {}, next);
|
||||
// "/api" also matches "/api/", which "/api/*path" would not
|
||||
expect(allSpy).toHaveBeenCalledExactlyOnceWith('/api', middleware);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in new issue
Block a user