Ok, now #text properly encodes HTML entities to avoid the problem sta…

…ted in GH #17. Supersedes the fix from 03e781b

Manifest.txt

@@ -25,10 +25,12 @@ lib/loofah/xml/document_fragment.rb
 lib/loofah/xss_foliate.rb
 test/helper.rb
 test/html5/test_sanitizer.rb
-test/test_active_record.rb
-test/test_ad_hoc.rb
-test/test_api.rb
-test/test_helpers.rb
-test/test_scrubber.rb
-test/test_scrubbers.rb
-test/test_xss_foliate.rb
+test/integration/test_ad_hoc.rb
+test/integration/test_helpers.rb
+test/integration/test_scrubbers.rb
+test/unit/test_active_record.rb
+test/unit/test_api.rb
+test/unit/test_helpers.rb
+test/unit/test_scrubber.rb
+test/unit/test_scrubbers.rb
+test/unit/test_xss_foliate.rb

lib/loofah/helpers.rb

@@ -7,7 +7,7 @@ class << self
       #   Loofah::Helpers.strip_tags("<div>Hello <b>there</b></div>") # => "Hello there"
       #
       def strip_tags(string_or_io)
-        Loofah.fragment(string_or_io).to_s
+        Loofah.fragment(string_or_io).text
       end
 
       #

lib/loofah/html/document.rb

@@ -10,10 +10,11 @@ class Document < Nokogiri::HTML::Document
       include Loofah::DocumentDecorator
 
       #
-      #  Returns a plain-text version of the markup contained by the document
+      #  Returns a plain-text version of the markup contained by the document,
+      #  with HTML entities encoded.
       #
       def text
-        xpath("/html/body").inner_text
+        encode_special_chars xpath("/html/body").inner_text
       end
       alias :inner_text :text
       alias :to_str     :text

lib/loofah/html/document_fragment.rb

@@ -29,7 +29,7 @@ def to_s
       #  Returns a plain-text version of the markup contained by the fragment
       #
       def text
-        serialize_roots.children.inner_text
+        encode_special_chars serialize_roots.children.inner_text
       end
       alias :inner_text :text
       alias :to_str     :text

test/integration/test_helpers.rb

@@ -0,0 +1,33 @@
+require File.expand_path(File.join(File.dirname(__FILE__), '..', 'helper'))
+
+class TestHelpers < Test::Unit::TestCase
+  context "#strip_tags" do
+    context "on safe markup" do
+      should "strip out tags" do
+        assert_equal "omgwtfbbq!!1!", Loofah::Helpers.strip_tags("<div>omgwtfbbq</div><span>!!1!</span>")
+      end
+    end
+
+    context "on hack attack" do
+      should "strip escape html entities" do
+        bad_shit = "&lt;script&gt;alert('evil')&lt;/script&gt;"
+        assert_equal bad_shit, Loofah::Helpers.strip_tags(bad_shit)
+      end
+    end
+  end
+
+  context "#sanitize" do
+    context "on safe markup" do
+      should "render the safe html" do
+        html = "<div>omgwtfbbq</div><span>!!1!</span>"
+        assert_equal html, Loofah::Helpers.sanitize(html)
+      end
+    end
+
+    context "on hack attack" do
+      should "strip the unsafe tags" do
+        assert_equal "alert('evil')<span>w00t</span>", Loofah::Helpers.sanitize("<script>alert('evil')</script><span>w00t</span>")
+      end
+    end
+  end
+end

test/integration/test_scrubbers.rb

@@ -13,8 +13,11 @@ class TestScrubbers < Test::Unit::TestCase
   NOFOLLOW_FRAGMENT = '<a href="http://www.example.com/">Click here</a>'
   NOFOLLOW_RESULT   = '<a href="http://www.example.com/" rel="nofollow">Click here</a>'
 
-  ENTITY_FRAGMENT   = "<p>this is &lt; that &quot;&amp;&quot; the other &gt; boo&apos;ya</p>"
-  ENTITY_TEXT       = %Q(this is < that "&" the other > boo\'ya)
+  ENTITY_FRAGMENT   = "<p>this is &lt; that &quot;&amp;&quot; the other &gt; boo&apos;ya</p><div>w00t</div>"
+  ENTITY_TEXT       = %Q(this is < that "&" the other > boo\'yaw00t)
+
+  ENTITY_HACK_ATTACK            = "<div><div>Hack attack!</div><div>&lt;script&gt;alert('evil')&lt;/script&gt;</div></div>"
+  ENTITY_HACK_ATTACK_TEXT_SCRUB = "Hack attack!&lt;script&gt;alert('evil')&lt;/script&gt;"
 
   context "Document" do
     context "#scrub!" do
@@ -79,6 +82,15 @@ class TestScrubbers < Test::Unit::TestCase
       end
     end
 
+    context "#text" do
+      should "leave behind only inner text with html entities still escaped" do
+        doc = Loofah::HTML::Document.parse "<html><body>#{ENTITY_HACK_ATTACK}</body></html>"
+        result = doc.text
+
+        assert_equal ENTITY_HACK_ATTACK_TEXT_SCRUB, result
+      end
+    end
+
     context "#to_s" do
       should "generate HTML" do
         doc = Loofah.scrub_document "<html><head><title>quux</title></head><body><div>foo</div></body></html>", :prune
@@ -221,8 +233,11 @@ class TestScrubbers < Test::Unit::TestCase
     end
 
     context "#text" do
-      should "remove entities" do
-        assert_equal ENTITY_TEXT, Loofah.scrub_fragment(ENTITY_FRAGMENT, :prune).text
+      should "leave behind only inner text with html entities still escaped" do
+        doc = Loofah::HTML::DocumentFragment.parse "<div>#{ENTITY_HACK_ATTACK}</div>"
+        result = doc.text
+
+        assert_equal ENTITY_HACK_ATTACK_TEXT_SCRUB, result
       end
     end
 

test/unit/test_helpers.rb

@@ -4,18 +4,18 @@ class TestHelpers < Test::Unit::TestCase
 
   HTML_STRING = "<div>omgwtfbbq</div>"
 
-  context "when calling strip_tags" do
-    should "invoke Loofah.fragment.to_s" do
+  context "#strip_tags" do
+    should "invoke Loofah.fragment.text" do
       mock_doc = mock
       Loofah.expects(:fragment).with(HTML_STRING).returns(mock_doc)
-      mock_doc.expects(:to_s)
+      mock_doc.expects(:text)
 
       Loofah::Helpers.strip_tags HTML_STRING
     end
   end
 
-  context "when calling sanitize" do
-    should "invoke Loofah.scrub_fragment(:escape).to_s" do
+  context "#sanitize" do
+    should "invoke Loofah.scrub_fragment(:strip).to_s" do
       mock_doc = mock
       Loofah.expects(:fragment).with(HTML_STRING).returns(mock_doc)
       mock_doc.expects(:scrub!).with(:strip).returns(mock_doc)