4 ms·
Does the code fixing this feel a little too innocuous to other people? Reading the code it seems really unlikely I'd see this and realize that deleting it would
by MaxGabriel 8y ago
Does the code fixing this feel a little too innocuous to other people? Reading the code it seems really unlikely I'd see this and realize that deleting it would create a severe security vulnerability:
v = v.select do |format|
format.symbol || format.ref == "*/*"
end
https://github.com/rails/rails/blob/efb706daad0e2e1039c6abb4879c837ef8bf4d10/actionpack/lib/action_dispatch/http/mime_negotiation.rb#L83-L85 https://github.com/rails/rails/blob/efb706daad0e2e1039c6abb4...
- tptacek 8y agoWhy would you randomly remove code from ActionView? It's some of the least user-serviceable code in Rails.
- tenderlove 8y agoWe're working to improve it. It's actually why jhawthorn found this issue.
- tenderlove 8y agoYa, that's one reason I rewrite the commit message to add the CVE. Hopefully people will view the blame before changing.
- decasia 8y agoI'm curious — is it not normal to add in-line explanatory comments in the Rails codebase? I'm thinking if I were writing this code in an application, and it looked this cryptic, I might at least add a comment noting what it was for. Not that people shouldn't look in git. But inline is easier to notice, no?
- riffraff 8y agoDon't we have tests for this?
- progval 8y agoThe tests are far from obvious too https://github.com/rails/rails/commit/f4c70c2222180b8d9d924f00af0c7fd632e26715#diff-10fcdd9642eb5b16366cccceb7da3116 https://github.com/rails/rails/commit/f4c70c2222180b8d9d924f...
- deleted 8y ago[deleted]