From ec3bfa2ead7337fe99ff036d4b8b5ae0a321f8cd Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Wed, 30 Jun 2010 00:12:48 +0200 Subject: [PATCH] s/escape_once/html_escape/, since html safety is the contract that now says whether something has to be escaped --- actionpack/CHANGELOG | 2 ++ .../action_view/helpers/form_tag_helper.rb | 2 +- .../lib/action_view/helpers/tag_helper.rb | 2 +- .../lib/action_view/helpers/url_helper.rb | 29 ++++++++++--------- actionpack/test/template/tag_helper_test.rb | 4 +-- actionpack/test/template/url_helper_test.rb | 4 +-- 6 files changed, 24 insertions(+), 19 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 "#{html_escape(name || url)}".html_safe end end @@ -328,7 +328,7 @@ module ActionView html_options.merge!("type" => "submit", "value" => name) - ("
" + + ("
" + method_tag + tag("input", html_options) + request_token_tag + "
").html_safe end @@ -474,24 +474,27 @@ module ActionView # :subject => "This is an example email" # # => My email 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? + 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 = html_escape(email_address) + 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 "".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 < 2 & 3', escape_once('1 < 2 & 3') end - def test_double_escaping_attributes + def test_tag_honors_html_safe_for_param_values ['1&2', '1 < 2', '“test“'].each do |escaped| - assert_equal %(), tag('a', :href => escaped) + assert_equal %(), 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 "
", button_to("Hello", "http://www.example.com/q1=v1&q2=v2") end - def test_button_to_with_escaped_query - assert_dom_equal "
", button_to("Hello", "http://www.example.com/q1=v1&q2=v2") + def test_button_to_with_html_safe_URL + assert_dom_equal "
", button_to("Hello", "http://www.example.com/q1=v1&q2=v2".html_safe) end def test_button_to_with_query_and_no_name