-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
The methodOverride middleware should also look for _method in req.query #169
Comments
hmm yeah not sure. This is the great joy of async :) everything is a PITA |
Yeah, but of all the flavors of pain in the world, this is the one we've chosen out of free will :) This is the only real use I have for the methodOverride middleware, so I'd be terribly sad to not see it implemented... But I can continue to use my own poor man's version, of course. |
I dont personally really see it as much of a security issue, because you could easily fashion a hack with the other things methodOverride support, people should never really assume any request is trust worthy so I would probably be fine with this, might be good to have some more input |
Also keep in mind that connect and express are very modular and you can easily make a custom middleware for your specific use case. |
@creationix: Yeah, that's what I've done, and it works great. I chose to file the ticket anyway because file uploads in browsers that don' t support http://www.w3.org/TR/FileAPI/ seemed like the only real use case for a method override middleware since it'll always be a POST request with Content-Type: multipart/form-data no matter what you do. In all other cases you have the option to use an XHR instead. @visionmedia: I agree, nothing is to be trusted, but there's a big difference between how much work an attacker has to do to make someone's browser do a POST vs. a GET request, so at least you should think twice before generically allowing any GET request to rewrite itself to eg. DELETE. Would be unfortunate if I could make you delete your stuff by getting you to view a forum post with an inline image pointing to http://visionmedia.ca/stuff?_method=DELETE while you were logged in. |
haha yeah, well like you said, you would have to omit GETs |
closing |
BTW, I made up a standalone module to do this if anybody else finds the need: https://github.com/nfriedly/connect-method-override-get |
I use this functionality too, would a pull request to implement this be accepted? It could be added with an option to turn it off (and perhaps even have it off by default). |
Not something we'd want to advocate. Plus, @nfriedly already made a module for it. |
I would like to route certain POST multipart/form-data file uploads to a PUT handler in my Express app. Since bodyDecoder doesn't (and shouldn't) populate req.body with the parameters in the incoming form, there's no way to tell the methodOverride middleware do its magic.
Since I cannot set the X-HTTP-Method-Override HTTP header either the only sane place to put the method in the query string. So in other words I want this to work:
POST /foo?_method=PUT
I know there would be severe security implications if a simple GET request could be translated into any other method, but maybe translating POST to anything else could be allowed?
Best regards,
Papandreou
The text was updated successfully, but these errors were encountered: