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

Regression in 2.9.0: string css attributes scrubbing #202

Closed
aert opened this issue Apr 7, 2021 · 5 comments · Fixed by #203
Closed

Regression in 2.9.0: string css attributes scrubbing #202

aert opened this issue Apr 7, 2021 · 5 comments · Fixed by #203

Comments

@aert
Copy link

aert commented Apr 7, 2021

This commit bf13d48 seem to scrub all css attribute of type "string", which impact font-family values that are not single word.

For example, the two examples in #130 don't work anymore.

@flavorjones
Copy link
Owner

Thanks for reporting this! I'll take a look.

@flavorjones
Copy link
Owner

Using this as the test script:

#!/usr/bin/env ruby

require 'loofah'

text = <<~EOF
<span style="font-size: 36px; font-family: 'AvenirNext-Regular';">This style gets stripped</span>
<span style="font-size: 36px; font-family: 'Avenir Next';">This style does not get stripped</span>
EOF

puts Loofah::VERSION
puts Loofah.fragment(text).scrub!(:strip)
puts Loofah.fragment(text).scrub!(:prune)

I git-bisected and agree that this commit introduced the issue:

bf13d48b7428aa36ee9dff083d017399249b2d60 is the first bad commit
commit bf13d48b7428aa36ee9dff083d017399249b2d60
Author: Mike Dalessio <mike.dalessio@gmail.com>
Date:   Wed Jan 13 13:44:19 2021 -0500

    fix: handle CSS functions in a CSS shorthand property
    
    Fixes #199.

 lib/loofah/html5/scrub.rb                 | 63 ++++++++++++++++++++-----------
 test/assets/testdata_sanitizer_tests1.dat |  8 ++--
 test/html5/test_scrub.rb                  | 10 -----
 test/html5/test_scrub_css.rb              | 32 ++++++++++++++++
 4 files changed, 76 insertions(+), 37 deletions(-)
 delete mode 100644 test/html5/test_scrub.rb
 create mode 100644 test/html5/test_scrub_css.rb

@flavorjones
Copy link
Owner

PR in #203

@flavorjones
Copy link
Owner

v2.9.1 has been released fixing this issue. Thanks again for reporting it!

@aert
Copy link
Author

aert commented Apr 8, 2021

@flavorjones No thank you, you're the one doing the great work here : ).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants