Skip to content

Fix pointer access bug in XML::NodeSet#16055

Merged
straight-shoota merged 1 commit intocrystal-lang:masterfrom
toddsundsted:master
Aug 6, 2025
Merged

Fix pointer access bug in XML::NodeSet#16055
straight-shoota merged 1 commit intocrystal-lang:masterfrom
toddsundsted:master

Conversation

@toddsundsted
Copy link
Contributor

@toddsundsted toddsundsted commented Aug 4, 2025

The following small file segfaults with Crystal version 1.17.x on both OSX and Ubuntu (Arm):

require "xml"

subject = XML.parse_html("<html></html>")
nodes = subject.xpath_nodes("//div//text()")

Results:

% crystal --version
Crystal 1.17.1 [19be240d1] (2025-07-22)

LLVM: 15.0.7
Default target: aarch64-apple-macosx11.0

% crystal test.cr
Invalid memory access (signal 11) at address 0x0
[0x10463e4c8] *Exception::CallStack::print_backtrace:Nil +112 in /Users/toddsundsted/.cache/crystal/crystal-run-test.tmp
[0x10462bab8] ~procProc(Int32, Pointer(LibC::SiginfoT), Pointer(Void), Nil)@/Users/toddsundsted/.asdf/installs/crystal/1.17.1/src/crystal/system/unix/signal.cr:173 +276 in /Users/toddsundsted/.cache/crystal/crystal-run-test.tmp
[0x1902c4624] _sigtramp +56 in /usr/lib/system/libsystem_platform.dylib
[0x1046cec1c] *XML::XPathContext#evaluate<String>:(Bool | Float64 | String | XML::NodeSet) +1056 in /Users/toddsundsted/.cache/crystal/crystal-run-test.tmp (2 times)
[0x1046cbe5c] *XML::Document@XML::Node#xpath<String, Nil, Nil>:(Bool | Float64 | String | XML::NodeSet) +428 in /Users/toddsundsted/.cache/crystal/crystal-run-test.tmp
[0x1046cbc48] *XML::Document@XML::Node#xpath_nodes<String, Nil, Nil>:XML::NodeSet +32 in /Users/toddsundsted/.cache/crystal/crystal-run-test.tmp
[0x1046cbc00] *XML::Document@XML::Node#xpath_nodes<String>:XML::NodeSet +32 in /Users/toddsundsted/.cache/crystal/crystal-run-test.tmp
[0x104624b28] __crystal_main +1000 in /Users/toddsundsted/.cache/crystal/crystal-run-test.tmp
[0x1046859e0] *Crystal::main_user_code<Int32, Pointer(Pointer(UInt8))>:Nil +12 in /Users/toddsundsted/.cache/crystal/crystal-run-test.tmp
[0x10468592c] *Crystal::main<Int32, Pointer(Pointer(UInt8))>:Int32 +64 in /Users/toddsundsted/.cache/crystal/crystal-run-test.tmp
[0x104628510] main +32 in /Users/toddsundsted/.cache/crystal/crystal-run-test.tmp

The problem is the logic used to check the pointer when creating a nodeset.

This PR adds a test to demonstrate the bug and a fix.

Copilot AI review requested due to automatic review settings August 4, 2025 11:56

This comment was marked as spam.

Sign in to view

ysbaddaden
ysbaddaden approved these changes Aug 4, 2025
View reviewed changes
Copy link
Collaborator

@ysbaddaden ysbaddaden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙇

@straight-shoota straight-shoota added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:serialization labels Aug 4, 2025
@straight-shoota straight-shoota added this to the 1.18.0 milestone Aug 4, 2025
@straight-shoota straight-shoota changed the title Fix pointer access bug in XML::NodeSet Fix pointer access bug in XML::NodeSet Aug 4, 2025
@straight-shoota straight-shoota merged commit b0a1d6f into crystal-lang:master Aug 6, 2025
45 checks passed
@straight-shoota straight-shoota mentioned this pull request Oct 10, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@ysbaddaden ysbaddaden ysbaddaden approved these changes

@straight-shoota straight-shoota straight-shoota approved these changes

Assignees

No one assigned

Labels

kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:serialization

Projects

None yet

Milestone

1.18.0

Development

Successfully merging this pull request may close these issues.

4 participants

@toddsundsted @ysbaddaden @straight-shoota