SimplyRestful named routes don't escape url's

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

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

Adam,

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

V/r
Anthony

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.

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&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

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.