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

Allowed methods broken in 5.2.0, 5.2.1 #182

Closed
MarkHerhold opened this issue Oct 1, 2015 · 3 comments
Closed

Allowed methods broken in 5.2.0, 5.2.1 #182

MarkHerhold opened this issue Oct 1, 2015 · 3 comments

Comments

@MarkHerhold
Copy link

I have an app that has "private" and "public" routers. When a request arrives at a route defined on the public router, but using a method (e.g. DELETE) defined on the router, it falls through to the private router in 5.2.0, 5.2.1. Versions 5.0.1, 5.1.0, and 5.1.1 exhibit the expected behavior.

Environment:
Ubuntu 15.04
node.js 4.0.0
koa 1.0.0

Expected:
DELETE http://localhost:3000/v1/public should return a 405 Method Not Allowed.
Working in version 5.0.1. Broken in 5.2.0 and 5.2.1.

Actual:
DELETE http://localhost:3000/v1/public returns a 401 Unauthorized 👎 because it falls through to the private router despite using app.use(publicRouter.allowedMethods()).

Reproduce:
To reproduce, run this with the versions mentioned above.

'use strict';
var koa = require('koa');
var Router = require('koa-router');

// working with koa-router 5.0.1
// broken with koa-router 5.1.2

var app = koa();

// init
var publicRouter = new Router({ prefix: '/v1' });
var privateRouter = new Router({ prefix: '/v1' });

// PUBLIC
publicRouter.get('/public', function* () {
    this.status = 200;
    this.body = 'Hi from /public!'
})

app.use(publicRouter.routes());
app.use(publicRouter.allowedMethods());

// PRIVATE
privateRouter.use(function* () {
    this.status = 401;
    // don't yield to next, we are blocking the user
});

privateRouter.get('/protected', function* () {
    this.status = 200;
    this.body = 'Hi from /protected!'
})

app.use(privateRouter.routes());
app.use(privateRouter.allowedMethods());

app.listen(3000);

If I am doing something incorrectly or if this is working as designed, please let me know. Thanks!

@MarkHerhold
Copy link
Author

@alexmingoia Many thanks!

ruiquelhas pushed a commit to seegno-forks/koa-router that referenced this issue Oct 16, 2015
@rockie
Copy link

rockie commented Feb 3, 2016

Hi @alexmingoia Imho, this should not be an issue. The "privateRouter" above uses a middleware without methods and paths, which should be treated as a middleware mounted on root and should be dispatched for all requests. I mean this is quite reasonable .

After applying this fix, the koa-static middleware mounted on a certain path in my app with koa-router is skipped like air. That's because all middlewares without an upcoming explicit method call (e.g. router.get/router.post/router.del...) are bypassed by the .route boolean check in the fix and will no longer be dispatched. BTW, for the same reason, below example in your README will not work anymore. I just wonder which behavior better matches your original design. If I am wrong, please feel free to correct me.


router
.use(session())
.use(authorize());

// use middleware only with given path
router.use('/users', userAuth());

// or with an array of paths
router.use(['/users', '/admin'], userAuth());

app.use(router.routes());

@YanLobat
Copy link

@alexmingoia I would agree with @rockie. Removing this line(I mean condition) will help us avoiding lots of forks of koa-router

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants