Skip to content

Commit

Permalink
fix: comments should not be emitted by DocumentFragment#text
Browse files Browse the repository at this point in the history
Previously any comment nodes that were top-level children of the
fragment were serialized.

Closes #221
  • Loading branch information
flavorjones committed Dec 10, 2021
1 parent 483a7e7 commit d30bf84
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 9 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
6 changes: 5 additions & 1 deletion lib/loofah/instance_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 26 additions & 8 deletions test/integration/test_html.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit d30bf84

Please sign in to comment.