Test that the block used in truncate is escaped if it is not HTML safe

Refactoring the truncate method to not do a sort-circuit return

2 files changed, 23 insertions, 25 deletions

diff --git a/actionpack/lib/action_view/helpers/text_helper.rb b/actionpack/lib/action_view/helpers/text_helper.rb
index 515bd78101..73c3c61a41 100644
--- a/actionpack/lib/action_view/helpers/text_helper.rb
+++ b/actionpack/lib/action_view/helpers/text_helper.rb
@@ -84,14 +84,13 @@ module ActionView
       #   truncate("Once upon a time in a world far far away") { link_to "Continue", "#" }
       #   # => "Once upon a time in a wo...<a href="#">Continue</a>"
       def truncate(text, options = {}, &block)
-        return unless text
+        if text
+          length  = options.fetch(:length, 30)
 
-        options = { :length => 30 }.merge!(options)
-        length  = options.delete(:length)
-
-        content = ERB::Util.html_escape(text.truncate(length, options))
-        content << capture(&block) if block_given? && text.length > length
-        content
+          content = ERB::Util.html_escape(text.truncate(length, options))
+          content << capture(&block) if block_given? && text.length > length
+          content
+        end
       end
 
       # Highlights one or more +phrases+ everywhere in +text+ by inserting it into
@@ -112,7 +111,7 @@ module ActionView
       #   # => You searched for: <a href="search?q=rails">rails</a>
       def highlight(text, phrases, options = {})
         highlighter = options.fetch(:highlighter, '<mark>\1</mark>')
-        
+
         text = sanitize(text) if options.fetch(:sanitize, true)
         if text.blank? || phrases.blank?
           text
@@ -175,12 +174,12 @@ module ActionView
       #   pluralize(0, 'person')
       #   # => 0 people
       def pluralize(count, singular, plural = nil)
-        word = if (count == 1 || count =~ /^1(\.0+)?$/) 
-          singular 
+        word = if (count == 1 || count =~ /^1(\.0+)?$/)
+          singular
         else
           plural || singular.pluralize
         end
-        
+
         "#{count || 0} #{word}"
       end
 
@@ -225,7 +224,7 @@ module ActionView
       #
       #   simple_format(my_text)
       #   # => "<p>Here is some basic text...\n<br />...with a line break.</p>"
-      # 
+      #
       #   simple_format(my_text, {}, :wrapper_tag => "div")
       #   # => "<div>Here is some basic text...\n<br />...with a line break.</div>"
       #
@@ -241,7 +240,7 @@ module ActionView
       #   # => "<p><span>I'm allowed!</span> It's true.</p>"
       def simple_format(text, html_options = {}, options = {})
         wrapper_tag = options.fetch(:wrapper_tag, :p)
-        
+
         text = sanitize(text) if options.fetch(:sanitize, true)
         paragraphs = split_paragraphs(text)
 
diff --git a/actionpack/test/template/text_helper_test.rb b/actionpack/test/template/text_helper_test.rb
index a7333a3af9..4b1c1ef78b 100644
--- a/actionpack/test/template/text_helper_test.rb
+++ b/actionpack/test/template/text_helper_test.rb
@@ -60,14 +60,14 @@ class TextHelperTest < ActionView::TestCase
     simple_format(text)
     assert_equal text_clone, text
   end
-  
+
   def test_simple_format_does_not_modify_the_html_options_hash
     options = { :class => "foobar"}
     passed_options = options.dup
     simple_format("some text", passed_options)
     assert_equal options, passed_options
   end
-  
+
   def test_simple_format_does_not_modify_the_options_hash
     options = { :wrapper_tag => :div, :sanitize => false }
     passed_options = options.dup
@@ -98,7 +98,7 @@ class TextHelperTest < ActionView::TestCase
     assert_equal "\354\225\204\353\246\254\353\236\221 \354\225\204\353\246\254 ...".force_encoding('UTF-8'),
       truncate("\354\225\204\353\246\254\353\236\221 \354\225\204\353\246\254 \354\225\204\353\235\274\353\246\254\354\230\244".force_encoding('UTF-8'), :length => 10)
   end
-  
+
   def test_truncate_does_not_modify_the_options_hash
     options = { :length => 10 }
     passed_options = options.dup
@@ -111,12 +111,6 @@ class TextHelperTest < ActionView::TestCase
     truncate("Here's a long test and I need a continue to read link", :length => 27) { link_to 'Continue', '#' }
   end
 
-  def test_truncate_should_not_mutate_the_options_hash
-    options = { :length => 27 }
-    truncate("Here's a long test and I need a continue to read link", options) { link_to 'Continue', '#' }
-    assert_equal({ :length => 27 }, options)
-  end
-
   def test_truncate_should_be_html_safe
     assert truncate("Hello World!", :length => 12).html_safe?
   end
@@ -135,6 +129,11 @@ class TextHelperTest < ActionView::TestCase
       truncate("<script>code!</script>Here's a long test and I need a continue to read link", :length => 27) { link_to 'Continue', '#' }
   end
 
+  def test_truncate_with_block_should_escape_the_block
+    assert_equal "Here's a long test and I...&lt;script&gt;alert('foo');&lt;/script&gt;",
+      truncate("Here's a long test and I need a continue to read link", :length => 27) { "<script>alert('foo');</script>" }
+  end
+
   def test_highlight_should_be_html_safe
     assert highlight("This is a beautiful morning", "beautiful").html_safe?
   end
@@ -224,7 +223,7 @@ class TextHelperTest < ActionView::TestCase
       highlight("<div>abc div</div>", "div", :highlighter => '<b>\1</b>')
     )
   end
-  
+
   def test_highlight_does_not_modify_the_options_hash
     options = { :highlighter => '<b>\1</b>', :sanitize => false }
     passed_options = options.dup
@@ -277,7 +276,7 @@ class TextHelperTest < ActionView::TestCase
   def test_excerpt_with_utf8
     assert_equal("...\357\254\203ciency could not be...".force_encoding('UTF-8'), excerpt("That's why e\357\254\203ciency could not be helped".force_encoding('UTF-8'), 'could', :radius => 8))
   end
-  
+
   def test_excerpt_does_not_modify_the_options_hash
     options = { :omission => "[...]",:radius => 5 }
     passed_options = options.dup
@@ -292,7 +291,7 @@ class TextHelperTest < ActionView::TestCase
   def test_word_wrap_with_extra_newlines
     assert_equal("my very very\nvery long\nstring\n\nwith another\nline", word_wrap("my very very very long string\n\nwith another line", :line_width => 15))
   end
-  
+
   def test_word_wrap_does_not_modify_the_options_hash
     options = { :line_width => 15 }
     passed_options = options.dup