url_for :escape parameter is ignored

While this maybe isn't the exact issue Jan encountered, I noticed something similar, where #content_tag_as_string completely ignores the escape param on the content. This leads to errors with the engineyard/rails_metrics plugin for example, and basically, for every call to #content_tag where the 2nd param is given as a String.

Patch can be found at: http://github.com/snusnu/rails/commit/7f1b5bc6452cd1ca65dae3283c4af...

I'll keep on looking if there's already another ticket for that specific issue, or maybe it's even the same issue as this ticket describes anyway. If not, I'll probably open another ticket and link it back to this one too

that issue is unrelated. the problem is in actionpack/lib/action_dispatch/routing/route_set.rb:

# route_set.rb
def url_for(options)
  ...
  rewritten_url << "##{Rack::Utils.escape(options[:anchor].to_param.to_s)}" if options[:anchor]
end

# actionpack/lib/action_view/helpers/url_helper.rb
def url_for(option)
  ...
  escape  = options.key?(:escape) ? options.delete(:escape) : false
  super
end

So what happens is that while url_for in url_helper.rb accepts an escape option, it doesn't get passed through route_set.rb (the key is deleted).

I could fix this but wanted to check first if it the desired behaviour, to me it looks like a bug.

The culprit is following code

rewritten_url << "##{Rack::Utils.escape(options[:anchor].to_param.to_s)}" if options[:anchor]

As you can see anchor tag is always escaped. And it does not depend on options that is passed to this method.

http://github.com/rails/rails/blob/master/actionpack/lib/action_dis...

I tried to look at HTTP RFC and could not find any reference that anchor tag should always be escaped. If no such reference is found then it should be considered a bug. The patch and test can be generated easily.

@neeraj: anchor tags are not even sent to the server, they are purely local. it still makes sense to escape the anchor tag to produce valid urls, but the content is more free form than for example the query string, so it should be possible to have that part not escaped automatically.

my concrete use case is google analytics tracking. there's an option to append tracking parameters to the anchor instead of having them as query parameters.

http://example.com?utm_campaign=bar
http://example.com/#utm_campaign=bar

However if the content of the anchor gets escaped the tracking stops working.

:anchor (as everything else on url_for) should respect the :escape parameter. Patches please! :)

(from [f8f4872fccbc6ba2b4970e4e9eab9ce7f0d19986]) Backported patch from [#4762 url_for :escape parameter is ignored ]

URL fragments should not have safe characters escaped. Ref: Appendix A,
http://tools.ietf.org/rfc/rfc3986.txt

Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/f8f4872fccbc6ba2b4970e4e9eab9c...

From http://tools.ietf.org/html/rfc3986#section-3.5, "/" and "?" should also be considered "safe" characters for the anchor. The patch doesn't look like it handles those characters. Also it looks like '%' should not be allowed.

This fix also needs to be applied to the rewrite_url function on the 2.3 branch. The :anchor option is still being escaped using CGI.escape. https://github.com/rails/rails/blob/v2.3.10/actionpack/lib/action_c...

I agree with Shayan: / and ? shouldn't be escaped. I would like to use URLs like www.server.com/controller/action#tab=controller/action, but Rails 3.0.3 escapes the /. Any chance to reopen this ticket?

is this an issue in 3-0-stable and/or master?

It's an issue in 2-3-stable, 3-0-stable and master. 3-0-stable and master use Rack::Mount::Utils.escape_uri which doesn't have '/' and '?' in the list since those need escaping for routes. 2-3-stable uses URI.escape directly which Rack::Mount::Util.escape_uri does as well, so we might as well use URI.escape directly in 3-0-stable and master.

Thank you. It's an issue on 3-0-stable and master. I've modified the tests, so it fails, if / or ? are escaped, but I don't dare to write a patch.

Well, I dared after all, and put Andrews suggestion into a patch. Hope, everything is alright the way I did it.

You need to double check the unsafe char list you are escaping - the default for URI.escape is UNSAFE, which includes anything not in the UNRESERVED and RESERVED list. The spec says it should be PCHAR + '/?' - this means #,[ & ] should be escaped but the default URI.escape doesn't encode them.

Hey Andrew, can I leave the ticket for you to push when you think the fix is ready?