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

Segmentation fault when creating a comment node for a DocumentFragment #677

Closed
kristi opened this issue May 14, 2012 · 4 comments
Closed

Comments

@kristi
Copy link

kristi commented May 14, 2012

The following code segfaults under Ruby 1.8.7 and Nokogiri 1.5.2

#!/usr/bin/ruby
require 'rubygems'
require 'nokogiri'
doc = Nokogiri::XML::DocumentFragment.parse('<p>hello world</p>')
c = Nokogiri::XML::Comment.new(doc,'moo')

Error message:

./nokogiri-comment.rb:5: [BUG] Segmentation fault
ruby 1.8.7 (2010-08-16 patchlevel 302) [x86_64-linux]
@ender672
Copy link
Member

Thanks for the sample code. I can reproduce this on my machine.

Looks like we are not handling children of document fragment nodes very well in Nokogiri.

In ext/nokogiri/xml_node.c:1387:

  if (node_has_a_document) {
    document = DOC_RUBY_OBJECT(node->doc);
    node_cache = DOC_NODE_CACHE(node->doc);
    rb_ary_push(node_cache, rb_node);
    rb_funcall(document, decorate, 1, rb_node);
  }

However, the node->doc pointer for the child of a document fragment will point to the document fragment, which does not have a node cache.

Looks like we need to test if node->doc is a document fragment, and if it is we need to use its document instead. I'm working on this, and I also need to audit if we are making the same mistake elsewhere in Nokogiri.

I'll keep this issue updated with progress.

@ender672
Copy link
Member

Ouch. The NOKOGIRI_ROOT_NODE macro gets called in about 20 different places in Nokogiri and it assumes that the doc pointer for a node is an XML document. This breaks when the doc pointer is a document fragment node:

in ext/nokogiri/nokogiri.h:123

#define NOKOGIRI_ROOT_NODE(_node) \
  st_insert(((nokogiriTuplePtr)(_node)->doc->_private)->unlinkedNodes, (st_data_t)(_node), (st_data_t)(_node))

Nokogiri treats document fragment nodes more like an element than a document (while libxml2 treats it as something in between).

I'm still wrapping my head around this. Will continue to update this ticket.

@kristi
Copy link
Author

kristi commented May 15, 2012

Thanks for looking into this. I wasn't sure from the documentation whether DocumentFragment was intended to act more like a Document or a NodeSet.

@flavorjones
Copy link
Member

Pull request merged!

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

No branches or pull requests

3 participants