Tests failing in Fedora

[pvalena]

Hello, I've tried to bring back sanitize 5.1.0 into Fedora, but it seems some tests are failing:

................................FFF.FFF.F.............F.......................
Finished in 0.495989s, 451.6227 runs/s, 2987.9677 assertions/s.
  1) Failure:
Malicious HTML::unsafe libxml2 server-side includes in attributes#test_0013_should escape unsafe characters in attributes [/builddir/build/BUILD/sanitize-5.1.0/usr/share/gems/gems/sanitize-5.1.0/test/test_malicious_html.rb:181]:
--- expected
+++ actual
@@ -1 +1 @@
-"<div src=\"examp&lt;!--%22%20onmouseover=alert(1)&gt;--&gt;le.com\">foo</div>"
+"<div src=\"examp<!--%22%20onmouseover=alert(1)>-->le.com\">foo</div>"
  2) Failure:
Malicious HTML::unsafe libxml2 server-side includes in attributes#test_0001_should escape unsafe characters in attributes [/builddir/build/BUILD/sanitize-5.1.0/usr/share/gems/gems/sanitize-5.1.0/test/test_malicious_html.rb:181]:
--- expected
+++ actual
@@ -1 +1 @@
-"<a action=\"examp&lt;!--%22%20onmouseover=alert(1)&gt;--&gt;le.com\">foo</a>"
+"<a action=\"examp<!--%22%20onmouseover=alert(1)>-->le.com\">foo</a>"
  3) Failure:
Malicious HTML::unsafe libxml2 server-side includes in attributes#test_0003_should escape unsafe characters in attributes [/builddir/build/BUILD/sanitize-5.1.0/usr/share/gems/gems/sanitize-5.1.0/test/test_malicious_html.rb:181]:
--- expected
+++ actual
@@ -1 +1 @@
-"<a href=\"examp&lt;!--%22%20onmouseover=alert(1)&gt;--&gt;le.com\">foo</a>"
+"<a href=\"examp<!--%22%20onmouseover=alert(1)>-->le.com\">foo</a>"
  4) Failure:
Malicious HTML::unsafe libxml2 server-side includes in attributes#test_0005_should escape unsafe characters in attributes [/builddir/build/BUILD/sanitize-5.1.0/usr/share/gems/gems/sanitize-5.1.0/test/test_malicious_html.rb:181]:
--- expected
+++ actual
@@ -1 +1 @@
-"<a src=\"examp&lt;!--%22%20onmouseover=alert(1)&gt;--&gt;le.com\">foo</a>"
+"<a src=\"examp<!--%22%20onmouseover=alert(1)>-->le.com\">foo</a>"
  5) Failure:
Malicious HTML::unsafe libxml2 server-side includes in attributes#test_0007_should escape unsafe characters in attributes [/builddir/build/BUILD/sanitize-5.1.0/usr/share/gems/gems/sanitize-5.1.0/test/test_malicious_html.rb:181]:
--- expected
+++ actual
@@ -1 +1 @@
-"<a name=\"examp&lt;!--%22%20onmouseover=alert(1)&gt;--&gt;le.com\">foo</a>"
+"<a name=\"examp<!--%22%20onmouseover=alert(1)>-->le.com\">foo</a>"
  6) Failure:
Malicious HTML::unsafe libxml2 server-side includes in attributes#test_0009_should escape unsafe characters in attributes [/builddir/build/BUILD/sanitize-5.1.0/usr/share/gems/gems/sanitize-5.1.0/test/test_malicious_html.rb:181]:
--- expected
+++ actual
@@ -1 +1 @@
-"<div action=\"examp&lt;!--%22%20onmouseover=alert(1)&gt;--&gt;le.com\">foo</div>"
+"<div action=\"examp<!--%22%20onmouseover=alert(1)>-->le.com\">foo</div>"
  7) Failure:
Malicious HTML::unsafe libxml2 server-side includes in attributes#test_0015_should not escape characters unnecessarily [/builddir/build/BUILD/sanitize-5.1.0/usr/share/gems/gems/sanitize-5.1.0/test/test_malicious_html.rb:206]:
--- expected
+++ actual
@@ -1 +1 @@
-"<div name='examp&lt;!--\" onmouseover=alert(1)&gt;--&gt;le.com'>foo</div>"
+"<div name='examp<!--\" onmouseover=alert(1)>-->le.com'>foo</div>"
  8) Failure:
Malicious HTML::unsafe libxml2 server-side includes in attributes#test_0011_should escape unsafe characters in attributes [/builddir/build/BUILD/sanitize-5.1.0/usr/share/gems/gems/sanitize-5.1.0/test/test_malicious_html.rb:181]:
--- expected
+++ actual
@@ -1 +1 @@
-"<div href=\"examp&lt;!--%22%20onmouseover=alert(1)&gt;--&gt;le.com\">foo</div>"
+"<div href=\"examp<!--%22%20onmouseover=alert(1)>-->le.com\">foo</div>"
224 runs, 1482 assertions, 8 failures, 0 errors, 0 skips

The whole package build log can be found here:
https://download.copr.fedorainfracloud.org/results/pvalena/rubygems/fedora-rawhide-x86_64/01352482-rubygem-sanitize/build.log.gz

It also fails on all Fedoras and EL+8:
https://copr.fedorainfracloud.org/coprs/pvalena/rubygems/build/1352482/

Versions:

ruby x86_64 2.7.1-130.fc33 fedora 41 k
rubygem-crass noarch 1.0.4-5.fc33 copr_base 18 k
rubygem-minitest noarch 5.14.0-201.fc32 fedora 45 k
rubygem-nokogiri x86_64 1.10.9-1.fc33.1 copr_base 128 k
rubygem-nokogumbo x86_64 2.0.2-1.fc33.1 copr_base 300 k

Let me know if I can do anything else to debug/workaround this issue, or if you need more info.

[flavorjones]

@pvalena I think these tests were introduced to test sanitize against a version of nokogiri with this patch applied to libxml2: sparklemotion/nokogiri#1877

If you're using sanitize with a version of Nokogiri linked against Fedora's distro of libxml2, then I would expect these tests to fail.

It might be possible to skip these tests if Nokogiri is compiled against system libraries; maybe @rgrove can comment on whether he'd be open to a change like that in the test suite.

[pvalena]

Ah yes, we try to avoid the use of bundled libraries. I was hoping this was not the case.
I'll skip the tests manually then, as I don't think we'll be fixing nokogiri.

Thanks for the explanation!

[rgrove]

Thanks for the assist, @flavorjones! You're right, these failures do appear to be related to the tests expecting the output that Nokogiri's bundled libxml2 would generate.

@pvalena I'd be open to a patch that alters the behavior of these tests either by detecting that a non-patched libxml2 is being used and asserting different output as appropriate, or by skipping these tests when a specific env var is set. But if you already have an easier way to work around this on your end, that works too. 😄

[pvalena]

@pvalena I'd be open to a patch that alters the behavior of these tests either by detecting that a non-patched libxml2 is being used and asserting different output as appropriate, or by skipping these tests when a specific env var is set. But if you already have an easier way to work around this on your end, that works too. 😄

Actually, my solution would be to ignore the failures. But as I'll probably avoid owning the package in Fedora (I currently don't have much use for it), it doesn't matter now.

@rgrove thanks!