replace SEGFAULT

[etm]

This one somewhat unexpectedly segfaults:

require 'rubygems'
require 'nokogiri'

doc = Nokogiri.XML('

')
node = doc.root.add_child(Nokogiri::XML::Node.new('hallo',doc))

nnode1 = Nokogiri::XML::Node.new('hallo',doc)
nnode2 = Nokogiri::XML::Node.new('test',doc)

node.replace(nnode1)
node.replace(nnode2)

---

This one works:

require 'rubygems'
require 'nokogiri'

doc = Nokogiri.XML('

')
node = doc.root.add_child(Nokogiri::XML::Node.new('hallo',doc))

nnode1 = Nokogiri::XML::Node.new('hallo',doc)
nnode2 = Nokogiri::XML::Node.new('test',doc)

node.replace(nnode1)
nnode1.replace(nnode2)

---

the name "replace" made me expect, that node is now REPLACED by nnode1 and can be used like nnode1. And of course segfaults are never good :-)

Cheers,

eTM

[ender672]

Thanks for the report with a simple script! I can reproduce this issue with a slight modification:

require 'nokogiri'
include Nokogiri::XML

doc = Nokogiri.XML('')

foo = Node.new('foo', doc)
bar = Node.new('bar', doc)

foo.replace(bar)

I'm still looking into the root cause before committing, but the issue appears to be in ext/nokogiri/xml_node.c:87 where:

  if (retval->type == XML_TEXT_NODE) {

Needs to be replaced with

  if (retval && retval->type == XML_TEXT_NODE) {

[ender672]

Looking at libxml2, tree.c:3850 (xmlReplaceNode)

    if ((old == NULL) || (old->type == XML_NAMESPACE_DECL) ||
        (old->parent == NULL)) {
        ...
        return(NULL);
    }

So, if you replace a node that has no parent, this function does nothing and will return NULL. Nokogiri currently doesn't anticipate this.

[etm]

grr, github ate my sample script cause if have not escaped the tags :-). Your simple script in fact is something different: replacing a node that is not connected to the tree indeed should just not happen.

My example is different:

require 'rubygems'
require 'nokogiri'

doc = Nokogiri.XML('

')
node = doc.root.add_child(Nokogiri::XML::Node.new('hallo',doc))

nnode1 = Nokogiri::XML::Node.new('hallo',doc)
nnode2 = Nokogiri::XML::Node.new('test',doc)

node.replace(nnode1)
node.replace(nnode2)

I think the least suprising behaviour would be that this is working. Meaning: the object node should afterwards be nnode1.

[ender672]

When you call node.replace(foo), node does not become foo. node gets disconnected from the document.

That is the current behavior. Not sure if it's worth changing the API, but the segfault is definitely a bug.

An API change would probably best fit into a separate ticket.

[ender672]

Fixed with 5a09ab9.

[tenderlove]

Can you paste the output of 'nokogiri -v'

Aaron Patterson
http://tenderlovemaking.com/
I'm on an iPhone so I apologize for top posting.

On Sep 13, 2012, at 4:14 AM, eTM [email protected] wrote:

This one somewhat unexpectedly segfaults:

require 'rubygems'
require 'nokogiri'

doc = Nokogiri.XML('')

node = doc.root.add_child(Nokogiri::XML::Node.new('hallo',doc))

nnode1 = Nokogiri::XML::Node.new('hallo',doc)
nnode2 = Nokogiri::XML::Node.new('test',doc)

node.replace(nnode1)
node.replace(nnode2)

This one works:

require 'rubygems'
require 'nokogiri'

doc = Nokogiri.XML('')

node = doc.root.add_child(Nokogiri::XML::Node.new('hallo',doc))

nnode1 = Nokogiri::XML::Node.new('hallo',doc)
nnode2 = Nokogiri::XML::Node.new('test',doc)

node.replace(nnode1)
nnode1.replace(nnode2)

the name "replace" made me expect, that node is now REPLACED by nnode1 and can be used like nnode1. And of course segfaults are never good :-)

Cheers,

eTM

�
Reply to this email directly or view it on GitHub.

[etm]

Nokogiri (1.5.5)

--- 
nokogiri: 1.5.5
ruby: 
  description: ruby 1.8.7 (2011-06-30 patchlevel 352) [i686-linux]
  engine: mri
  platform: i686-linux
  version: 1.8.7
libxml: 
  compiled: 2.7.8
  loaded: 2.7.8
  binding: extension
warnings: []