UrlRewriter#rewrite_url should call #to_param on the value given in :anchor option, just as #url_for does

[#2746 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>

2 files changed, 27 insertions, 1 deletions

diff --git a/actionpack/lib/action_controller/routing/generation/url_rewriter.rb b/actionpack/lib/action_controller/routing/generation/url_rewriter.rb
index 9717582b5e..52b66c9303 100644
--- a/actionpack/lib/action_controller/routing/generation/url_rewriter.rb
+++ b/actionpack/lib/action_controller/routing/generation/url_rewriter.rb
@@ -172,7 +172,7 @@ module ActionController
         path = rewrite_path(options)
         rewritten_url << ActionController::Base.relative_url_root.to_s unless options[:skip_relative_url_root]
         rewritten_url << (options[:trailing_slash] ? path.sub(/\?|\z/) { "/" + $& } : path)
-        rewritten_url << "##{options[:anchor]}" if options[:anchor]
+        rewritten_url << "##{CGI.escape(options[:anchor].to_param.to_s)}" if options[:anchor]
 
         rewritten_url
       end
diff --git a/actionpack/test/controller/url_rewriter_test.rb b/actionpack/test/controller/url_rewriter_test.rb
index ad915e7a57..9b8d07222b 100644
--- a/actionpack/test/controller/url_rewriter_test.rb
+++ b/actionpack/test/controller/url_rewriter_test.rb
@@ -46,6 +46,20 @@ class UrlRewriterTests < ActionController::TestCase
     )
   end
 
+  def test_anchor_should_call_to_param
+    assert_equal(
+      'http://test.host/c/a/i#anchor',
+      @rewriter.rewrite(:controller => 'c', :action => 'a', :id => 'i', :anchor => Struct.new(:to_param).new('anchor'))
+    )
+  end
+
+  def test_anchor_should_be_cgi_escaped
+    assert_equal(
+      'http://test.host/c/a/i#anc%2Fhor',
+      @rewriter.rewrite(:controller => 'c', :action => 'a', :id => 'i', :anchor => Struct.new(:to_param).new('anc/hor'))
+    )
+  end
+
   def test_overwrite_params
     @params[:controller] = 'hi'
     @params[:action] = 'bye'
@@ -110,6 +124,18 @@ class UrlWriterTests < ActionController::TestCase
     )
   end
 
+  def test_anchor_should_call_to_param
+    assert_equal('/c/a#anchor',
+      W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :anchor => Struct.new(:to_param).new('anchor'))
+    )
+  end
+
+  def test_anchor_should_be_cgi_escaped
+    assert_equal('/c/a#anc%2Fhor',
+      W.new.url_for(:only_path => true, :controller => 'c', :action => 'a', :anchor => Struct.new(:to_param).new('anc/hor'))
+    )
+  end
+
   def test_default_host
     add_host!
     assert_equal('http://www.basecamphq.com/c/a/i',