Skip to content
This repository

Doesn't Seem To Differentiate Post vs Get vs URL #3

Closed
datarem opened this Issue December 06, 2011 · 9 comments

2 participants

Remy Christoph Tavan
Remy

If I pass post and get parameters to an app.post and try and assert values it uses the get parameters.

curl -d 'password=1234567' http://localhost:3000/backend/check/?password=fjf
Password can't be shorter than 6 characters and it throws an exception.

Maybe use a similar syntax to req.param('password', req.body) with req.assert('password)?

Christoph Tavan
Owner

I agree, if req.param() provided a way to specify from which source to extract the parameters then req.assert() should be able to handle that as well. However I can't really find the place where you got the syntax req.param('password', req.body) from. To my knowledge in express.js url-parameters take precedence over get-parameters which take precedence over post-parameters if one uses req.param(), see https://github.com/visionmedia/express/blob/master/lib/request.js#L244-258

If you do not expect any get-parameters you could use

req.query = null;

as a workaround to unset all query-parameters before doing the asserts.

So currently req.assert() somehow just behaves like the express.js-provided req.param().

Any thoughts?

Remy

Yeah sorry my bad, misunderstood what I was looking at. (I guess I was trying to be optimistic.)
I've been using something similar to req.query.email = req.params.email = req.body.email; as a workaround, but it seems that req.assert() could work in a way where you pass it req.body, req.params, req.query or post, get, route and it uses that.

To me req.param() seems like a rather large security hole in it's current form for unsuspecting programmers. Ask for variables via post have them given to req.param via get. But that is just me.

Christoph Tavan
Owner

Yeah, I admit req.param() may be a little confusing, but as express-validator is a middleware for express.js I want to stick with the way, express.js behaves.

You can still use plain node-validator to validate just the bits that you want so you don't get confused by strange express.js magic ;)

Maybe you might consider filing an issue for express.js that addresses your concerns?

Christoph Tavan
Owner

Another idea would be to add something like

req.query.assert('email')
req.body.assert('email')
etc.

But I'm not sure how useful that would be if the application later still can use req.param('email')....

Remy

Yeah I may go to just regular node-validator and sadly drop this, but I do understand sticking with the way expressjs handles things.

Posted here visionmedia/express#622

Christoph Tavan
Owner

Cool, thanks for the upstream issue. Closing this one.

Christoph Tavan ctavan closed this December 07, 2011
Christoph Tavan ctavan reopened this December 07, 2011
Christoph Tavan
Owner

Hmm, after I've read the issue I reopen the issue here. I'll think about some way to handle that stuff nicer in express-validator.

Remy

The only thing I can think of is having it where you either pass the req.body etc. or just saying post, route, or get as a second parameter. If that is the case I can just req.body.example afterwards to get my sanitized or asserted variable. Personally my favorite thing is the easy error handling with assertions, because with my way I could basically just use node-validator I just don't get the nice assertions.

Christoph Tavan
Owner
ctavan commented June 06, 2013

#32 provides a special method checkBody for checking the body only, similar methods could be added for checkQuery and checkParams. If anyone has the need for these feel free to open a pull request for it.

Christoph Tavan ctavan closed this June 06, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.