From eff8f156d15ab8bed6a042c901dd2bc8f38892c5 Mon Sep 17 00:00:00 2001 From: luddwichr Date: Fri, 14 Feb 2025 22:25:16 +0100 Subject: [PATCH 1/3] fix(platform-express) respect existing parser middlewares when using Express 5 Express 5 made the router public API again and renamed the field from app._router to app.router. This broke the detection mechanism whether a middleware named "jsonParser" or "urlencodedParser" is already registered or not. Unfortunately, https://github.com/nestjs/nest/pull/14574/ only fixed the issue partially. This commit now uses app.router everywhere. To avoid future regressions a test was added to verify the expected behavior. --- .../adapters/express-adapter.ts | 2 +- .../test/adapters/express-adapter.spec.ts | 46 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 packages/platform-express/test/adapters/express-adapter.spec.ts diff --git a/packages/platform-express/adapters/express-adapter.ts b/packages/platform-express/adapters/express-adapter.ts index 978746e47..5c5cf4f3d 100644 --- a/packages/platform-express/adapters/express-adapter.ts +++ b/packages/platform-express/adapters/express-adapter.ts @@ -470,7 +470,7 @@ export class ExpressAdapter extends AbstractHttpAdapter< private isMiddlewareApplied(name: string): boolean { const app = this.getInstance(); return ( - !!app._router && + !!app.router && !!app.router.stack && isFunction(app.router.stack.filter) && app.router.stack.some( diff --git a/packages/platform-express/test/adapters/express-adapter.spec.ts b/packages/platform-express/test/adapters/express-adapter.spec.ts new file mode 100644 index 000000000..35eba319c --- /dev/null +++ b/packages/platform-express/test/adapters/express-adapter.spec.ts @@ -0,0 +1,46 @@ +import { ExpressAdapter } from '@nestjs/platform-express'; +import { expect } from 'chai'; +import * as express from 'express'; +import * as sinon from 'sinon'; + +afterEach(() => sinon.restore()); + +describe.only('ExpressAdapter', () => { + describe('registerParserMiddleware', () => { + it('should register the express built-in parsers for json and urlencoded payloads', () => { + const expressInstance = express(); + const jsonParserInstance = express.json(); + const urlencodedInstance = express.urlencoded(); + const jsonParserSpy = sinon + .stub(express, 'json') + .returns(jsonParserInstance); + const urlencodedParserSpy = sinon + .stub(express, 'urlencoded') + .returns(urlencodedInstance); + const useSpy = sinon.spy(expressInstance, 'use'); + const expressAdapter = new ExpressAdapter(expressInstance); + + expressAdapter.registerParserMiddleware(); + + expect(useSpy.calledTwice).to.be.true; + expect(useSpy.calledWith(sinon.match.same(jsonParserInstance))).to.be + .true; + expect(useSpy.calledWith(sinon.match.same(urlencodedInstance))).to.be + .true; + expect(jsonParserSpy.calledOnceWith({})).to.be.true; + expect(urlencodedParserSpy.calledOnceWith({ extended: true })).to.be.true; + }); + + it('should not register default parsers if custom parsers have already been registered', () => { + const expressInstance = express(); + expressInstance.use(function jsonParser() {}); + expressInstance.use(function urlencodedParser() {}); + const useSpy = sinon.spy(expressInstance, 'use'); + const expressAdapter = new ExpressAdapter(expressInstance); + + expressAdapter.registerParserMiddleware(); + + expect(useSpy.called).to.be.false; + }); + }); +}); From 123f653d0a286781c56517b98745cf8813bff5c1 Mon Sep 17 00:00:00 2001 From: luddwichr Date: Fri, 14 Feb 2025 22:36:36 +0100 Subject: [PATCH 2/3] fix(platform-express) respect existing parser middlewares when using Express 5 Express 5 made the router public API again and renamed the field from app._router to app.router. This broke the detection mechanism whether a middleware named "jsonParser" or "urlencodedParser" is already registered or not. Unfortunately, https://github.com/nestjs/nest/pull/14574/ only fixed the issue partially. This commit now uses app.router everywhere. To avoid future regressions a test was added to verify the expected behavior. --- packages/platform-express/test/adapters/express-adapter.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/platform-express/test/adapters/express-adapter.spec.ts b/packages/platform-express/test/adapters/express-adapter.spec.ts index 35eba319c..30059f85c 100644 --- a/packages/platform-express/test/adapters/express-adapter.spec.ts +++ b/packages/platform-express/test/adapters/express-adapter.spec.ts @@ -5,7 +5,7 @@ import * as sinon from 'sinon'; afterEach(() => sinon.restore()); -describe.only('ExpressAdapter', () => { +describe('ExpressAdapter', () => { describe('registerParserMiddleware', () => { it('should register the express built-in parsers for json and urlencoded payloads', () => { const expressInstance = express(); From 224cffd44247dd2d559503127e1b8dda2373ccd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20My=C5=9Bliwiec?= Date: Mon, 17 Feb 2025 10:34:38 +0100 Subject: [PATCH 3/3] test: move after each to describe block --- .../platform-express/test/adapters/express-adapter.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/platform-express/test/adapters/express-adapter.spec.ts b/packages/platform-express/test/adapters/express-adapter.spec.ts index 30059f85c..8970e117f 100644 --- a/packages/platform-express/test/adapters/express-adapter.spec.ts +++ b/packages/platform-express/test/adapters/express-adapter.spec.ts @@ -3,9 +3,9 @@ import { expect } from 'chai'; import * as express from 'express'; import * as sinon from 'sinon'; -afterEach(() => sinon.restore()); - describe('ExpressAdapter', () => { + afterEach(() => sinon.restore()); + describe('registerParserMiddleware', () => { it('should register the express built-in parsers for json and urlencoded payloads', () => { const expressInstance = express();