Skip to content

Commit

Permalink
update: fix async-app app chaining when mounting sub app
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
micaelbergeron committed Feb 7, 2024
1 parent 9228630 commit cd0a7c7
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 6 deletions.
26 changes: 26 additions & 0 deletions features/advanced.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
"""
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down
14 changes: 11 additions & 3 deletions src/async.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 18 additions & 2 deletions src/examples/advanced/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
24 changes: 23 additions & 1 deletion src/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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') || {};
Expand Down Expand Up @@ -73,5 +92,8 @@ pickledCucumber(setup, {
},
entities,
http: httpSupertest(supertest(advancedApp) as any),
initialContext: () => ({
$app: advancedApp,
}),
usage: true,
});

0 comments on commit cd0a7c7

Please sign in to comment.