Skip to content

Commit

Permalink
Do not scrub attributes on a removed node
Browse files Browse the repository at this point in the history
This should improve performance by eliminating unneeded scrubbing of attributes.
  • Loading branch information
flavorjones committed Aug 13, 2024
1 parent c5734e5 commit 2eaa944
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 1 deletion.
3 changes: 2 additions & 1 deletion lib/rails/html/scrubbers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ def scrub(node)
return CONTINUE if skip_node?(node)

unless (node.element? || node.comment?) && keep_node?(node)
return STOP if scrub_node(node) == STOP
scrub_node(node)
return STOP
end

scrub_attributes(node)
Expand Down
36 changes: 36 additions & 0 deletions test/scrubbers_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -215,3 +215,39 @@ def test_returns_stop_from_scrub_if_scrub_node_does
assert_scrub_stopped "<script>remove me</script>"
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_attributes_of_a_removed_node
@scrubber = TestPermitScrubber.new

input = "<div class='foo' href='bar'><svg xlink:href='asdf'><set></set></svg></div>"
frag = scrub_fragment(input)
assert_equal("<div class=\"foo\"></div>", frag)

assert_equal(["div"], @scrubber.instance_variable_get(:@scrub_attributes_args))
end
end

0 comments on commit 2eaa944

Please sign in to comment.