diff --git a/lib/rails/html/scrubbers.rb b/lib/rails/html/scrubbers.rb index a1317ad..6182abb 100644 --- a/lib/rails/html/scrubbers.rb +++ b/lib/rails/html/scrubbers.rb @@ -72,10 +72,11 @@ def scrub(node) return CONTINUE if skip_node?(node) unless (node.element? || node.comment?) && keep_node?(node) - return STOP if scrub_node(node) == STOP + return STOP unless scrub_node(node) == CONTINUE end scrub_attributes(node) + CONTINUE end protected @@ -107,8 +108,11 @@ def scrub_node(node) def scrub_attributes(node) if @attributes node.attribute_nodes.each do |attr| - attr.remove if scrub_attribute?(attr.name) - scrub_attribute(node, attr) + if scrub_attribute?(attr.name) + attr.remove + else + scrub_attribute(node, attr) + end end scrub_css_attribute(node) diff --git a/test/scrubbers_test.rb b/test/scrubbers_test.rb index 8db2d85..b0529ea 100644 --- a/test/scrubbers_test.rb +++ b/test/scrubbers_test.rb @@ -207,11 +207,65 @@ def scrub_node(node) end end - def setup - @scrubber = ScrubStopper.new + class ScrubContinuer < Rails::HTML::PermitScrubber + def scrub_node(node) + Loofah::Scrubber::CONTINUE + end end def test_returns_stop_from_scrub_if_scrub_node_does + @scrubber = ScrubStopper.new assert_scrub_stopped "" end + + def test_returns_continue_from_scrub_if_scrub_node_does + @scrubber = ScrubContinuer.new + assert_node_skipped "" + end +end + +class PermitScrubberMinimalOperationsTest < ScrubberTest + class TestPermitScrubber < Rails::HTML::PermitScrubber + def initialize + @scrub_attribute_args = [] + @scrub_attributes_args = [] + + super + + self.tags = ["div"] + self.attributes = ["class"] + end + + def scrub_attributes(node) + @scrub_attributes_args << node.name + + super + end + + def scrub_attribute(node, attr) + @scrub_attribute_args << [node.name, attr.name] + + super + end + end + + def test_does_not_scrub_removed_attributes + @scrubber = TestPermitScrubber.new + + input = "
" + frag = scrub_fragment(input) + assert_equal("
", frag) + + assert_equal([["div", "class"]], @scrubber.instance_variable_get(:@scrub_attribute_args)) + end + + def test_does_not_scrub_attributes_of_a_removed_node + @scrubber = TestPermitScrubber.new + + input = "
" + frag = scrub_fragment(input) + assert_equal("
", frag) + + assert_equal(["div"], @scrubber.instance_variable_get(:@scrub_attributes_args)) + end end