From cd0a7c7e1e202cfb5d5f27c5f6b10333abd10168 Mon Sep 17 00:00:00 2001
From: Micael Bergeron <mbergeron@mural.co>
Date: Wed, 7 Feb 2024 09:00:46 -0500
Subject: [PATCH] update: fix async-app app chaining when mounting sub app

When mounting a sub app with `app.use(path, subApp)`, express does a
check to see if an app was sent through, or if it was a middleware.

In async-app, we are converting all MiddlewareArguments which confuses
express to think that the mounted app isn't an sub application, but a
simple middleware.

Thus, express doesn't fire the `mount` event, which in turns skips a
bunch of express routines:

 - sets the `mountpath` on the subApp
 - copy some settings from the parent app (for instance "trust proxy")

With this change, we can now use `req.app.path` to get the full route
the request was matched for, and `req.ip` will be properly propagated
through.
---
 features/advanced.feature    | 26 ++++++++++++++++++++++++++
 package.json                 |  2 ++
 src/async.ts                 | 14 +++++++++++---
 src/examples/advanced/app.ts | 20 ++++++++++++++++++--
 src/test.ts                  | 24 +++++++++++++++++++++++-
 5 files changed, 80 insertions(+), 6 deletions(-)

diff --git a/features/advanced.feature b/features/advanced.feature
index 83bba7c..682f982 100644
--- a/features/advanced.feature
+++ b/features/advanced.feature
@@ -137,3 +137,29 @@ Scenario: deprecated get todos
   Then the response is 302
   And the response headers at deprecated-for is "GET /todos/${U.username}"
   And the response headers at location contains "/todos/${U.username}"
+
+Scenario: mounted apps (nest level 1)
+  Given app setting "trust proxy" is enabled
+  Given the request header X-Forwarded-For is "1.1.1.1"
+  When GET /sub/test
+  Then the response is 200
+  And the response payload includes
+  """
+  {
+    "route": "/sub/test",
+    "ip": "1.1.1.1"
+  }
+  """
+
+Scenario: mounted apps (nest level 2)
+  Given app setting "trust proxy" is enabled
+  Given the request header X-Forwarded-For is "10.0.1.3"
+  When GET /sub/sub/test
+  Then the response is 200
+  And the response payload includes
+  """
+  {
+    "route": "/sub/sub/test",
+    "ip": "10.0.1.3"
+  }
+  """
diff --git a/package.json b/package.json
index d068fdb..033b8be 100644
--- a/package.json
+++ b/package.json
@@ -44,7 +44,9 @@
     "build": "npm run lint && tsc",
     "dist": "rm -rf dist && npm run build && npm run test && cp package*.json README.md LICENSE dist && rm -rf ./dist/test.* ./dist/examples",
     "lint": "tslint -p .",
+    "lint:fix": "tslint -p . --fix",
     "pack": "npm run dist && cd dist && npm pack",
+    "test:debug": "node --inspect-brk=0.0.0.0:9429 --no-lazy ./node_modules/.bin/cucumber-js --require-module ts-node/register --exit -r src/test.ts",
     "test": "cucumber-js --require-module ts-node/register --exit -r src/test.ts",
     "watch": "tsc -w"
   }
diff --git a/src/async.ts b/src/async.ts
index 25102ef..c5310d7 100644
--- a/src/async.ts
+++ b/src/async.ts
@@ -107,9 +107,17 @@ const mapMiddleware = <TEntities extends Entities>(
   return fn;
 };
 
-export default <TEntities extends Entities, TSchema>(
-  { errorHandler, compileSchema }: AsyncOptions<TEntities, TSchema> = {},
-): Converter<TEntities, TSchema> => (args, context) => {
+export default <TEntities extends Entities, TSchema>({
+  errorHandler,
+  compileSchema,
+}: AsyncOptions<TEntities, TSchema> = {}): Converter<TEntities, TSchema> => (
+  args,
+  context,
+) => {
+  if (context.method === 'use') {
+    return args;
+  }
+
   const statusCode = args.find(isNumber) || DEFAULT_STATUS_CODE;
 
   const schema = args
diff --git a/src/examples/advanced/app.ts b/src/examples/advanced/app.ts
index 0e40df0..8dc7edf 100644
--- a/src/examples/advanced/app.ts
+++ b/src/examples/advanced/app.ts
@@ -7,8 +7,8 @@ import bodyParser from 'body-parser';
 import express from 'express';
 import { join } from 'path';
 
-import { deprecate } from '../..';
-import createApp, { Req } from './async-app';
+import { App, deprecate } from '../..';
+import createApp, { ExampleEntities, Req } from './async-app';
 import can from './can';
 import { addTodo, addUser, getTodosForUser } from './db';
 import load from './load';
@@ -112,4 +112,20 @@ app.use(
   express.static(join(__dirname, '../../../src/examples/advanced/docs')),
 );
 
+// --- Mounted sub app --- //
+const mountSubApp = (app: App<ExampleEntities, {}>) => {
+  const subApp = createApp();
+  app.use('/sub', subApp);
+
+  subApp.get('/test', 'Mounted app route information', (req: Req) => ({
+    ip: req.ip,
+    route: join(req.app.path(), req.path),
+  }));
+
+  return subApp;
+};
+
+const sub = mountSubApp(app);
+mountSubApp(sub);
+
 export default app;
diff --git a/src/test.ts b/src/test.ts
index cdfd2f9..cb8a96e 100644
--- a/src/test.ts
+++ b/src/test.ts
@@ -8,12 +8,31 @@ import { entities } from './examples/advanced/test';
 import orderMiddlewares from './order';
 import { Decorator, Middleware } from './types';
 
+type App = typeof advancedApp;
 type Mid = Middleware<any> & { $name?: string };
 interface MidMap {
   [name: string]: Mid;
 }
 
-const setup: SetupFn = ({ compare, getCtx, Given, setCtx, Then, When }) => {
+const setup: SetupFn = ({
+  compare,
+  getCtx,
+  Given,
+  setCtx,
+  Then,
+  When,
+  onTearDown,
+}) => {
+  // === App setup === //
+  Given('app setting "(.*)" is enabled', (setting) => {
+    const app = getCtx<App>('$app');
+    app.enable(setting);
+
+    onTearDown(() => {
+      app.disable(setting);
+    });
+  });
+
   // === Order ============================================================== //
   const addMiddleware = (name: string, attrs: Decorator) => {
     const ms = getCtx<MidMap>('$middlewares') || {};
@@ -73,5 +92,8 @@ pickledCucumber(setup, {
   },
   entities,
   http: httpSupertest(supertest(advancedApp) as any),
+  initialContext: () => ({
+    $app: advancedApp,
+  }),
   usage: true,
 });