Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow CSS properties to have quoted string values #203

Merged
merged 1 commit into from Apr 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 13 additions & 2 deletions CHANGELOG.md
@@ -1,11 +1,22 @@
# Changelog

### 2.9.0 / 2021-01-14
### next / unreleased

### Bug fixes

* Fix a regression in v2.9.0 which inappropriately removed CSS properties with quoted string values. [[#202](https://github.com/flavorjones/loofah/issues/202)]


## 2.9.0 / 2021-01-14

### Features

* Handle CSS functions in a CSS shorthand property (like `background`). [[#199](https://github.com/flavorjones/loofah/issues/199), [#200](https://github.com/flavorjones/loofah/issues/200)]


### 2.8.0 / 2020-11-25
## 2.8.0 / 2020-11-25

### Features

* Allow CSS properties `order`, `flex-direction`, `flex-grow`, `flex-wrap`, `flex-shrink`, `flex-flow`, `flex-basis`, `flex`, `justify-content`, `align-self`, `align-items`, and `align-content`. [[#197](https://github.com/flavorjones/loofah/issues/197)] (Thanks, [@miguelperez](https://github.com/miguelperez)!)

Expand Down
7 changes: 6 additions & 1 deletion lib/loofah/html5/scrub.rb
Expand Up @@ -9,6 +9,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/
CRASS_SEMICOLON = { node: :semicolon, raw: ";" }
CSS_IMPORTANT = '!important'
CSS_PROPERTY_STRING_WITHOUT_EMBEDDED_QUOTES = /\A(["'])?[^"']+\1\z/

class << self
def allowed_element?(element_name)
Expand Down Expand Up @@ -92,7 +93,11 @@ def scrub_css(style)
when :whitespace
nil
when :string
nil
if child[:raw] =~ CSS_PROPERTY_STRING_WITHOUT_EMBEDDED_QUOTES
Crass::Parser.stringify(child)
else
nil
end
when :function
if SafeList::ALLOWED_CSS_FUNCTIONS.include?(child[:name].downcase)
Crass::Parser.stringify(child)
Expand Down
29 changes: 29 additions & 0 deletions test/html5/test_scrub_css.rb
Expand Up @@ -29,4 +29,33 @@ class UnitHTML5Scrub < Loofah::TestCase
Loofah::HTML5::Scrub.scrub_css("background: linear-gradient(transparent 50%, #ffff66 50%);")
end
end

describe "property string values" do
it "allows hypenated values" do
text = %q(font-family:'AvenirNext-Regular';)
assert_equal(text, Loofah::HTML5::Scrub.scrub_css(text))

text = %q(font-family:"AvenirNext-Regular";)
assert_equal(text, Loofah::HTML5::Scrub.scrub_css(text))
end

it "allows embedded spaces in values" do
text = %q(font-family:'Avenir Next';)
assert_equal(text, Loofah::HTML5::Scrub.scrub_css(text))

text = %q(font-family:"Avenir Next";)
assert_equal(text, Loofah::HTML5::Scrub.scrub_css(text))
end

it "does not allow values with embedded or irregular quotes" do
assert_empty(Loofah::HTML5::Scrub.scrub_css(%q(font-family:'AvenirNext"-Regular';)))
assert_empty(Loofah::HTML5::Scrub.scrub_css(%q(font-family:"AvenirNext'-Regular";)))

assert_empty(Loofah::HTML5::Scrub.scrub_css(%q(font-family:'AvenirNext-Regular;)))
assert_empty(Loofah::HTML5::Scrub.scrub_css(%q(font-family:'AvenirNext-Regular";)))

assert_empty(Loofah::HTML5::Scrub.scrub_css(%q(font-family:"AvenirNext-Regular;)))
assert_empty(Loofah::HTML5::Scrub.scrub_css(%q(font-family:"AvenirNext-Regular';)))
end
end
end
20 changes: 20 additions & 0 deletions test/integration/test_ad_hoc.rb
Expand Up @@ -260,5 +260,25 @@ def test_dont_remove_whitespace_between_tags
end
end
end

it "handles property string values" do
input = <<~EOF
<span style="font-size: 36px; font-family: 'AvenirNext-Regular';">variation 1a</span>
<span style="font-size: 36px; font-family: AvenirNext-Regular;">variation 1b</span>
<span style="font-size: 36px; font-family: 'Avenir Next';">variation 2a</span>
<span style="font-size: 36px; font-family: Avenir Next;">variation 2b</span>
EOF

expected = <<~EOF
<span style="font-size:36px;font-family:'AvenirNext-Regular';">variation 1a</span>
<span style="font-size:36px;font-family:AvenirNext-Regular;">variation 1b</span>
<span style="font-size:36px;font-family:'Avenir Next';">variation 2a</span>
<span style="font-size:36px;font-family:Avenir Next;">variation 2b</span>
EOF

actual = Loofah.scrub_fragment(input, :escape)

assert_equal(expected, actual.to_html)
end
end
end