diff --git a/CHANGELOG.md b/CHANGELOG.md index 58ae8ff..ca593b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +## next / unreleased + +* Slightly improve performance. + + Assuming elements are more common than comments, make one less method call per node. + + *Mike Dalessio* + ## 1.4.1 / 2021-08-18 * Fix regression in v1.4.0 that did not pass comment nodes to the scrubber. diff --git a/lib/rails/html/scrubbers.rb b/lib/rails/html/scrubbers.rb index 385d357..09cfe95 100644 --- a/lib/rails/html/scrubbers.rb +++ b/lib/rails/html/scrubbers.rb @@ -68,7 +68,7 @@ def scrub(node) end return CONTINUE if skip_node?(node) - unless (node.comment? || node.element?) && keep_node?(node) + unless (node.element? || node.comment?) && keep_node?(node) return STOP if scrub_node(node) == STOP end diff --git a/test/scrubbers_test.rb b/test/scrubbers_test.rb index e6612e9..a825404 100644 --- a/test/scrubbers_test.rb +++ b/test/scrubbers_test.rb @@ -41,6 +41,16 @@ def test_default_scrub_behavior assert_scrubbed 'hello', 'hello' end + def test_default_scrub_removes_comments + assert_scrubbed('
one
three', + '
one
three') + end + + def test_default_scrub_removes_processing_instructions + assert_scrubbed('
one
three', + '
one
three') + end + def test_default_attributes_removal_behavior assert_scrubbed '

hello

', '

hello

' end @@ -56,6 +66,12 @@ def test_leaves_only_supplied_tags assert_scrubbed html, 'leave me now' end + def test_leaves_comments_when_supplied_as_tag + @scrubber.tags = %w(div comment) + assert_scrubbed('
one
three', + '
one
three') + end + def test_leaves_only_supplied_tags_nested html = 'leave me now' @scrubber.tags = %w(tag) @@ -112,50 +128,6 @@ def test_attributes_accessor_validation end end -class PermitScrubberSubclassTest < ScrubberTest - def setup - @scrubber = Class.new(::Rails::Html::PermitScrubber) do - attr :nodes_seen - - def initialize - super() - @nodes_seen = [] - end - - def keep_node?(node) - @nodes_seen << node.name - super(node) - end - end.new - end - - def test_elements_are_checked - html = %Q("
") - Loofah.scrub_fragment(html, @scrubber) - assert_includes(@scrubber.nodes_seen, "div") - assert_includes(@scrubber.nodes_seen, "a") - assert_includes(@scrubber.nodes_seen, "tr") - end - - def test_comments_are_checked - # this passes in v1.3.0 but fails in v1.4.0 - html = %Q("
") - Loofah.scrub_fragment(html, @scrubber) - assert_includes(@scrubber.nodes_seen, "div") - assert_includes(@scrubber.nodes_seen, "comment") - assert_includes(@scrubber.nodes_seen, "tr") - end - - def test_craftily_named_processing_instructions_are_not_checked - # this fails in v1.3.0 but passes in v1.4.0 - html = %Q("
") - Loofah.scrub_fragment(html, @scrubber) - assert_includes(@scrubber.nodes_seen, "div") - refute_includes(@scrubber.nodes_seen, "a") - assert_includes(@scrubber.nodes_seen, "tr") - end -end - class TargetScrubberTest < ScrubberTest def setup @scrubber = Rails::Html::TargetScrubber.new