SimplyRestful named routes don't escape url's

rubyonrails-core
1

Is there a reason why SimplyRestful named routes don't escape url's? Thus using any additional query parameters with them results in invalid HTML.

An example:

url_for(:controller => "entries",                                          :action => "new",                                          :enterable_id => @company.id,                                          :enterable_type => "Company",                                          :return_to => request.request_uri)

=> <a href="/entries/new?enterable_id=1&amp;enterable_type=Company&amp;return_to=%2Fcompanies%2F1"

vs.

new_entry_path( :enterable_id => @company.id,                                          :enterable_type => "Company",                                          :return_to => request.request_uri)

=> <a href="/entries/new?enterable_id=1&enterable_type=Company&return_to=%2Fcompanies%2F1"

I can prepare a patch for this if this is simply a bug/omission, just wondered if escaping is deliberately left out.

Cheers, //jarkko

2

ah, I just ran into this today when I was checking the validity of my xhtml.

core team, is this just an omission? If so, Jarkko, I would definitely be willing to help you out on this. I've been trying to find where the restful helper '_url' methods are created w/o much luck so far. Or can someone point me in the right direction?

Thanks, adam

3

Adam,

They are mapped in actionpack/lib/action_controller/resources.rb

V/r Anthony

4

That's funny, because form tags double encode named routes.

<%= form_tag foo_path(...) %>

foo_path converts to an unencoded string form_tag calls url_for which encodes all strings form_tag calls tag which processes and encodes all attributes.

I'd say you can't go wrong without a good set of test cases to back you up.

5

woops!

I figured a way around this problem but it isn't pretty.

link_to title, graph_url(:id => @graph.id, :zoom => zoom_factor(zoom_key), :range=> date_range(range_key) )

http://test.host/graphs/40?range=10&zoom=4

link_to title, hash_for_graph_url(:id => @graph.id, :zoom => zoom_factor( zoom_key), :range=> date_range(range_key) )

/graphs/40?range=10&amp;zoom=4

that last one is what I want; I think most people would want the same thing, but that is too 'tricky'.

Rick, as you indirectly pointed out, doing it at the resource '_url' level is not the right place to do it (ie; what if it is used in a method that already encodes, or is printed to text and not html where it doesn't need to be encoded, etc).

That would make link_to the correct place to change this logic. Since url_for already handles strings, can we replace this logic in ActionView/Helpers/UrlHelper#link_to:         # url = options.is_a?(String) ? options : self.url_for(options, *parameters_for_method_reference)         url = self.url_for(options, *parameters_for_method_reference)

that causes the output to go from: http://test.host/graphs/43?_method=put&zoom=4 to: http://test.host/graphs/43?_method=put&amp;zoom=4

if something like that makes sense I'll put a patch up on to Trac, complete with test cases rick! :wink:

thanks, Adam

6

Not quite - rather that the url_for helper escapes ALL raw strings passed. There was a patch for that (and other URl escaping bugs) literally a month ago. Nobody noticed.