Merge pull request #273 from flavorjones/flavorjones-css-whitespace-h…

…andling fix: scrub_css is more consistent with whitespace
flavorjones · Oct 10, 2023 · 5d9a22f · 5d9a22f
2 parents edde5f2 + 876116e
commit 5d9a22f
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 4 deletions.
diff --git a/lib/loofah/html5/scrub.rb b/lib/loofah/html5/scrub.rb
@@ -10,6 +10,7 @@ module Scrub
       CSS_KEYWORDISH = /\A(#[0-9a-fA-F]+|rgb\(\d+%?,\d*%?,?\d*%?\)?|-?\d{0,3}\.?\d{0,10}(ch|cm|r?em|ex|in|lh|mm|pc|pt|px|Q|vmax|vmin|vw|vh|%|,|\))?)\z/ # rubocop:disable Layout/LineLength
       CRASS_SEMICOLON = { node: :semicolon, raw: ";" }
       CSS_IMPORTANT = "!important"
+      CSS_WHITESPACE = " "
       CSS_PROPERTY_STRING_WITHOUT_EMBEDDED_QUOTES = /\A(["'])?[^"']+\1\z/
       DATA_ATTRIBUTE_NAME = /\Adata-[\w-]+\z/
 
@@ -87,7 +88,7 @@ def scrub_css(style)
             value = node[:children].map do |child|
               case child[:node]
               when :whitespace
-                nil
+                CSS_WHITESPACE
               when :string
                 if CSS_PROPERTY_STRING_WITHOUT_EMBEDDED_QUOTES.match?(child[:raw])
                   Crass::Parser.stringify(child)
@@ -106,12 +107,12 @@ def scrub_css(style)
               else
                 child[:raw]
               end
-            end.compact
+            end.compact.join.strip
 
             next if value.empty?
 
-            value << CSS_IMPORTANT if node[:important]
-            propstring = format("%s:%s", name, value.join(" "))
+            value << CSS_WHITESPACE << CSS_IMPORTANT if node[:important]
+            propstring = format("%s:%s", name, value)
             sanitized_node = Crass.parse_properties(propstring).first
             sanitized_tree << sanitized_node << CRASS_SEMICOLON
           end

diff --git a/test/html5/test_scrub_css.rb b/test/html5/test_scrub_css.rb
@@ -69,6 +69,26 @@ class UnitHTML5Scrub < Loofah::TestCase
       assert_empty(Loofah::HTML5::Scrub.scrub_css('font-family:"AvenirNext-Regular;'))
       assert_empty(Loofah::HTML5::Scrub.scrub_css(%q(font-family:"AvenirNext-Regular';)))
     end
+
+    it "keeps whitespace around delimiters if it's already there" do
+      assert_equal(
+        "font:13px / 1.5 Arial , sans-serif;",
+        Loofah::HTML5::Scrub.scrub_css("font: 13px / 1.5 Arial , sans-serif;"),
+      )
+    end
+
+    it "does not insert spaces around delimiters if they aren't already there" do
+      assert_equal(
+        "font:13px/1.5 Arial, sans-serif;",
+        Loofah::HTML5::Scrub.scrub_css("font: 13px/1.5 Arial, sans-serif;"),
+      )
+    end
+  end
+
+  describe "whitespace" do
+    it "converts all whitespace to a single space except initial and final whitespace" do
+      assert_equal("font:12px Arial;", Loofah::HTML5::Scrub.scrub_css("font: \n\t 12px \n\t Arial \n\t ;"))
+    end
   end
 
   describe "colors" do