From d329b41942df6c9e3b25dbf655c2a23c05a804d4 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 17 Jun 2010 12:56:15 -0300 Subject: [PATCH] Make text_helpers methods which return valid html to return it as safe and sanitize the input always unless :sanitize => false is set [#4825 state:committed] --- actionpack/lib/action_view/helpers/text_helper.rb | 20 ++++-- actionpack/test/template/text_helper_test.rb | 68 +++++++++------------ 2 files changed, 42 insertions(+), 46 deletions(-) diff --git a/actionpack/lib/action_view/helpers/text_helper.rb b/actionpack/lib/action_view/helpers/text_helper.rb index 654f3c8..c7f9659 100644 --- a/actionpack/lib/action_view/helpers/text_helper.rb +++ b/actionpack/lib/action_view/helpers/text_helper.rb @@ -112,13 +112,13 @@ module ActionView end options.reverse_merge!(:highlighter => '\1') - text = h(text) unless text.html_safe? || options[:safe] + text = sanitize(text) unless options[:sanitize] == false if text.blank? || phrases.blank? text else match = Array(phrases).map { |p| Regexp.escape(p) }.join('|') text.gsub(/(#{match})(?!(?:[^<]*?)(?:["'])[^<>]*>)/i, options[:highlighter]) - end + end.html_safe end # Extracts an excerpt from +text+ that matches the first instance of +phrase+. @@ -248,9 +248,9 @@ module ActionView # simple_format("Look ma! A class!", :class => 'description') # # => "

Look ma! A class!

" def simple_format(text, html_options={}, options={}) - text = '' if text.nil? + text = ''.html_safe if text.nil? start_tag = tag('p', html_options, true) - text = h(text) unless text.html_safe? || options[:safe] + text = sanitize(text) unless options[:sanitize] == false text.gsub!(/\r\n?/, "\n") # \r\n and \r -> \n text.gsub!(/\n\n+/, "

\n\n#{start_tag}") # 2+ newline -> paragraph text.gsub!(/([^\n]\n)(?=[^\n])/, '\1
') # 1 newline -> br @@ -494,7 +494,11 @@ module ActionView link_text = block_given?? yield(href) : href href = 'http://' + href unless scheme - content_tag(:a, link_text, link_attributes.merge('href' => href), !(options[:safe] || text.html_safe?)) + punctuation.reverse.join('') + unless options[:sanitize] == false + link_text = sanitize(link_text) + href = sanitize(href) + end + content_tag(:a, link_text, link_attributes.merge('href' => href), !!options[:sanitize]) + punctuation.reverse.join('') end end.html_safe end @@ -509,7 +513,11 @@ module ActionView text.html_safe else display_text = (block_given?) ? yield(text) : text - display_text = h(display_text) unless options[:safe] + + unless options[:sanitize] == false + text = sanitize(text) + display_text = sanitize(display_text) unless text == display_text + end mail_to text, display_text, html_options end end diff --git a/actionpack/test/template/text_helper_test.rb b/actionpack/test/template/text_helper_test.rb index 82c81dd..d173c5b 100644 --- a/actionpack/test/template/text_helper_test.rb +++ b/actionpack/test/template/text_helper_test.rb @@ -19,6 +19,10 @@ class TextHelperTest < ActionView::TestCase assert_equal 'foobar', output_buffer end + def test_simple_format_should_be_html_safe + assert simple_format(" test with html tags ").html_safe? + end + def test_simple_format assert_equal "

", simple_format(nil) @@ -36,26 +40,18 @@ class TextHelperTest < ActionView::TestCase assert_equal %Q(

para 1

\n\n

para 2

), simple_format("para 1\n\npara 2", :class => 'test') end - def test_simple_format_should_be_html_safe - assert simple_format(" test with html tags ").html_safe? + def test_simple_format_should_sanitize_input_when_sanitize_option_is_not_false + assert_equal "

test with unsafe string

", simple_format(" test with unsafe string ") end - def test_simple_format_should_escape_unsafe_input - assert_equal "

<b> test with unsafe string </b><script>code!</script>

", simple_format(" test with unsafe string ") - end - - def test_simple_format_should_not_escape_input_if_safe_option - assert_equal "

test with unsafe string

", simple_format(" test with unsafe string ", {}, :safe => true) - end - - def test_simple_format_should_not_escape_safe_input - assert_equal "

test with safe string

", simple_format(" test with safe string ".html_safe) + def test_simple_format_should_not_sanitize_input_when_sanitize_option_is_false + assert_equal "

test with unsafe string

", simple_format(" test with unsafe string ", {}, :sanitize => false) end def test_truncate_should_not_be_html_safe assert !truncate("Hello World!", :length => 12).html_safe? end - + def test_truncate assert_equal "Hello World!", truncate("Hello World!", :length => 12) assert_equal "Hello Wor...", truncate("Hello World!!", :length => 12) @@ -128,24 +124,17 @@ class TextHelperTest < ActionView::TestCase assert_equal ' ', highlight(' ', 'blank text is returned verbatim') end - def test_highlight_should_escape_unsafe_input + def test_highlight_should_sanitize_input assert_equal( - "This is a beautiful morning<script>code!</script>", + "This is a beautiful morning", highlight("This is a beautiful morning", "beautiful") ) end - def test_highlight_should_not_escape_input_if_safe_option + def test_highlight_should_not_sanitize_if_sanitize_option_if_false assert_equal( "This is a beautiful morning", - highlight("This is a beautiful morning", "beautiful", :safe => true) - ) - end - - def test_highlight_should_not_escape_safe_input - assert_equal( - "This is a beautiful morning", - highlight("This is a beautiful morning".html_safe, "beautiful") + highlight("This is a beautiful morning", "beautiful", :sanitize => false) ) end @@ -179,23 +168,23 @@ class TextHelperTest < ActionView::TestCase def test_highlight_with_html assert_equal( - "<p>This is a beautiful morning, but also a beautiful day</p>", + "

This is a beautiful morning, but also a beautiful day

", highlight("

This is a beautiful morning, but also a beautiful day

", "beautiful") ) assert_equal( - "<p>This is a <em>beautiful</em> morning, but also a beautiful day</p>", + "

This is a beautiful morning, but also a beautiful day

", highlight("

This is a beautiful morning, but also a beautiful day

", "beautiful") ) assert_equal( - "<p>This is a <em class="error">beautiful</em> morning, but also a beautiful <span class="last">day</span></p>", + "

This is a beautiful morning, but also a beautiful day

", highlight("

This is a beautiful morning, but also a beautiful day

", "beautiful") ) assert_equal( - "<p class="beautiful">This is a beautiful morning, but also a beautiful day</p>", + "

This is a beautiful morning, but also a beautiful day

", highlight("

This is a beautiful morning, but also a beautiful day

", "beautiful") ) assert_equal( - "<p>This is a beautiful <a href="http://example.com/beautiful#top?what=beautiful%20morning&when=now+then">morning</a>, but also a beautiful day</p>", + "

This is a beautiful morning, but also a beautiful day

", highlight("

This is a beautiful morning, but also a beautiful day

", "beautiful") ) end @@ -317,9 +306,13 @@ class TextHelperTest < ActionView::TestCase end end - def generate_result(link_text, href = nil) + def generate_result(link_text, href = nil, escape = false) href ||= link_text - %{#{CGI::escapeHTML link_text}} + if escape + %{#{CGI::escapeHTML link_text}} + else + %{#{link_text}} + end end def test_auto_link_should_be_html_safe @@ -424,19 +417,14 @@ class TextHelperTest < ActionView::TestCase assert_equal %(

#{link10_result} Link

), auto_link("

#{link10_raw} Link

") end - def test_auto_link_should_sanitize_unsafe_input - link_raw = %{http://www.rubyonrails.com?id=1&num=2} - assert_equal %{http://www.rubyonrails.com?id=1&num=2}, auto_link(link_raw) - end - - def test_auto_link_should_sanitize_unsafe_input + def test_auto_link_should_sanitize_input_when_sanitize_option_is_not_false link_raw = %{http://www.rubyonrails.com?id=1&num=2} - assert_equal %{http://www.rubyonrails.com?id=1&num=2}, auto_link(link_raw, :safe => true) + assert_equal %{http://www.rubyonrails.com?id=1&num=2}, auto_link(link_raw) end - def test_auto_link_should_not_sanitize_safe_input + def test_auto_link_should_not_sanitize_input_when_sanitize_option_is_false link_raw = %{http://www.rubyonrails.com?id=1&num=2} - assert_equal %{http://www.rubyonrails.com?id=1&num=2}, auto_link(link_raw.html_safe) + assert_equal %{http://www.rubyonrails.com?id=1&num=2}, auto_link(link_raw, :sanitize => false) end def test_auto_link_other_protocols -- 1.7.1