From d30bf84ef08e53f9b3ef65f128925f365914926b Mon Sep 17 00:00:00 2001
From: Mike Dalessio <mike.dalessio@gmail.com>
Date: Thu, 9 Dec 2021 21:32:44 -0500
Subject: [PATCH] fix: comments should not be emitted by DocumentFragment#text

Previously any comment nodes that were top-level children of the
fragment were serialized.

Closes #221
---
 CHANGELOG.md                   |  7 +++++++
 lib/loofah/instance_methods.rb |  6 +++++-
 test/integration/test_html.rb  | 34 ++++++++++++++++++++++++++--------
 3 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index bfa408f6..bc763ab6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,12 @@
 # Changelog
 
+## next / unreleased
+
+### Bug fixes
+
+* Loofah::HTML::DocumentFragment#text no longer serializes top-level comment children. [[#221](https://github.com/flavorjones/loofah/issues/221)]
+
+
 ## 2.12.0 / 2021-08-11
 
 ### Features
diff --git a/lib/loofah/instance_methods.rb b/lib/loofah/instance_methods.rb
index 8b49dbb6..316ac9d2 100644
--- a/lib/loofah/instance_methods.rb
+++ b/lib/loofah/instance_methods.rb
@@ -93,7 +93,11 @@ module TextBehavior
     #    frag.text(:encode_special_chars => false) # => "<script>alert('EVIL');</script>"
     #
     def text(options = {})
-      result = serialize_root.children.inner_text rescue ""
+      result = if serialize_root
+        serialize_root.children.reject(&:comment?).map(&:inner_text).join("")
+      else
+        ""
+      end
       if options[:encode_special_chars] == false
         result # possibly dangerous if rendered in a browser
       else
diff --git a/test/integration/test_html.rb b/test/integration/test_html.rb
index ab9746eb..8ae60a8f 100644
--- a/test/integration/test_html.rb
+++ b/test/integration/test_html.rb
@@ -3,18 +3,36 @@
 class IntegrationTestHtml < Loofah::TestCase
   context "html fragment" do
     context "#to_s" do
-      it "not include head tags (like style)" do
-        skip "depends on nokogiri version"
-        html = Loofah.fragment "<style>foo</style><div>bar</div>"
-        assert_equal "<div>bar</div>", html.to_s
+      it "includes header tags (like style)" do
+        html = "<style>foo</style><div>bar</div>"
+        expected = "<style>foo</style><div>bar</div>"
+        assert_equal(expected, Loofah.fragment(html).to_s)
+
+        # assumption check is that Nokogiri does the same
+        assert_equal(expected, Nokogiri::HTML4::DocumentFragment.parse(html).to_s)
+        assert_equal(expected, Nokogiri::HTML5::DocumentFragment.parse(html).to_s)
       end
     end
 
     context "#text" do
-      it "not include head tags (like style)" do
-        skip "depends on nokogiri version"
-        html = Loofah.fragment "<style>foo</style><div>bar</div>"
-        assert_equal "bar", html.text
+      it "includes header tags (like style)" do
+        html = "<style>foo</style><div>bar</div>"
+        expected = "foobar"
+        assert_equal(expected, Loofah.fragment(html).text)
+
+        # assumption check is that Nokogiri does the same
+        assert_equal(expected, Nokogiri::HTML4::DocumentFragment.parse(html).text)
+        assert_equal(expected, Nokogiri::HTML5::DocumentFragment.parse(html).text)
+      end
+
+      it "does not include cdata tags (like comments)" do
+        html = "<div>bar<!-- comment1 --></div><!-- comment2 -->"
+        expected = "bar"
+        assert_equal(expected, Loofah.fragment(html).text)
+
+        # assumption check is that Nokogiri does the same
+        assert_equal(expected, Nokogiri::HTML4::DocumentFragment.parse(html).text)
+        assert_equal(expected, Nokogiri::HTML5::DocumentFragment.parse(html).text)
       end
     end