Regressions in Loofah 2.9.0 and 2.9.1

[jacobherrington]

Hey @flavorjones 👋

Thanks for the work you put into maintaining this project, it's really cool.

I was looking into the test failures over on https://github.com/rails/rails-html-sanitizer, and noticed that both failures coincided with the changes in #200.

One of the changes was easy to address (some whitespace is now stripped that wasn't being stripped in 2.8.0), but the other failing test looks like it might indicate a slightly more complex bug.

Here are some reproduction steps:

# with Loofah 2.9.0
require "loofah"
Loofah::VERSION
# => "2.9.0"
input = %(background-image:\0075\0072\006C\0028'\006a\0061\0076\0061\0073\0063\0072\0069\0070\0074\003a\0061\006c\0065\0072\0074\0028.1027\0058.1053\0053\0027\0029'\0029)
Loofah::HTML5::Scrub.scrub_css(input)
# => "background-image:\a 5 \a 2 \x06 \x02 8 \x02 9;"

# with Loofah 2.8.0
require "loofah"
Loofah::VERSION
# => "2.8.0"
input = %(background-image:\0075\0072\006C\0028'\006a\0061\0076\0061\0073\0063\0072\0069\0070\0074\003a\0061\006c\0065\0072\0074\0028.1027\0058.1053\0053\0027\0029'\0029)
Loofah::HTML5::Scrub.scrub_css(input)
=> ""

# for good measure, I checked with Loofah 2.9.1 too!
# it's quite a bit different, which might be a clue :)
require "loofah"
Loofah::VERSION
# => "2.9.1"
input = %(background-image:\0075\0072\006C\0028'\006a\0061\0076\0061\0073\0063\0072\0069\0070\0074\003a\0061\006c\0065\0072\0074\0028.1027\0058.1053\0053\0027\0029'\0029)
Loofah::HTML5::Scrub.scrub_css(input)
# => "background-image:\a 5 \a 2 \x06 \x02 8 '\x06a\x061\a6\x061\a3\x063\a2\x069\a0\a4\x03a\x061\x06c\x065\a2\a4\x028.1027\x058.1053\x053\x027\x029' \x02 9;"

You can also take a look at the issue someone opened in rails-html-sanitizer if it's helpful.

I can look into this behavior as time permits, but this week is the first time I've ever looked at the project, so you (or another contributor) might have better luck!

[flavorjones]

Thanks for opening this ticket! I'll take a look.

[flavorjones]

Hi @jacobherrington. Please see rails/rails-html-sanitizer#113 for my fix to the rails-html-sanitizer tests. See also #205 for a brief history of that encoding test.

The TL;DR is that this isn't a regression, it's Loofah doing the right thing; and the test is invalid (but fixed in that PR).

Thanks again!