[libxml-devel] Segmentation fault when add the cloned/copied node

Sat Dec 1 13:02:56 EST 2007

Hi,

I created a patch.
I'm not sure that I really understood the problem, but following patch
works well for me.

my environment is:
Linux (CentOS 5.0)  kernel 2.6.18-8.1.15.el5  on  VMWare Workstation
ruby 1.8.5 (2006-08-25) [i386-linux]  (CentOS's latest package)
libxml 2.6.26 (CentOS's latest package)

The problem is in ruby_xml_node_child_set_aux():
 when the argument 'rnode' is a copied text node, 'chld' is not copied because
cnode->doc==NULL && cnode->parent==NULL && cnode->type==XML_TEXT_NODE.

But xmlAddChild *FREES* the second argument if it's a text node (I've
read libxml source). This causes double-freeing.
see http://xmlsoft.org/html/libxml-tree.html#xmlAddChild

So, what we need to do is to copy the node and return cnode->node
instead of chld.

# In addition, we need to define 'clone' method to avoid segmentation
fault because
# default Object#clone method is not suitable for ext-modules.

here's a patch (including unit tests).

Index: ext/xml/ruby_xml_node.c
===================================================================

--- ext/xml/ruby_xml_node.c	(revision 220)
+++ ext/xml/ruby_xml_node.c	(working copy)
@@ -327,7 +327,7 @@

   chld = cnode->node;

-  if ( chld->parent != NULL || chld->doc != NULL ) {
+  if ( chld->parent != NULL || chld->doc != NULL || chld->type ==
XML_TEXT_NODE ) {
     chld=xmlCopyNode(chld,1);
     copied=1;
     if ( do_raise == 1 )
@@ -342,7 +342,9 @@
   }

   // wish I could return a new wrapped chld, but ruby only returns the rhs
-  return ruby_xml_node2_wrap(cXMLNode,chld);
+  // chld is freed if xmlAddChild() successed and chld->type == XML_TEXT_NODE,
+  // so we need another copy of cnode->node.
+  return ruby_xml_node2_wrap(cXMLNode,cnode->node);
 }

 /*
Index: ext/xml/libxml.rb
===================================================================
--- ext/xml/libxml.rb	(revision 220)
+++ ext/xml/libxml.rb	(working copy)
@@ -72,6 +72,14 @@
   def <=>(other)
     to_s <=> other.to_s
   end
+
+  def clone
+    copy(false)
+  end
 end

 class XML::Attr
Index: tests/tc_xml_node_copy.rb
===================================================================
--- tests/tc_xml_node_copy.rb	(revision 0)
+++ tests/tc_xml_node_copy.rb	(revision 0)
@@ -0,0 +1,40 @@
+require "libxml_test"
+require "xml/libxml"
+require 'test/unit'
+
+# see mailing list archive
+# [libxml-devel] Segmentation fault when add the cloned/copied node
+# 2007/11/27 20:51
+class TC_XML_Node_Copy < Test::Unit::TestCase
+  def setup
+	str = <<-STR
+      <html><body>
+        <div class="textarea" id="t1" style="STATIC">foo</div>
+        <div class="textarea" id="t2" style="STATIC">bar</div>
+      </body></html>
+    STR
+	
+	doc = XML::Parser.string(str).parse
+	
+	xpath = "//div"
+	@div1 = doc.find(xpath).to_a[0]
+	@div2 = doc.find(xpath).to_a[1]
+  end
+
+  def test_libxml_node_copy_not_segv
+	@div2.each do |child|
+	  c = child.copy(false)
+	  @div1.child_add(c)
+	end
+	assert @div1.to_s =~ /foo/
+  end
+
+  def test_libxml_node_clone_not_segv
+	@div2.each do |child|
+	  c = child.clone
+	  @div1.child_add(c)
+	end
+	assert @div1.to_s =~ /foo/
+  end
+
+end # TC_XML_Node_Copy


-- 
FUKUDA, Keisuke
-------------- next part --------------
A non-text attachment was scrubbed...
Name: libxml_ruby_clone_segv_fix.patch
Type: application/octet-stream
Size: 2244 bytes
Desc: not available
Url : http://rubyforge.org/pipermail/libxml-devel/attachments/20071202/daeae4be/attachment.obj 
