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

Remove mixinParams function and replace with req.param(...) #1

Closed
bminer opened this issue Oct 20, 2011 · 5 comments
Closed

Remove mixinParams function and replace with req.param(...) #1

bminer opened this issue Oct 20, 2011 · 5 comments

Comments

@bminer
Copy link

bminer commented Oct 20, 2011

The mixinParams function is not necessary because Express 2.0 already provides support for accessing route params, query strings, and HTTP bodies with req.param(...)

See http://expressjs.com/guide.html#req.param() for more information

This should clean up the code a bit. :)

@ctavan
Copy link
Member

ctavan commented Oct 20, 2011

Hey bminer, thanks for your feedback!

For validation I agree, we should be able to use req.param(). For sanitization however things will become a bit more tricky since we actually want to modify the parameters and req.param() is read-only. So we somehow have to figure out, whether the param we want to sanitize is in req.params, req.query or req.body and write it back to the correct location...

Anyways I agree we should get rid of the mixinParams-method. If you want, you can provide me with a pull-request, otherwise I'll see if I can implement it during the weekend.

@bminer
Copy link
Author

bminer commented Oct 20, 2011

Good point. I suppose you could just store back into req.param no matter
what, which is sort of what's happening right now. Or, you could do
something like req.param(...) -- first try replacing req.params[x] if it
exists, otherwise replace req.query[x], then req.body[x]... in that order.
The latter seems like a better approach since you can still access your
vars (sanitized or not) through req.body, req.params, and req.query, but you
can also access them via req.param(...).

I'm not too great with Git, and my time is extremely limited right now. I
could take care of this, but I'd probably just email you the code (I know...
gross). Let me know what you want me to do. If you can get to it this
weekend or whatever, that's fine with me, too. Thanks!

On Thu, Oct 20, 2011 at 8:50 AM, Christoph Tavan <
[email protected]>wrote:

Hey bminer, thanks for your feedback!

For validation I agree, we should be able to use req.param(). For
sanitization however things will become a bit more tricky since we actually
want to modify the parameters and req.param() is read-only. So we somehow
have to figure out, whether the param we want to sanitize is in req.params,
req.query or req.body and write it back to the correct location...

Anyways I agree we should get rid of the mixinParams-method. If you want,
you can provide me with a pull-request, otherwise I'll see if I can
implement it during the weekend.

Reply to this email directly or view it on GitHub:
#1 (comment)

@ctavan
Copy link
Member

ctavan commented Oct 20, 2011

If you can wait until the end of the weekend, I'll implement your idea.

ctavan added a commit that referenced this issue Oct 20, 2011
…d req.body. req.check now no longer relies on req.mixinParams(), see #1
ctavan added a commit that referenced this issue Oct 20, 2011
…) as well.

Introduce new updateParam() method to replace params with the filtered version
See #1
@ctavan ctavan closed this as completed in fbd6461 Oct 20, 2011
@bminer
Copy link
Author

bminer commented Oct 28, 2011

Thank you very much! Awesome

rustybailey pushed a commit that referenced this issue Feb 5, 2016
Change lodash method names to match 4.x changes
rustybailey pushed a commit that referenced this issue Mar 13, 2016
Support for defining location of validation for each schema param.
@lock
Copy link

lock bot commented Jun 2, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants