koajs / joi-router

Configurable, input and output validated routing for koa
MIT License
450 stars 96 forks source link

How to use this middleware with koa-bodyparser #31

Closed momsse closed 5 years ago

momsse commented 8 years ago

Hello and thank you all for the work done on this project.

I'm trying to integrate this middleware into a project that already uses koa-bodyparser (required middleware to koa-passport).

Unfortunately when trying to reach a route when this middleware is declared the road never answers. I guess there is a conflict with co-body somewhere.

Do we have a workaround for using this middleware with koa-bodyparser or am i going wrong ?

const koa = require('koa');
const bodyParser = require('koa-bodyparser');
const router = require('koa-joi-router');
const Joi = router.Joi;

const app = koa();
const helloRouter = router();

app.use(bodyParser()); // <-- comment and the call will work

helloRouter.route({
  method: 'post',
  path: '/hello',
  validate: {
    body: {
      name: Joi.string().required()
    },
    type: 'json'
  },
  handler: function* hello() {
    const { name } = this.request.body;
    this.body = `Hello ${name}`;
  }
});

app.use(helloRouter.middleware());

app.listen(3000);

The call : curl -X POST -H "Content-Type: application/json" -d '{ "name": "John" }' "http://localhost:3000/hello"

aheckmann commented 8 years ago

Do you get an http status 400 back otherwise I'm guessing co-body doesn't know how to handle this situation and hangs. If so, please try completely removing the 'type' property and let me know how it goes.

On Wed, Oct 5, 2016, 12:12 PM momsse notifications@github.com wrote:

Hello and thank you all for the work done on this project.

I'm trying to integrate this middleware into a project that already uses koa-bodyparser https://github.com/koajs/bodyparser (required middleware to koa-passport https://www.npmjs.com/package/koa-passport).

Unfortunately when trying to reach a route when this middleware is declared the road never answers. I guess there is a conflict with co-body somewhere.

Do we have a workaround for using this middleware with koa-bodyparser or am i going wrong ?

const koa = require('koa');const bodyParser = require('koa-bodyparser');const router = require('koa-joi-router');const Joi = router.Joi; const app = koa();const helloRouter = router(); app.use(bodyParser()); // <-- comment and the call will work helloRouter.route({ method: 'post', path: '/hello', validate: { body: { name: Joi.string().required() }, type: 'json' }, handler: function* hello() { const { name } = this.request.body; this.body = Hello ${name}; } }); app.use(helloRouter.middleware()); app.listen(3000);

The call : curl -X POST -H "Content-Type: application/json" -d '{ "name": "John" }' "http://localhost:3000/hello"

β€” You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/koajs/joi-router/issues/31, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKLsuVPVd1-XnJnFRfhoWpPtUHu-TDFks5qw_aJgaJpZM4KPLXB .

momsse commented 8 years ago

Thanks for your fast answer.

The request just hang indefinitely :/ Here the response just after removing the type from the validate object :

AssertionError: validate.type must be declared when using validate.body
    at checkValidators (/Users/momsse/Developer/tech-watch-manager/node_modules/koa-joi-router/joi-router.js:215:5)
    at Router.validateRouteSpec [as _validateRouteSpec] (/Users/momsse/Developer/tech-watch-manager/node_modules/koa-joi-router/joi-router.js:151:3)
    at Router.addRoute [as _addRoute] (/Users/momsse/Developer/tech-watch-manager/node_modules/koa-joi-router/joi-router.js:111:8)
    at Router.route (/Users/momsse/Developer/tech-watch-manager/node_modules/koa-joi-router/joi-router.js:96:10)
    at Object.<anonymous> (/Users/momsse/Developer/tech-watch-manager/demo.js:15:13)
    at Module._compile (module.js:556:32)
    at Object.Module._extensions..js (module.js:565:10)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)
aheckmann commented 8 years ago

Looks like there aren't any work-arounds short of disabling input validation at this time. https://github.com/koajs/joi-router#handling-non-validated-input

It shouldn't be too hard to separate body parsing from validation a little cleaner but it would mean allowing potentially misconfigured routers which will not fail until runtime. It means more flexibility at cost of more production issues. Maybe we dump a debug message when we encounter this at start up and NODE_ENV!=production

On Wed, Oct 5, 2016 at 12:48 PM momsse notifications@github.com wrote:

Thanks for your fast answer.

The request just hang indefinitely :/ Here the response just after removing the type from the validate object :

AssertionError: validate.type must be declared when using validate.body at checkValidators (/Users/momsse/Developer/tech-watch-manager/node_modules/koa-joi-router/joi-router.js:215:5) at Router.validateRouteSpec as _validateRouteSpec at Router.addRoute as _addRoute at Router.route (/Users/momsse/Developer/tech-watch-manager/node_modules/koa-joi-router/joi-router.js:96:10) at Object. (/Users/momsse/Developer/tech-watch-manager/demo.js:15:13) at Module._compile (module.js:556:32) at Object.Module._extensions..js (module.js:565:10) at Module.load (module.js:473:32) at tryModuleLoad (module.js:432:12) at Function.Module._load (module.js:424:3)

β€” You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/koajs/joi-router/issues/31#issuecomment-251779347, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKLsmbJL-4dRTx03zincrTcD3vk79ltks5qw_8CgaJpZM4KPLXB .

momsse commented 8 years ago

πŸ‘ for the debug message to have some understanding of what is going on.

For now i'm using passport and koa-bodyparser middlewares after joi-router ones to make things works.

lironess commented 7 years ago

Can you please provide sample code using koa-joi-router and passport that works? I'm getting the same error too

momsse commented 7 years ago

@lironess the repo is public => https://github.com/momsse/tech-watch-manager/blob/master/server/config/routes/auth.router.js

lironess commented 7 years ago

I see you used both routers.. is there any kind of workaround to use only one?

momsse commented 7 years ago

Since we cannot use body-parser with koa-joi-router at this time i have not found any other solution than use koa-router + body-parser for passeport :/

yelworc commented 6 years ago

Just stumbled upon this as well; my use case: validating a signed POST request (a Twilio webhook call) before invoking the request handler function.

Couldn't koa-joi-router simply skip the body parsing step in case ctx.request.body already exists? Just a thought, I'm probably missing something here.

hilkeheremans commented 6 years ago

I'm using koa-body and had the same issue. Following @yelworc's suggestion, I forked the repo and skipped the body parsing step should it already be parsed.

Seems to work fine with validation still intact, although I should note that limits on body size (maxBody option) should no longer be configured in joi-router at that point, but rather in whatever body parser you use.

See also https://github.com/hilkeheremans/joi-router or npm @thinman/koa-joi-router

Holding off on a PR until one of the contributes chimes in with an opinion.

ziyofun commented 6 years ago

I find that the body already parsed can not emit any listener in readStream function of raw-body, so there may be some issue with raw-body

AlexHayton commented 6 years ago

I second the need for @hilkeheremans' body parser skip to become part of the official joi-router codebase. This really threw me when I used the project and it's a frustrating developer experience.

It also prevents you from adding more middleware in the routes that read things from the body.

@hilkeheremans please can you create a PR? If I don't hear back I'll make one myself and credit you.

hilkeheremans commented 6 years ago

Alex,

On holiday so can’t PR atm, feel free to go ahead!

On 1 Jun 2018, 07:00 +0100, Alex Hayton notifications@github.com, wrote:

I second the need for @hilkeheremanshttps://github.com/hilkeheremans' body parser skip to become part of the official joi-router codebase. This really threw me when I used the project.

It also prevents you from adding more middleware in the routes that read things from the body.

@hilkeheremanshttps://github.com/hilkeheremans please can you create a PR? If I don't hear back I'll make one myself and credit you.

β€” You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/koajs/joi-router/issues/31#issuecomment-393769234, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AJ32YKfanWLz9vdIcbCvo7Fz7Y88JsGoks5t4NiMgaJpZM4KPLXB.

pke commented 6 years ago

Maybe this lib should not even depend on a body parser at all but rather integrate into whatever is available upstream in middleware. So it could just take body as being parsed already and validate it.

yelworc commented 5 years ago

I guess with PR #76 this can be closed.

aheckmann commented 5 years ago

closing per #76