From a2edfdc5d2ba4da3c5d0d3cfef6b3479bd9f1a11 Mon Sep 17 00:00:00 2001
From: Mike Dalessio <mike.dalessio@gmail.com>
Date: Tue, 13 Aug 2024 10:49:27 -0400
Subject: [PATCH 1/2] Do not scrub attributes on a removed node

This should improve performance by eliminating unneeded scrubbing of attributes.
---
 lib/rails/html/scrubbers.rb |  3 ++-
 test/scrubbers_test.rb      | 48 +++++++++++++++++++++++++++++++++++--
 2 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/lib/rails/html/scrubbers.rb b/lib/rails/html/scrubbers.rb
index a1317ad..396eed0 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
diff --git a/test/scrubbers_test.rb b/test/scrubbers_test.rb
index 8db2d85..d512213 100644
--- a/test/scrubbers_test.rb
+++ b/test/scrubbers_test.rb
@@ -207,11 +207,55 @@ 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 "<script>remove me</script>"
   end
+
+  def test_returns_continue_from_scrub_if_scrub_node_does
+    @scrubber = ScrubContinuer.new
+    assert_node_skipped "<script>keep 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

From b4efdad8bde7809b0d514ab27ab839a2a561c64d Mon Sep 17 00:00:00 2001
From: Mike Dalessio <mike.dalessio@gmail.com>
Date: Tue, 13 Aug 2024 10:50:11 -0400
Subject: [PATCH 2/2] Do not scrub removed attributes

This should improve performance by eliminating needless sanitization
of attributes that are already removed.
---
 lib/rails/html/scrubbers.rb |  7 +++++--
 test/scrubbers_test.rb      | 10 ++++++++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/lib/rails/html/scrubbers.rb b/lib/rails/html/scrubbers.rb
index 396eed0..6182abb 100644
--- a/lib/rails/html/scrubbers.rb
+++ b/lib/rails/html/scrubbers.rb
@@ -108,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 d512213..b0529ea 100644
--- a/test/scrubbers_test.rb
+++ b/test/scrubbers_test.rb
@@ -249,6 +249,16 @@ def scrub_attribute(node, attr)
     end
   end
 
+  def test_does_not_scrub_removed_attributes
+    @scrubber = TestPermitScrubber.new
+
+    input = "<div class='foo' href='bar'></div>"
+    frag = scrub_fragment(input)
+    assert_equal("<div class=\"foo\"></div>", 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