This project is archived and is in readonly mode.
params in functional tests not sent to controller encoded
Reported by johnjosephbachir (at gmail) | October 26th, 2010 @ 06:39 PM | in 3.0.5
I'm not exactly sure at what layer this problem is happening, so
I'll just describe the behavior. I have an action in my
foo controller, bar:
def bar
"123!@/#$%^" == params[:number]
end
and a spec (also tried with test_unit, same behavior):
it "should be equal" do
post :bar, :number => "123!@/#$%^"
end
now, that failed, and complained that there was no route that matched what I was asking for. I then realized that another spec worked, where the param being sent was completely alphanumeric. So I tried:
it "should be equal"
post :bar, :number => CGI::escape("123!@/#$%^")
end
And it worked! So the problem was the '/' being sent as part of the URL.
However, my code didn't work, because the param value was "still" escaped in the controller. I thought, am I crazy or doesn't rails unescape params for me? (been a rails developer for 4 years). Just to make sure I tried it in the browser, and I was right, rails does do the unescaping.
Also, aren't controller specs/tests supposed to never be at the http level? I'm not sending a url to the controller, I'm invoking it with some values... escaping, urls, etc, should never ben an issue, right?
Let me know if you'd like any more info. I can put together some example code of that's helpful.
Comments and changes to this ticket
-
Andrew White October 26th, 2010 @ 08:50 PM
- Assigned user set to “Andrew White”
- Importance changed from “” to “Low”
-
Andrew White October 29th, 2010 @ 12:47 PM
- State changed from “new” to “open”
- Milestone set to “3.0.2”
You're right that the params in controller functional tests are never at the HTTP level which is why your param is still escaped. If you look at the code which processes the request then all that happens to the params is that they are merged into to @controller.params.
The problem is that slashes aren't allowed in path parameters so you get the route not matching error. There are two things you can do to workaround the problem. The first one is to customise the path parameter regexp (e.g: :number => /.+/). The second escape the parameter manually (as you've done) - which you'd have to do in your app as well as only query params are automatically escaped.
I think there's definitely something that needs to be addressed here the question is how - do we automatically escape the params on the way in or automatically unescape the params on the way out.
-
-
Xavier Noria November 2nd, 2010 @ 11:52 AM
Param values in functional tests are raw, whatever you put goes.
I am not sure Rails can do the escaping automatically because values may not be strings. They should in your tests generally speaking as a best practice, because the controller expects them to be strings. But in practice sometimes they aren't, eg a model's ID passed directly as an integer, nested hashes.... In particular Rails cannot assume they are strings.
Converting values to strings should be done before considering this patch in my view. That has been sometimes discussed, I don't remember why isn't done off the top of my head. Anyone?
That is, either values are raw, or else they fully emulate HTTP params. A mix of both could be confusing IMO.
-
-
-
johnjosephbachir (at gmail) November 17th, 2010 @ 03:07 AM
Since submitting this ticket I've realized/concluded that in the scenario described, we actually should not expect rails to do the decoding. The whole point of url encoding something is that so that url syntax can be used within a url variable. So, there is in fact no way for the application stack to inherently know when part of a url is encoded.
Sorry for confusing the issue -- this ticket can be closed.
Cheers,
John -
Rohit Arondekar December 12th, 2010 @ 02:21 AM
- State changed from “open” to “invalid”
-
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
Andrew White
Jeff Kreeftmeijer
Jeremy Kemper
Piotr Sarnacki
Rohit Arondekar
Ryan Bigg
Xavier Noria