ActionView.url_for doesn't escape by default

ActionView::Helpers::UrlHelper#url_for used to escape the URLs it generated by
default.  This was most commonly seen when generating a path with multiple
query parameters, e.g.

  url_for(:controller => :foo, :action => :bar, :this => 123, :that => 456)

would return

  http://example.com/foo/bar?that=456&this=123

escaping an ampersand that shouldn't be escaped.  This is both wrong and
inconsistent with the behavior of ActionController#url_for, and is changed.

Signed-off-by: Michael Koziarski <michael@koziarski.com>

1 files changed, 13 insertions, 3 deletions

diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb
index 111a7619b5..cc3b2455d7 100644
--- a/actionpack/test/template/url_helper_test.rb
+++ b/actionpack/test/template/url_helper_test.rb
@@ -22,7 +22,7 @@ class UrlHelperTest < ActionView::TestCase
 
   def test_url_for_escapes_urls
     @controller.url = "http://www.example.com?a=b&c=d"
-    assert_equal "http://www.example.com?a=b&amp;c=d", url_for(:a => 'b', :c => 'd')
+    assert_equal "http://www.example.com?a=b&c=d", url_for(:a => 'b', :c => 'd')
     assert_equal "http://www.example.com?a=b&amp;c=d", url_for(:a => 'b', :c => 'd', :escape => true)
     assert_equal "http://www.example.com?a=b&c=d", url_for(:a => 'b', :c => 'd', :escape => false)
   end
@@ -42,6 +42,16 @@ class UrlHelperTest < ActionView::TestCase
     assert_equal 'javascript:history.back()', url_for(:back)
   end
 
+  def test_url_for_from_hash_doesnt_escape_ampersand
+    @controller = TestController.new
+    @view = ActionView::Base.new
+    @view.controller = @controller
+
+    path = @view.url_for(:controller => :cheeses, :foo => :bar, :baz => :quux)
+
+    assert_equal '/cheeses?baz=quux&foo=bar', path
+  end
+
   # todo: missing test cases
   def test_button_to_with_straight_url
     assert_dom_equal "<form method=\"post\" action=\"http://www.example.com\" class=\"button-to\"><div><input type=\"submit\" value=\"Hello\" /></div></form>", button_to("Hello", "http://www.example.com")
@@ -298,7 +308,7 @@ class UrlHelperTest < ActionView::TestCase
     @controller.request = RequestMock.new("http://www.example.com/weblog/show?order=desc&page=1")
     @controller.url = "http://www.example.com/weblog/show?order=desc&page=1"
     assert_equal "Showing", link_to_unless_current("Showing", { :action => "show", :controller => "weblog", :order=>'desc', :page=>'1' })
-    assert_equal "Showing", link_to_unless_current("Showing", "http://www.example.com/weblog/show?order=desc&amp;page=1")
+    assert_equal "Showing", link_to_unless_current("Showing", "http://www.example.com/weblog/show?order=desc&page=1")
     assert_equal "Showing", link_to_unless_current("Showing", "http://www.example.com/weblog/show?order=desc&page=1")
 
     @controller.request = RequestMock.new("http://www.example.com/weblog/show?order=desc")
@@ -308,7 +318,7 @@ class UrlHelperTest < ActionView::TestCase
 
     @controller.request = RequestMock.new("http://www.example.com/weblog/show?order=desc&page=1")
     @controller.url = "http://www.example.com/weblog/show?order=desc&page=2"
-    assert_equal "<a href=\"http://www.example.com/weblog/show?order=desc&amp;page=2\">Showing</a>", link_to_unless_current("Showing", { :action => "show", :controller => "weblog" })
+    assert_equal "<a href=\"http://www.example.com/weblog/show?order=desc&page=2\">Showing</a>", link_to_unless_current("Showing", { :action => "show", :controller => "weblog" })
     assert_equal "<a href=\"http://www.example.com/weblog/show?order=desc&amp;page=2\">Showing</a>", link_to_unless_current("Showing", "http://www.example.com/weblog/show?order=desc&page=2")