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.

Sign up for GitHub

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

Jump to bottom

dep: update libxml2 to 2.13.0 and libxslt to 1.1.40 #3230

Merged
merged 6 commits into from
Jun 13, 2024
Merged

dep: update libxml2 to 2.13.0 and libxslt to 1.1.40 #3230

merged 6 commits into from
Jun 13, 2024

Conversation

flavorjones
Copy link
Member

@flavorjones flavorjones commented Jun 12, 2024 •
edited
Loading

@flavorjones
Copy link
Member Author

Looks like we need to wait for a companion libxslt release with GNOME/libxslt@75967fc0 to address the compilation issues.

@flavorjones flavorjones changed the title dep: update libxml2 to 2.13.0 dep: update libxml2 to 2.13.0 and libxslt to 1.1.40 Jun 12, 2024
@flavorjones
Copy link
Member Author

Added libxslt upgrade to 1.1.40 to this PR

@flavorjones
Copy link
Member Author

@rgrove Take note that sanitize is going to have some test failures once this makes it into a release (likely v1.17.0 later in the summer):

https://github.com/sparklemotion/nokogiri/actions/runs/9487728125/job/26145008195?pr=3230

  1) Failure:
Sanitize::Transformers::CleanDoctype::when :allow_doctype is true#test_0002_should not allow obviously invalid doctype declarations in documents [/__w/nokogiri/nokogiri/sanitize/test/test_clean_doctype.rb:47]:
--- expected
+++ actual
@@ -1 +1 @@
-"<!DOCTYPE html><html>foo</html>"
+"<!DOCTYPE blah><html>foo</html>"

@rgrove
Copy link
Contributor

rgrove commented Jun 12, 2024

@rgrove Take note that sanitize is going to have some test failures once this makes it into a release (likely v1.17.0 later in the summer)

Thanks for the heads up! I'll look into this.

@flavorjones
Copy link
Member Author

@rgrove Hmm, I'm digging in and something weird is going on, it's not Sanitize.

@flavorjones
Copy link
Member Author

@rgrove Ahhhh ... GNOME/libxml2@a3713f78

Disallow xmlNodeSetName on DTD nodes. DTD nodes don't store the name in a dictionary. Calling xmlNodeSetName with a DTD node could result in an invalid free.

@rgrove
Copy link
Contributor

rgrove commented Jun 12, 2024

Fun! So it looks like Sanitize may need a small change after all. 🙂

@flavorjones
Copy link
Member Author

The downstream Mechanize test failure is due to GNOME/libxml2@8d0aaf4

Not totally sure what the relationship is between the change and the behavior, but I'm also not totally sure I care, we've seen edge case encoding changes like this before and the Mechanize test in question is more descriptive than prescriptive ... sigh. I'll probably just change the test.

@flavorjones
Copy link
Member Author

Ah, this is because Mechanize's encoding_error? method is checking the error message contents ... blurgh. Will update that and I think it should be fine.

@flavorjones
Copy link
Member Author

See sparklemotion/mechanize#644

@flavorjones
Copy link
Member Author

I've opened rgrove/sanitize#238 on sanitize as a potential workaround for the DTD name-change limitation.

@flavorjones
test: let's add coverage for the doctype rename workaround
7b713b9 
I want to make sure that we don't break the thing we're asking
Sanitize to do; and I want to make sure we have valgrind coverage for it.

See rgrove/sanitize#238 for context.
This was referenced Jun 12, 2024
Merged
Closed
@flavorjones
Copy link
Member Author

The failure showing up on freebsd is because my upstream fix from #1941 shipped in v2.13.0! Removing the patch in patches/libxml2/0003-libxml2.la-is-in-top_builddir.patch! 🚀

@flavorjones flavorjones force-pushed the flavorjones-dep-libxml2-2.13.0 branch from 8bc0eef to 539c35c Compare June 13, 2024 13:21
@flavorjones flavorjones merged commit 92fb6dc into main Jun 13, 2024
132 checks passed
@flavorjones flavorjones deleted the flavorjones-dep-libxml2-2.13.0 branch June 13, 2024 14:30
@flavorjones flavorjones added this to the v1.17.0 milestone Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.17.0
Development

Successfully merging this pull request may close these issues.
2 participants
@flavorjones @rgrove