Escape query strings in the href attribute of URLs created by url_helper. #2333

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@2572 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 20 insertions, 2 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index 1f33d22e56..0e20b46fbb 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Escape query strings in the href attribute of URLs created by url_helper.  #2333 [Michael Schuerig <michael@schuerig.de>]
+
 * Improved line number reporting for template errors [Nicholas Seckar]
 
 * Added :locals support for render :inline #2463 [mdabney@cavoksolutions.com]
diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb
index 4b7a2270dc..118b5b158b 100644
--- a/actionpack/lib/action_view/helpers/url_helper.rb
+++ b/actionpack/lib/action_view/helpers/url_helper.rb
@@ -46,7 +46,7 @@ module ActionView
         else
           tag_options = nil
         end
-        url = options.is_a?(String) ? options : url_for(options, *parameters_for_method_reference)
+        url = html_escape(options.is_a?(String) ? options : url_for(options, *parameters_for_method_reference))
         "<a href=\"#{url}\"#{tag_options}>#{name||url}</a>"
       end
 
@@ -107,7 +107,7 @@ module ActionView
 
         url, name = options.is_a?(String) ?
           [ options,  name || options ] :
-          [ url_for(options), name || url_for(options) ]
+          [ url_for(options), name || html_escape(url_for(options)) ]
 
         html_options.merge!("type" => "submit", "value" => name)
 
diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb
index e85f710c44..78ad185542 100644
--- a/actionpack/test/template/url_helper_test.rb
+++ b/actionpack/test/template/url_helper_test.rb
@@ -25,6 +25,14 @@ class UrlHelperTest < Test::Unit::TestCase
     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")
   end
 
+  def test_button_to_with_query
+    assert_dom_equal "<form method=\"post\" action=\"http://www.example.com/q1=v1&amp;q2=v2\" class=\"button-to\"><div><input type=\"submit\" value=\"Hello\" /></div></form>", button_to("Hello", "http://www.example.com/q1=v1&q2=v2")
+  end
+
+  def test_button_to_with_query_and_no_name
+    assert_dom_equal "<form method=\"post\" action=\"http://www.example.com?q1=v1&amp;q2=v2\" class=\"button-to\"><div><input type=\"submit\" value=\"http://www.example.com?q1=v1&amp;q2=v2\" /></div></form>", button_to(nil, "http://www.example.com?q1=v1&q2=v2")
+  end
+
   def test_button_to_with_javascript_confirm
     assert_dom_equal(
       "<form method=\"post\" action=\"http://www.example.com\" class=\"button-to\"><div><input onclick=\"return confirm('Are you sure?');\" type=\"submit\" value=\"Hello\" /></div></form>",
@@ -47,6 +55,14 @@ class UrlHelperTest < Test::Unit::TestCase
     assert_dom_equal "<a href=\"http://www.example.com\">Hello</a>", link_to("Hello", "http://www.example.com")
   end
 
+  def test_link_tag_with_query
+    assert_dom_equal "<a href=\"http://www.example.com?q1=v1&amp;q2=v2\">Hello</a>", link_to("Hello", "http://www.example.com?q1=v1&q2=v2")
+  end
+
+  def test_link_tag_with_query_and_no_name
+    assert_dom_equal "<a href=\"http://www.example.com?q1=v1&amp;q2=v2\">http://www.example.com?q1=v1&amp;q2=v2</a>", link_to(nil, "http://www.example.com?q1=v1&q2=v2")
+  end
+
   def test_link_tag_with_custom_onclick
     assert_dom_equal "<a href=\"http://www.example.com\" onclick=\"alert('yay!')\">Hello</a>", link_to("Hello", "http://www.example.com", :onclick => "alert('yay!')")
   end