From fc2f09f2ea8270b983aa52c3d72d3d0de89ba56d Mon Sep 17 00:00:00 2001 From: Bruno Michel Date: Sun, 7 Feb 2010 00:42:19 +0100 Subject: [PATCH] content_tag should escape its input --- .../lib/action_view/helpers/active_model_helper.rb | 6 +++--- actionpack/lib/action_view/helpers/date_helper.rb | 6 +++--- .../lib/action_view/helpers/form_options_helper.rb | 5 ++--- .../lib/action_view/helpers/form_tag_helper.rb | 6 +++--- .../lib/action_view/helpers/javascript_helper.rb | 2 +- actionpack/lib/action_view/helpers/tag_helper.rb | 2 +- actionpack/lib/action_view/helpers/url_helper.rb | 2 +- actionpack/test/template/form_tag_helper_test.rb | 16 ++++++++-------- actionpack/test/template/tag_helper_test.rb | 2 ++ actionpack/test/template/url_helper_test.rb | 2 +- 10 files changed, 25 insertions(+), 24 deletions(-) diff --git a/actionpack/lib/action_view/helpers/active_model_helper.rb b/actionpack/lib/action_view/helpers/active_model_helper.rb index e106bb0..c87e216 100644 --- a/actionpack/lib/action_view/helpers/active_model_helper.rb +++ b/actionpack/lib/action_view/helpers/active_model_helper.rb @@ -127,7 +127,7 @@ module ActionView if (obj = (object.respond_to?(:errors) ? object : instance_variable_get("@#{object}"))) && (errors = obj.errors[method]) content_tag("div", - "#{options[:prepend_text]}#{ERB::Util.html_escape(errors.first)}#{options[:append_text]}", + "#{options[:prepend_text]}#{ERB::Util.html_escape(errors.first)}#{options[:append_text]}".html_safe, :class => options[:css_class] ) else @@ -228,14 +228,14 @@ module ActionView object.errors.full_messages.map do |msg| content_tag(:li, ERB::Util.html_escape(msg)) end - end.join + end.join.html_safe contents = '' contents << content_tag(options[:header_tag] || :h2, header_message) unless header_message.blank? contents << content_tag(:p, message) unless message.blank? contents << content_tag(:ul, error_messages) - content_tag(:div, contents, html) + content_tag(:div, contents.html_safe, html) end else '' diff --git a/actionpack/lib/action_view/helpers/date_helper.rb b/actionpack/lib/action_view/helpers/date_helper.rb index 8be2f76..c2810b3 100644 --- a/actionpack/lib/action_view/helpers/date_helper.rb +++ b/actionpack/lib/action_view/helpers/date_helper.rb @@ -815,7 +815,7 @@ module ActionView tag_options[:selected] = "selected" if selected == i select_options << content_tag(:option, value, tag_options) end - select_options.join("\n") + "\n" + (select_options.join("\n") + "\n").html_safe end # Builds select tag from date type and html select options @@ -833,9 +833,9 @@ module ActionView select_html = "\n" select_html << content_tag(:option, '', :value => '') + "\n" if @options[:include_blank] select_html << prompt_option_tag(type, @options[:prompt]) + "\n" if @options[:prompt] - select_html << select_options_as_html.to_s + select_html << select_options_as_html - (content_tag(:select, select_html, select_options) + "\n").html_safe + (content_tag(:select, select_html.html_safe, select_options) + "\n").html_safe end # Builds a prompt option tag with supplied options or from default options diff --git a/actionpack/lib/action_view/helpers/form_options_helper.rb b/actionpack/lib/action_view/helpers/form_options_helper.rb index 0ffe770..7f74be2 100644 --- a/actionpack/lib/action_view/helpers/form_options_helper.rb +++ b/actionpack/lib/action_view/helpers/form_options_helper.rb @@ -572,10 +572,9 @@ module ActionView end if value.blank? && options[:prompt] prompt = options[:prompt].kind_of?(String) ? options[:prompt] : I18n.translate('helpers.select.prompt', :default => 'Please select') - "\n" + option_tags - else - option_tags + option_tags = "\n" + option_tags end + option_tags.html_safe end end diff --git a/actionpack/lib/action_view/helpers/form_tag_helper.rb b/actionpack/lib/action_view/helpers/form_tag_helper.rb index 6ed6c31..088960c 100644 --- a/actionpack/lib/action_view/helpers/form_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/form_tag_helper.rb @@ -90,9 +90,9 @@ module ActionView html_name = (options[:multiple] == true && !name.to_s.ends_with?("[]")) ? "#{name}[]" : name if blank = options.delete(:include_blank) if blank.kind_of?(String) - option_tags = "" + option_tags + option_tags = "".html_safe + option_tags else - option_tags = "" + option_tags + option_tags = "".html_safe + option_tags end end content_tag :select, option_tags, { "name" => html_name, "id" => sanitize_to_id(name) }.update(options.stringify_keys) @@ -277,7 +277,7 @@ module ActionView end escape = options.key?("escape") ? options.delete("escape") : true - content = html_escape(content) if escape + content = content.html_safe unless escape content_tag :textarea, content, { "name" => name, "id" => sanitize_to_id(name) }.update(options) end diff --git a/actionpack/lib/action_view/helpers/javascript_helper.rb b/actionpack/lib/action_view/helpers/javascript_helper.rb index 8fdaa8c..2c73ff8 100644 --- a/actionpack/lib/action_view/helpers/javascript_helper.rb +++ b/actionpack/lib/action_view/helpers/javascript_helper.rb @@ -93,7 +93,7 @@ module ActionView end def javascript_cdata_section(content) #:nodoc: - "\n//#{cdata_section("\n#{content}\n//")}\n" + "\n//#{cdata_section("\n#{content}\n//")}\n".html_safe end end end diff --git a/actionpack/lib/action_view/helpers/tag_helper.rb b/actionpack/lib/action_view/helpers/tag_helper.rb index a3a8185..1c3eb20 100644 --- a/actionpack/lib/action_view/helpers/tag_helper.rb +++ b/actionpack/lib/action_view/helpers/tag_helper.rb @@ -127,7 +127,7 @@ module ActionView def content_tag_string(name, content, options, escape = true) tag_options = tag_options(options, escape) if options - "<#{name}#{tag_options}>#{content}".html_safe + "<#{name}#{tag_options}>#{ERB::Util.h content}".html_safe end def tag_options(options, escape = true) diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index 168a3bd..88ce2a2 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -493,7 +493,7 @@ module ActionView char = c.chr string << (char =~ /\w/ ? sprintf("%%%x", c) : char) end - content_tag "a", name || email_address_encoded, html_options.merge({ "href" => "#{string}#{extras}" }) + content_tag "a", name || email_address_encoded.html_safe, html_options.merge({ "href" => "#{string}#{extras}" }) else content_tag "a", name || email_address_obfuscated, html_options.merge({ "href" => "mailto:#{email_address}#{extras}" }) end diff --git a/actionpack/test/template/form_tag_helper_test.rb b/actionpack/test/template/form_tag_helper_test.rb index 6ac5df1..0ceec1a 100644 --- a/actionpack/test/template/form_tag_helper_test.rb +++ b/actionpack/test/template/form_tag_helper_test.rb @@ -127,19 +127,19 @@ class FormTagHelperTest < ActionView::TestCase end def test_select_tag - actual = select_tag "people", "" + actual = select_tag "people", "".html_safe expected = %() assert_dom_equal expected, actual end def test_select_tag_with_multiple - actual = select_tag "colors", "", :multiple => :true + actual = select_tag "colors", "".html_safe, :multiple => :true expected = %() assert_dom_equal expected, actual end def test_select_tag_disabled - actual = select_tag "places", "", :disabled => :true + actual = select_tag "places", "".html_safe, :disabled => :true expected = %() assert_dom_equal expected, actual end @@ -150,13 +150,13 @@ class FormTagHelperTest < ActionView::TestCase end def test_select_tag_with_include_blank - actual = select_tag "places", "", :include_blank => true + actual = select_tag "places", "".html_safe, :include_blank => true expected = %() assert_dom_equal expected, actual end def test_select_tag_with_include_blank_with_string - actual = select_tag "places", "", :include_blank => "string" + actual = select_tag "places", "".html_safe, :include_blank => "string" expected = %() assert_dom_equal expected, actual end @@ -282,9 +282,9 @@ class FormTagHelperTest < ActionView::TestCase assert_dom_equal %(), check_box_tag("admin", 1, true, 'disabled' => true, :readonly => "yes") assert_dom_equal %(), check_box_tag("admin", 1, true, :disabled => false, :readonly => nil) assert_dom_equal %(), tag(:input, :type => "checkbox", :checked => false) - assert_dom_equal %(), select_tag("people", "", :multiple => true) - assert_dom_equal %(), select_tag("people[]", "", :multiple => true) - assert_dom_equal %(), select_tag("people", "", :multiple => nil) + assert_dom_equal %(), select_tag("people", "".html_safe, :multiple => true) + assert_dom_equal %(), select_tag("people[]", "".html_safe, :multiple => true) + assert_dom_equal %(), select_tag("people", "".html_safe, :multiple => nil) end def test_stringify_symbol_keys diff --git a/actionpack/test/template/tag_helper_test.rb b/actionpack/test/template/tag_helper_test.rb index 433f651..3858ffd 100644 --- a/actionpack/test/template/tag_helper_test.rb +++ b/actionpack/test/template/tag_helper_test.rb @@ -37,6 +37,8 @@ class TagHelperTest < ActionView::TestCase assert content_tag("a", "Create", "href" => "create").html_safe? assert_equal content_tag("a", "Create", "href" => "create"), content_tag("a", "Create", :href => "create") + assert_equal "

<script>evil_js</script>

", + content_tag(:p, '') end def test_content_tag_with_block_in_erb diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb index e904e88..418c050 100644 --- a/actionpack/test/template/url_helper_test.rb +++ b/actionpack/test/template/url_helper_test.rb @@ -346,7 +346,7 @@ class UrlHelperTest < ActionView::TestCase end def test_mail_to_with_img - assert_dom_equal %(), mail_to('feedback@example.com', '') + assert_dom_equal %(), mail_to('feedback@example.com', ''.html_safe) end def test_mail_to_with_hex -- 1.6.3.3