Middleware used in a nested router is evaluated on unrelated routes.

[yamavol]

Descriptions

The middleware used in a nested-router can be triggered if the regexp matches (matchedRoute) with the unrelated path. The behavior is something intuitive or not easily predictable.

I wrote one router like this, intending to run "checkAuth" middleware when the user accesses to /a/1 or any other subroutes written under /a.

function createAjaxRouter() {
  const router = createRouter('/a');
  router.use(checkAuth);
  router.get('/1', routeHandler(1))
  return router.routes();
}

However, accessing /about (defined elsewhere) can trigger "checkAuth". This is because middleware is evaluated when the request-path matches /a(.*).

This issue can be avoided by defining the routes /about before this nested router, but manually managing the order of nested routes is difficult.

I wonder if the library can guarantee that middleware used inside the nested-routes are used only in nested routes.

Environment

node.js version: v11.6.0 (Windows)
npm/yarn and version: 6.14.5
@koa/router version: 9.0.1
koa version: 2.12.0

Code sample:

const Koa = require('koa');
const Router = require('@koa/router');

function createRouter(prefix) {
  return new Router({
    prefix
  });
}

function routeHandler(x) { 
  return async ctx => {
    console.log(x);
    ctx.body = `${x}`;
  }
}

async function checkAuth(ctx, next) {
  console.log('checkAuth');
  // if (ctx.query['auth'] != "1") {
  //   throw new Error("not authed");
  // }
  await next();
}

function createAjaxRouter() {
  // match for '/a(.*)'
  const router = createRouter('/a');
  router.use(checkAuth);
  router.get('/1', routeHandler(1))
  return router.routes();
}

function createIndexRouter() {
  // match for '(.*)'
  const router = createRouter();
  router.get('/', routeHandler('index'));
  router.get('/about', routeHandler('about'));
  return router.routes();
}

function setRouter(app) {
  // match for '(.*)'
  const router = createRouter();
  router.use(createAjaxRouter());
  router.use(createIndexRouter());

  // order of path in routes.router.stack =>
  // ['/a(.*)', '/a/1', '/', '/about']
  const routes = router.routes();
  app.use(router.routes());
  app.use(router.allowedMethods());
}

const app = new Koa();
setRouter(app);
app.listen(3010); 

Gist

Expected Behavior:

Access to /about should not run the middleware used for nested routes with prefix: /a

Actual Behavior:

Access to /about can trigger the middleware defined in the nested routes.

[aravindanve]

@yamavol Did you find a workaround for this?

Here is a full reproduction of the issue:

const Koa = require('koa');
const KoaRouter = require('@koa/router');
const supertest = require('supertest');

const routerBranch1 = new KoaRouter()
    .use((ctx, next) => {
        console.log('middleware in branch 1');
        return next();
    })
    .get('/', ctx => {
        console.log('branch 1');
        ctx.body = 'branch 1';
    });

const routerBranch2 = new KoaRouter()
    .use((ctx, next) => {
        console.log('middleware in branch 2');
        return next();
    })
    .get('/', ctx => {
        console.log('branch 2');
        ctx.body = 'branch 2';
    });

const router = new KoaRouter()
    .use('/examples', routerBranch1.routes())
    .use('/examples', routerBranch1.allowedMethods())
    .use('/examples/:exampleId/items', routerBranch2.routes())
    .use('/examples/:exampleId/items', routerBranch2.allowedMethods());
    
const app = new Koa()
    .use(router.routes())
    .use(router.allowedMethods());

const server = app.listen();
const request = supertest(server);

const run = async () => {
    await request.get('/examples');
    await request.get('/examples/1/items');
    server.close();
};

run().catch(console.error);

Prints:

middleware in branch 1
branch 1
middleware in branch 1
middleware in branch 2
branch 2

[julienw]

for me it seems to work to reverse the order of the route definition:

const router = new KoaRouter()
  .use('/examples/:exampleId/items', routerBranch2.routes())
  .use('/examples/:exampleId/items', routerBranch2.allowedMethods())
  .use('/examples', routerBranch1.routes())
  .use('/examples', routerBranch1.allowedMethods());

This gives:

middleware in branch 1
branch 1
middleware in branch 2
branch 2

Hope this helps!

[aravindanve]

@julienw I really wanted a way to not have to order them like that (or order them at all). Because I am adding routes programmatically from a schema, and this complicates things. So I ended up writing a router. Thanks nevertheless!

https://www.npmjs.com/package/koa-branch-router

[julienw]

Looks interesting, thanks for the pointer !

[scorbiclife]

Did I understand correctly that

1. When nesting routers,
   the inner routes are copied and merged into the outer router,
   making the structure of the outer router a flat list rather than a tree?

2. Item 1 is the cause of the behavior in the above example?
   (i.e. middleware in routerBranch1 running on route in routerBranch2)

3. In order to make the middleware "local" to the router,
   I should make a new router for every "local scope" like this?

const Koa = require('koa');
const KoaRouter = require('@koa/router');

const routerBranch1 = new KoaRouter()
    .use((ctx, next) => {
        console.log('middleware in branch 1');
        return next();
    })
    .get('/', ctx => {
        console.log('branch 1');
        ctx.body = 'branch 1';
    });

const routerBranch2 = new KoaRouter()
    .use((ctx, next) => {
        console.log('middleware in branch 2');
        return next();
    })
    .get('/', ctx => {
        console.log('branch 2');
        ctx.body = 'branch 2';
    });

const router1 = new KoaRouter()
    .use('/examples', routerBranch1.routes())
    .use('/examples', routerBranch1.allowedMethods())

const router2 = new KoaRouter()
    .use('/examples/:exampleId/items', routerBranch2.routes())
    .use('/examples/:exampleId/items', routerBranch2.allowedMethods());

const app = new Koa()
    .use(router1.routes())
    .use(router1.allowedMethods())
    .use(router2.routes())
    .use(router2.allowedMethods());

const server = app.listen(3000);

[julienw]

I don't understand your line 1; indeed I believe the problem happens when not creating new routers.

I tried to fix this in #44 but this was rejected. I believe that the solution when using koa-router is indeed to create routers for each context. Or use another router :-)

[scorbiclife]

I think #44 is orthogonal to this issue.

That issue is about the Router.used middleware not executing when no paths are matched.
In order to solve that one needs to use router.(get|post|...|all) instead.

This issue is about Router.used middleware on "inner router 1" executing on a match in "inner router 2", which, I suspect, is because there is no "local scope" for the inner routers. (This is what I intended with Item 1, although it doesn't seem very clear on second read)

Still, thank you for your input.

[pixelvm]

I think it is this ...

const Koa = require('koa');
const Router = require('@koa/router');

const app = new Koa();

app.use(async (ctx, next) => {
  console.log(ctx.url);
  await next();
});

async function m1 (ctx, next) {
  ctx.body = ctx.body ?? [ctx.url];
  ctx.body.push('before m1');
  await next();
  ctx.body.push('afrer m1');
}

async function m2 (ctx, next) {
  ctx.body = ctx.body ?? [ctx.url];
  ctx.body.push('before m2');
  await next();
  ctx.body.push('before m2');
}

async function rfunc (ctx) {
  ctx.body = ctx.body ?? [ctx.url];
  ctx.body.push('get ' + ctx.url);
}

// tow Router objects
const r1 = new Router({ prefix: '/r1' });
r1.use(m1);
r1.get('/', rfunc);

const r1s1 = new Router({ prefix: '/r1/s1' });
r1s1.use(m1);
r1s1.use(m2);
r1s1.get('/', rfunc);

// code 1
// get /r1/s1 , m1 is called twice; result is:
// [
//   "/r1/s1",
//   "before m1",
//   "before m1",
//   "before m2",
//   "get /r1/s1",
//   "afrer m2",
//   "afrer m1",
//   "afrer m1"
// ]
// const router = new Router();
// router.use(r1.routes()).use(r1s1.routes());
// app.use(router.routes());

// code 2
// get /r1/s1 , m1 is called once; result is:
// [
//   "/r1/s1",
//   "before m1",
//   "before m2",
//   "get /r1/s1",
//   "afrer m2",
//   "afrer m1"
// ]
app.use(r1.routes()).use(r1s1.routes());

app.listen(8080);