s/escape_once/html_escape/, since html safety is the contract that now says whether something has to be escaped

6 files changed, 25 insertions, 20 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index 80be915407..32aba2091a 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
 *Rails 3.0.0 [Release Candidate] (unreleased)*
 
+* link_to, button_to, and tag/tag_options now rely on html_escape instead of escape_once. [fxn]
+
 * url_for returns always unescaped strings, and the :escape option is gone. [fxn]
 
 * Added accept-charset parameter and _snowman hidden field to force the contents
diff --git a/actionpack/lib/action_view/helpers/form_tag_helper.rb b/actionpack/lib/action_view/helpers/form_tag_helper.rb
index 7fea5eb055..4c1b751160 100644
--- a/actionpack/lib/action_view/helpers/form_tag_helper.rb
+++ b/actionpack/lib/action_view/helpers/form_tag_helper.rb
@@ -539,7 +539,7 @@ module ActionView
 
         def extra_tags_for_form(html_options)
           snowman_tag = tag(:input, :type => "hidden",
-                            :name => "_snowman", :value => "☃")
+                            :name => "_snowman", :value => "☃".html_safe)
 
           method = html_options.delete("method").to_s
 
diff --git a/actionpack/lib/action_view/helpers/tag_helper.rb b/actionpack/lib/action_view/helpers/tag_helper.rb
index d4e8b3d587..5d032b32a7 100644
--- a/actionpack/lib/action_view/helpers/tag_helper.rb
+++ b/actionpack/lib/action_view/helpers/tag_helper.rb
@@ -122,7 +122,7 @@ module ActionView
                 attrs << %(#{key}="#{key}") if value
               elsif !value.nil?
                 final_value = value.is_a?(Array) ? value.join(" ") : value
-                final_value = escape_once(final_value) if escape
+                final_value = html_escape(final_value) if escape
                 attrs << %(#{key}="#{final_value}")
               end
             end
diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb
index 7d1d00d1fa..b8d6dc22f2 100644
--- a/actionpack/lib/action_view/helpers/url_helper.rb
+++ b/actionpack/lib/action_view/helpers/url_helper.rb
@@ -243,7 +243,7 @@ module ActionView
             tag_options = nil
           end
 
-          href_attr = "href=\"#{escape_once(url)}\"" unless href
+          href_attr = "href=\"#{html_escape(url)}\"" unless href
           "<a #{href_attr}#{tag_options}>#{html_escape(name || url)}</a>".html_safe
         end
       end
@@ -328,7 +328,7 @@ module ActionView
 
         html_options.merge!("type" => "submit", "value" => name)
 
-        ("<form method=\"#{form_method}\" action=\"#{escape_once url}\" #{"data-remote=\"true\"" if remote} class=\"button_to\"><div>" +
+        ("<form method=\"#{form_method}\" action=\"#{html_escape(url)}\" #{"data-remote=\"true\"" if remote} class=\"button_to\"><div>" +
           method_tag + tag("input", html_options) + request_token_tag + "</div></form>").html_safe
       end
 
@@ -474,24 +474,27 @@ module ActionView
       #            :subject => "This is an example email"
       #   # => <a href="mailto:me@domain.com?cc=ccaddress@domain.com&subject=This%20is%20an%20example%20email">My email</a>
       def mail_to(email_address, name = nil, html_options = {})
+        email_address = html_escape(email_address)
+
         html_options = html_options.stringify_keys
         encode = html_options.delete("encode").to_s
         cc, bcc, subject, body = html_options.delete("cc"), html_options.delete("bcc"), html_options.delete("subject"), html_options.delete("body")
 
-        string = ''
-        extras = ''
-        extras << "cc=#{Rack::Utils.escape(cc).gsub("+", "%20")}&" unless cc.nil?
-        extras << "bcc=#{Rack::Utils.escape(bcc).gsub("+", "%20")}&" unless bcc.nil?
-        extras << "body=#{Rack::Utils.escape(body).gsub("+", "%20")}&" unless body.nil?
-        extras << "subject=#{Rack::Utils.escape(subject).gsub("+", "%20")}&" unless subject.nil?
-        extras = "?" << extras.gsub!(/&?$/,"") unless extras.empty?
-
-        email_address_obfuscated = html_escape(email_address)
+        extras = []
+        extras << "cc=#{Rack::Utils.escape(cc).gsub("+", "%20")}" unless cc.nil?
+        extras << "bcc=#{Rack::Utils.escape(bcc).gsub("+", "%20")}" unless bcc.nil?
+        extras << "body=#{Rack::Utils.escape(body).gsub("+", "%20")}" unless body.nil?
+        extras << "subject=#{Rack::Utils.escape(subject).gsub("+", "%20")}" unless subject.nil?
+        extras = extras.empty? ? '' : '?' + html_escape(extras.join('&'))
+
+        email_address_obfuscated = email_address.dup
         email_address_obfuscated.gsub!(/@/, html_options.delete("replace_at")) if html_options.has_key?("replace_at")
         email_address_obfuscated.gsub!(/\./, html_options.delete("replace_dot")) if html_options.has_key?("replace_dot")
 
+        string = ''
+
         if encode == "javascript"
-          "document.write('#{content_tag("a", name || email_address_obfuscated.html_safe, html_options.merge({ "href" => "mailto:"+email_address+extras }))}');".each_byte do |c|
+          "document.write('#{content_tag("a", name || email_address_obfuscated.html_safe, html_options.merge("href" => "mailto:#{email_address}#{extras}".html_safe))}');".each_byte do |c|
             string << sprintf("%%%x", c)
           end
           "<script type=\"#{Mime::JS}\">eval(decodeURIComponent('#{string}'))</script>".html_safe
@@ -508,9 +511,9 @@ module ActionView
             char = c.chr
             string << (char =~ /\w/ ? sprintf("%%%x", c) : char)
           end
-          content_tag "a", name || email_address_encoded.html_safe, html_options.merge({ "href" => "#{string}#{extras}" })
+          content_tag "a", name || email_address_encoded.html_safe, html_options.merge("href" => "#{string}#{extras}".html_safe)
         else
-          content_tag "a", name || email_address_obfuscated.html_safe, html_options.merge({ "href" => "mailto:#{email_address}#{extras}" })
+          content_tag "a", name || email_address_obfuscated.html_safe, html_options.merge("href" => "mailto:#{email_address}#{extras}".html_safe)
         end
       end
 
diff --git a/actionpack/test/template/tag_helper_test.rb b/actionpack/test/template/tag_helper_test.rb
index ec5fe3d1d7..507cdca8d0 100644
--- a/actionpack/test/template/tag_helper_test.rb
+++ b/actionpack/test/template/tag_helper_test.rb
@@ -95,9 +95,9 @@ class TagHelperTest < ActionView::TestCase
     assert_equal '1 &lt; 2 &amp; 3', escape_once('1 < 2 &amp; 3')
   end
   
-  def test_double_escaping_attributes
+  def test_tag_honors_html_safe_for_param_values
     ['1&amp;2', '1 &lt; 2', '&#8220;test&#8220;'].each do |escaped|
-      assert_equal %(<a href="#{escaped}" />), tag('a', :href => escaped)
+      assert_equal %(<a href="#{escaped}" />), tag('a', :href => escaped.html_safe)
     end
   end
   
diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb
index 035f501f03..befb55fb48 100644
--- a/actionpack/test/template/url_helper_test.rb
+++ b/actionpack/test/template/url_helper_test.rb
@@ -65,8 +65,8 @@ class UrlHelperTest < ActiveSupport::TestCase
     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_escaped_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&amp;q2=v2")
+  def test_button_to_with_html_safe_URL
+    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&amp;q2=v2".html_safe)
   end
 
   def test_button_to_with_query_and_no_name