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

test failures against current loofah/nokogiri #111

Closed
terceiro opened this issue Feb 14, 2021 · 6 comments · Fixed by #113
Closed

test failures against current loofah/nokogiri #111

terceiro opened this issue Feb 14, 2021 · 6 comments · Fixed by #113

Comments

@terceiro
Copy link

$ bundle list
Gems included by the bundle:
  * activesupport (6.1.2.1)
  * concurrent-ruby (1.1.8)
  * crass (1.0.6)
  * i18n (1.8.9)
  * loofah (2.9.0)
  * mini_portile2 (2.5.0)
  * minitest (5.14.3)
  * nokogiri (1.11.1)
  * racc (1.5.2)
  * rails-dom-testing (2.0.3)
  * rails-html-sanitizer (1.3.0)
  * rake (13.0.3)
  * tzinfo (2.0.4)
  * zeitwerk (2.4.2)
Use `bundle info` to print more detailed information about a gem
$ bundle exec rake
/usr/lib/ruby-standalone/bin/ruby -w -I"lib" /home/terceiro/.ruby-standalone/gems/ruby/2.7.0/gems/rake-13.0.3/lib/rake/rake_test_loader.rb "test/sanitizer_test.rb" "test/scrubbers_test.rb" 
Run options: --seed 28994

# Running:

..........F.......................................................................F...............................................................................................................................................................................................................................

Finished in 0.130447s, 2345.7817 runs/s, 2476.1029 assertions/s.

  1) Failure:
SanitizersTest#test_should_sanitize_div_background_image_unicode_encoded [/tmp/rails-html-sanitizer/test/sanitizer_test.rb:417]:
--- expected
+++ actual
@@ -1,3 +1,3 @@
-# encoding: UTF-8
+# encoding: ASCII-8BIT
 #    valid: true
-""
+"background-image:\a 5 \a 2 \x06 \x02 8 \x02 9;"


  2) Failure:
SanitizersTest#test_scrub_style_if_style_attribute_option_is_passed [/tmp/rails-html-sanitizer/test/sanitizer_test.rb:274]:
--- expected
+++ actual
@@ -1,3 +1,3 @@
 # encoding: UTF-8
 #    valid: true
-"<p style=\"color: #000;\"></p>"
+"<p style=\"color:#000;\"></p>"


306 runs, 323 assertions, 2 failures, 0 errors, 0 skips
rake aborted!
Command failed with status (1): [ruby -w -I"lib" /home/terceiro/.ruby-standalone/gems/ruby/2.7.0/gems/rake-13.0.3/lib/rake/rake_test_loader.rb "test/sanitizer_test.rb" "test/scrubbers_test.rb" ]
/home/terceiro/.ruby-standalone/gems/ruby/2.7.0/gems/rake-13.0.3/exe/rake:27:in `<top (required)>'
Tasks: TOP => default => test
(See full trace by running task with --trace)

failure 2) is trivial to fix; but failure 1) is more involved. The project CI is currently broken with these exact same failures.

@boutil
Copy link

boutil commented Feb 19, 2021

using loofah 2.7.0 makes all the tests pass.

@Segaja
Copy link

Segaja commented Mar 14, 2021

Is it possible to fix that? I have similar errors when trying to package this for Archlinux:

/usr/bin/ruby -w -I"lib" /usr/lib/ruby/gems/2.7.0/gems/rake-13.0.3/lib/rake/rake_test_loader.rb "test/sanitizer_test.rb" "test/scrubbers_test.rb"
Run options: --seed 23644

# Running:

............................................F

Failure:
SanitizersTest#test_uri_escaping_of_src_attr_in_a_tag_in_safe_list_sanitizer [/build/ruby-rails-html-sanitizer/src/rails-html-sanitizer-1.3.0/test/sanitizer_test.rb:500]:
--- expected
+++ actual
@@ -1 +1 @@
-"<a src=\"examp&lt;!--%22%20unsafeattr=foo()&gt;--&gt;le.com\">test</a>"
+"<a src=\"examp<!--%22%20unsafeattr=foo()>-->le.com\">test</a>"


rails test build/ruby-rails-html-sanitizer/src/rails-html-sanitizer-1.3.0/test/sanitizer_test.rb:493

.........................................................................................................................................................F

Failure:
SanitizersTest#test_uri_escaping_of_name_attr_in_a_tag_in_safe_list_sanitizer [/build/ruby-rails-html-sanitizer/src/rails-html-sanitizer-1.3.0/test/sanitizer_test.rb:510]:
--- expected
+++ actual
@@ -1 +1 @@
-"<a name=\"examp&lt;!--%22%20unsafeattr=foo()&gt;--&gt;le.com\">test</a>"
+"<a name=\"examp<!--%22%20unsafeattr=foo()>-->le.com\">test</a>"


rails test build/ruby-rails-html-sanitizer/src/rails-html-sanitizer-1.3.0/test/sanitizer_test.rb:503

.......F

Failure:
SanitizersTest#test_uri_escaping_of_href_attr_in_a_tag_in_safe_list_sanitizer [/build/ruby-rails-html-sanitizer/src/rails-html-sanitizer-1.3.0/test/sanitizer_test.rb:490]:
--- expected
+++ actual
@@ -1 +1 @@
-"<a href=\"examp&lt;!--%22%20unsafeattr=foo()&gt;--&gt;le.com\">test</a>"
+"<a href=\"examp<!--%22%20unsafeattr=foo()>-->le.com\">test</a>"


rails test build/ruby-rails-html-sanitizer/src/rails-html-sanitizer-1.3.0/test/sanitizer_test.rb:483

...................................................F

Failure:
SanitizersTest#test_should_sanitize_div_background_image_unicode_encoded [/build/ruby-rails-html-sanitizer/src/rails-html-sanitizer-1.3.0/test/sanitizer_test.rb:417]:
--- expected
+++ actual
@@ -1 +1,3 @@
-""
+# encoding: ASCII-8BIT
+#    valid: true
+"background-image:\a 5 \a 2 \x06 \x02 8 \x02 9;"


rails test build/ruby-rails-html-sanitizer/src/rails-html-sanitizer-1.3.0/test/sanitizer_test.rb:415

...............F

Failure:
SanitizersTest#test_scrub_style_if_style_attribute_option_is_passed [/build/ruby-rails-html-sanitizer/src/rails-html-sanitizer-1.3.0/test/sanitizer_test.rb:274]:
--- expected
+++ actual
@@ -1 +1 @@
-"<p style=\"color: #000;\"></p>"
+"<p style=\"color:#000;\"></p>"


rails test build/ruby-rails-html-sanitizer/src/rails-html-sanitizer-1.3.0/test/sanitizer_test.rb:272

....F

Failure:
SanitizersTest#test_uri_escaping_of_name_action_in_a_tag_in_safe_list_sanitizer [/build/ruby-rails-html-sanitizer/src/rails-html-sanitizer-1.3.0/test/sanitizer_test.rb:520]:
--- expected
+++ actual
@@ -1 +1 @@
-"<a action=\"examp&lt;!--%22%20unsafeattr=foo()&gt;--&gt;le.com\">test</a>"
+"<a action=\"examp<!--%22%20unsafeattr=foo()>-->le.com\">test</a>"


rails test build/ruby-rails-html-sanitizer/src/rails-html-sanitizer-1.3.0/test/sanitizer_test.rb:513

..........................

Finished in 0.257575s, 1188.0049 runs/s, 1254.0052 assertions/s.
306 runs, 323 assertions, 6 failures, 0 errors, 0 skips
rake aborted!
Command failed with status (1): [ruby -w -I"lib" /usr/lib/ruby/gems/2.7.0/gems/rake-13.0.3/lib/rake/rake_test_loader.rb "test/sanitizer_test.rb" "test/scrubbers_test.rb" ]

Tasks: TOP => test
(See full trace by running task with --trace)

@jacobherrington
Copy link

jacobherrington commented Apr 7, 2021

There was a breaking change in Loofah 2.9.0 #112 fixes one of the two tests that fail. I have read quite a bit of Loofah and Nokogiri code today, so I'll try to fix the other failing test as well. 😅

@jacobherrington
Copy link

jacobherrington commented Apr 7, 2021

I can say that the second test is also a regression in Loofah 2.9.0; it probably makes the most sense to open an issue in that repository regarding this issue.

# 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)
=> ""

Unfortunately, upgrading to Loofah 2.9.1 (released earlier today to address a regression) does not fix the issue:

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;"

@flavorjones
Copy link
Member

I'm on it.

@flavorjones
Copy link
Member

See #113.

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.

5 participants