From 0b6ce3422370647cad3e91263a291f69b313d65b Mon Sep 17 00:00:00 2001
From: Andrew White <andyw@pixeltrix.co.uk>
Date: Sun, 27 Jun 2010 09:16:46 +0100
Subject: [PATCH] Restores the escaping of urls generated from hashes. [#4765
 state:resolved]
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

HTML specifications recommend the escaping of urls in web pages,
which url_for does by default for string urls and consquently
urls generated by path helpers as these return strings.

Hashes passed to url_for are not escaped by default and this
commit reverses this default so that they are escaped.

Undoes the changes of this commit:
http://github.com/rails/rails/commit/1b3195b63ca44f0a70b61b75fcf4991cb2fbb944

Signed-off-by: José Valim <jose.valim@gmail.com>
---
 actionpack/lib/action_view/helpers/url_helper.rb |  2 +-
 actionpack/test/template/url_helper_test.rb      | 12 ++++--------
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb
index 6af11e632f..cbde9b94a7 100644
--- a/actionpack/lib/action_view/helpers/url_helper.rb
+++ b/actionpack/lib/action_view/helpers/url_helper.rb
@@ -104,7 +104,7 @@ module ActionView
           options
         when Hash
           options = { :only_path => options[:host].nil? }.update(options.symbolize_keys)
-          escape  = options.key?(:escape) ? options.delete(:escape) : false
+          escape  = options.key?(:escape) ? options.delete(:escape) : true
           super
         when :back
           escape = false
diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb
index 72d4897630..897228677b 100644
--- a/actionpack/test/template/url_helper_test.rb
+++ b/actionpack/test/template/url_helper_test.rb
@@ -41,7 +41,7 @@ class UrlHelperTest < ActiveSupport::TestCase
   alias url_hash hash_for
 
   def test_url_for_escapes_urls
-    assert_equal "/?a=b&c=d", url_for(abcd)
+    assert_equal "/?a=b&amp;c=d", url_for(abcd)
     assert_equal "/?a=b&amp;c=d", url_for(abcd(:escape => true))
     assert_equal "/?a=b&c=d", url_for(abcd(:escape => false))
   end
@@ -53,6 +53,7 @@ class UrlHelperTest < ActiveSupport::TestCase
 
   def test_url_for_escapes_url_once
     assert_equal "/?a=b&amp;c=d", url_for("/?a=b&amp;c=d")
+    assert_equal "/?a=b&amp;c=d", url_for(abcd)
   end
 
   def test_url_for_with_back
@@ -67,11 +68,6 @@ class UrlHelperTest < ActiveSupport::TestCase
     assert_equal 'javascript:history.back()', url_for(:back)
   end
 
-  def test_url_for_from_hash_doesnt_escape_ampersand
-    path = url_for(hash_for(:foo => :bar, :baz => :quux))
-    assert_equal '/?baz=quux&foo=bar', sort_query_string_params(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")
@@ -345,7 +341,7 @@ class UrlHelperTest < ActiveSupport::TestCase
       link_to_unless_current("Showing", "http://www.example.com/?order=asc")
 
     @request = request_for_url("/?order=desc")
-    assert_equal %{<a href="/?order=desc&page=2\">Showing</a>},
+    assert_equal %{<a href="/?order=desc&amp;page=2\">Showing</a>},
       link_to_unless_current("Showing", hash_for(:order => "desc", :page => 2))
     assert_equal %{<a href="http://www.example.com/?order=desc&amp;page=2">Showing</a>},
       link_to_unless_current("Showing", "http://www.example.com/?order=desc&page=2")
@@ -415,7 +411,7 @@ class UrlHelperTest < ActiveSupport::TestCase
   private
     def sort_query_string_params(uri)
       path, qs = uri.split('?')
-      qs = qs.split('&').sort.join('&') if qs
+      qs = qs.split('&amp;').sort.join('&amp;') if qs
       qs ? "#{path}?#{qs}" : path
     end
 end