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

[fix] allowedMethods will fail if two routes are defined #182

Closed
3 tasks done
lbesson-della opened this issue Aug 19, 2024 · 1 comment · Fixed by #183
Closed
3 tasks done

[fix] allowedMethods will fail if two routes are defined #182

lbesson-della opened this issue Aug 19, 2024 · 1 comment · Fixed by #183
Assignees
Labels
bug Something isn't working

Comments

@lbesson-della
Copy link
Contributor

lbesson-della commented Aug 19, 2024

Describe the bug

In commit f7ffb93

if (ctx.matched) {
  ctx.matched.push.apply(ctx.matched, matched.path);
} else {
  ctx.matched = matched.path;
}

has been changed to

if (ctx.matched) {
  ctx.matched.push(matched.path);
} else {
  ctx.matched = matched.path;
}

This causes an issue with allowedMethods if more than a router is defined

Actual behavior

TypeError: Cannot read properties of undefined (reading 'length')
    at D:\code\legisway-connector\node_modules\@koa\router\lib\router.js:330:47

Expected behavior

The code should not fail

Code to reproduce

import Router from '@koa/router';
import Koa from 'koa';

const app = new Koa();
const router1 = new Router();
const router2 = new Router();
router1.get('/foo', ctx => {
  ctx.body = 'foo';
});
router2.get('/bar', ctx => {
  ctx.body = 'bar';
});
app.use(router1.routes());
app.use(router1.allowedMethods({ throw: true }));
app.use(router2.routes());
app.use(router2.allowedMethods({ throw: true }));
app.listen(8888);

$ curl  localhost:8888

Checklist

  • I have searched through GitHub issues for similar issues.
  • I have completely read through the README and documentation.
  • I have tested my code with the latest version of Node.js and this package and confirmed it is still not working.
@lbesson-della lbesson-della added the bug Something isn't working label Aug 19, 2024
@lbesson-della lbesson-della changed the title [fix] allowedMethods will fail if two routes are f [fix] allowedMethods will fail if two routes are defined Aug 19, 2024
@3imed-jaberi
Copy link
Member

@lbesson-della great catch! Thanks for your reaction here. Always we should pass the context in this kind of code style.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants