Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: prevent multiple route executions per request #568

Merged
merged 2 commits into from
Jul 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions src/driver/express/ExpressDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,20 +160,28 @@ export class ExpressDriver extends BaseDriver {
// prepare route and route handler function
const route = ActionMetadata.appendBaseRoute(this.routePrefix, actionMetadata.fullRoute);
const routeHandler = function routeHandler(request: any, response: any, next: Function) {
// Express calls the "get" route automatically when we call the "head" route:
// Reference: https://expressjs.com/en/4x/api.html#router.METHOD
// This causes a double action execution on our side, which results in an unhandled rejection,
// saying: "Can't set headers after they are sent".
// The following line skips action processing when the request method does not match the action method.
if (actionMetadata.type !== "all" && request.method.toLowerCase() !== actionMetadata.type)
return next();

return executeCallback({request, response, next});
};

// This ensures that a request is only processed once to prevent unhandled rejections saying
// "Can't set headers after they are sent"
// Some examples of reasons a request may cause multiple route calls:
// * Express calls the "get" route automatically when we call the "head" route:
// Reference: https://expressjs.com/en/4x/api.html#router.METHOD
// This causes a double execution on our side.
// * Multiple routes match the request (e.g. GET /users/me matches both @All(/users/me) and @Get(/users/:id)).
// The following middleware only starts an action processing if the request has not been processed before.
const routeGuard = function routeGuard(request: any, response: any, next: Function) {
if (!request.routingControllersStarted) {
request.routingControllersStarted = true;
return next();
}
};

// finally register action in express
this.express[actionMetadata.type.toLowerCase()](...[
route,
routeGuard,
...beforeMiddlewares,
...defaultMiddlewares,
routeHandler,
Expand Down
12 changes: 12 additions & 0 deletions src/driver/koa/KoaDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,21 @@ export class KoaDriver extends BaseDriver {
return executeCallback(options);
};

// This ensures that a request is only processed once. Multiple routes may match a request
// e.g. GET /users/me matches both @All(/users/me) and @Get(/users/:id)), only the first matching route should
// be called.
// The following middleware only starts an action processing if the request has not been processed before.
const routeGuard = (context: any, next: () => Promise<any>) => {
if (!context.request.routingControllersStarted) {
context.request.routingControllersStarted = true;
return next();
}
};

// finally register action in koa
this.router[actionMetadata.type.toLowerCase()](...[
route,
routeGuard,
...beforeMiddlewares,
...defaultMiddlewares,
routeHandler,
Expand Down