Merge pull request #678 from ender672/doc_frag_fix_1

Test for fragment node before assuming a node doc pointer is a doc.

ext/nokogiri/nokogiri.c

@@ -43,6 +43,26 @@ void vasprintf_free (void *p)
 #endif
 #endif
 
+void nokogiri_root_node(xmlNodePtr node)
+{
+  xmlDocPtr doc;
+  nokogiriTuplePtr tuple;
+
+  doc = node->doc;
+  if (doc->type == XML_DOCUMENT_FRAG_NODE) doc = doc->doc;
+  tuple = (nokogiriTuplePtr)doc->_private;
+  st_insert(tuple->unlinkedNodes, (st_data_t)node, (st_data_t)node);
+}
+
+void nokogiri_root_nsdef(xmlNsPtr ns, xmlDocPtr doc)
+{
+  nokogiriTuplePtr tuple;
+
+  if (doc->type == XML_DOCUMENT_FRAG_NODE) doc = doc->doc;
+  tuple = (nokogiriTuplePtr)doc->_private;
+  st_insert(tuple->unlinkedNodes, (st_data_t)ns, (st_data_t)ns);
+}
+
 void Init_nokogiri()
 {
 #ifndef __MACRUBY__

ext/nokogiri/nokogiri.h

@@ -120,11 +120,8 @@ extern VALUE mNokogiriHtml ;
 extern VALUE mNokogiriHtmlSax ;
 extern VALUE mNokogiriXslt ;
 
-#define NOKOGIRI_ROOT_NODE(_node) \
-  st_insert(((nokogiriTuplePtr)(_node)->doc->_private)->unlinkedNodes, (st_data_t)(_node), (st_data_t)(_node))
-
-#define NOKOGIRI_ROOT_NSDEF(_nsDef, _doc)     \
-  st_insert(((nokogiriTuplePtr)(_doc)->_private)->unlinkedNodes, (st_data_t)(_nsDef), (st_data_t)(_nsDef))
+void nokogiri_root_node(xmlNodePtr);
+void nokogiri_root_nsdef(xmlNsPtr, xmlDocPtr);
 
 #ifdef DEBUG
 

ext/nokogiri/xml_attr.c

@@ -65,7 +65,7 @@ static VALUE new(int argc, VALUE *argv, VALUE klass)
       NULL
   );
 
-  NOKOGIRI_ROOT_NODE((xmlNodePtr)node);
+  nokogiri_root_node((xmlNodePtr)node);
 
   rb_node = Nokogiri_wrap_xml_node(klass, (xmlNodePtr)node);
   rb_obj_call_init(rb_node, argc, argv);

ext/nokogiri/xml_cdata.c

@@ -25,7 +25,7 @@ static VALUE new(int argc, VALUE *argv, VALUE klass)
       NIL_P(content) ? 0 : (int)RSTRING_LEN(content)
   );
 
-  NOKOGIRI_ROOT_NODE(node);
+  nokogiri_root_node(node);
 
   rb_node = Nokogiri_wrap_xml_node(klass, node);
   rb_obj_call_init(rb_node, argc, argv);

ext/nokogiri/xml_comment.c

@@ -27,7 +27,7 @@ static VALUE new(int argc, VALUE *argv, VALUE klass)
   rb_node = Nokogiri_wrap_xml_node(klass, node);
   rb_obj_call_init(rb_node, argc, argv);
 
-  NOKOGIRI_ROOT_NODE(node);
+  nokogiri_root_node(node);
 
   if(rb_block_given_p()) rb_yield(rb_node);
 

ext/nokogiri/xml_document.c

@@ -102,7 +102,7 @@ static VALUE set_root(VALUE self, VALUE root)
 
     if(old_root) {
       xmlUnlinkNode(old_root);
-      NOKOGIRI_ROOT_NODE(old_root);
+      nokogiri_root_node(old_root);
     }
 
     return root;
@@ -121,7 +121,7 @@ static VALUE set_root(VALUE self, VALUE root)
   }
 
   xmlDocSetRootElement(doc, new_root);
-  if(old_root) NOKOGIRI_ROOT_NODE(old_root);
+  if(old_root) nokogiri_root_node(old_root);
   return root;
 }
 

ext/nokogiri/xml_document_fragment.c

@@ -20,7 +20,7 @@ static VALUE new(int argc, VALUE *argv, VALUE klass)
 
   node = xmlNewDocFragment(xml_doc->doc);
 
-  NOKOGIRI_ROOT_NODE(node);
+  nokogiri_root_node(node);
 
   rb_node = Nokogiri_wrap_xml_node(klass, node);
   rb_obj_call_init(rb_node, argc, argv);

ext/nokogiri/xml_entity_reference.c

@@ -24,7 +24,7 @@ static VALUE new(int argc, VALUE *argv, VALUE klass)
       (const xmlChar *)StringValuePtr(name)
   );
 
-  NOKOGIRI_ROOT_NODE(node);
+  nokogiri_root_node(node);
 
   rb_node = Nokogiri_wrap_xml_node(klass, node);
   rb_obj_call_init(rb_node, argc, argv);

ext/nokogiri/xml_node.c

@@ -51,7 +51,7 @@ static void relink_namespace(xmlNodePtr reparented)
         } else {
           reparented->nsDef = curr->next;
         }
-        NOKOGIRI_ROOT_NSDEF(curr, reparented->doc);
+        nokogiri_root_nsdef(curr, reparented->doc);
       } else {
         prev = curr;
       }
@@ -132,7 +132,7 @@ static VALUE reparent_node_with(VALUE pivot_obj, VALUE reparentee_obj, pivot_rep
      *  uninteresting libxml2 implementation detail). as a result, we cannot
      *  reparent the actual reparentee, so we reparent a duplicate.
      */
-    NOKOGIRI_ROOT_NODE(reparentee);
+    nokogiri_root_node(reparentee);
     if (!(reparentee = xmlDocCopyNode(reparentee, pivot->doc, 1))) {
       rb_raise(rb_eRuntimeError, "Could not reparent node (xmlDocCopyNode)");
     }
@@ -162,7 +162,7 @@ static VALUE reparent_node_with(VALUE pivot_obj, VALUE reparentee_obj, pivot_rep
     new_next_text = xmlDocCopyNode(next_text, pivot->doc, 1) ;
 
     xmlUnlinkNode(next_text);
-    NOKOGIRI_ROOT_NODE(next_text);
+    nokogiri_root_node(next_text);
 
     xmlAddNextSibling(pivot, new_next_text);
   }
@@ -375,7 +375,7 @@ static VALUE duplicate_node(int argc, VALUE *argv, VALUE self)
   dup = xmlDocCopyNode(node, node->doc, (int)NUM2INT(level));
   if(dup == NULL) return Qnil;
 
-  NOKOGIRI_ROOT_NODE(dup);
+  nokogiri_root_node(dup);
 
   return Nokogiri_wrap_xml_node(rb_obj_class(self), dup);
 }
@@ -391,7 +391,7 @@ static VALUE unlink_node(VALUE self)
   xmlNodePtr node;
   Data_Get_Struct(self, xmlNode, node);
   xmlUnlinkNode(node);
-  NOKOGIRI_ROOT_NODE(node);
+  nokogiri_root_node(node);
   return self;
 }
 
@@ -489,7 +489,7 @@ static VALUE replace(VALUE self, VALUE new_node)
 
     xmlNodePtr pivot;
     Data_Get_Struct(self, xmlNode, pivot);
-    NOKOGIRI_ROOT_NODE(pivot);
+    nokogiri_root_node(pivot);
 
     return reparent;
 }
@@ -681,7 +681,7 @@ static VALUE set(VALUE self, VALUE property, VALUE value)
   if (prop && prop->children) {
     for (cur = prop->children; cur; cur = cur->next) {
       if (cur->_private) {
-        NOKOGIRI_ROOT_NODE(cur);
+        nokogiri_root_node(cur);
         xmlUnlinkNode(cur);
       }
     }
@@ -901,7 +901,7 @@ static VALUE set_content(VALUE self, VALUE content)
   while (NULL != child) {
     next = child->next ;
     xmlUnlinkNode(child) ;
-    NOKOGIRI_ROOT_NODE(child) ;
+    nokogiri_root_node(child);
     child = next ;
   }
 
@@ -1133,7 +1133,7 @@ static VALUE new(int argc, VALUE *argv, VALUE klass)
 
   node = xmlNewNode(NULL, (xmlChar *)StringValuePtr(name));
   node->doc = doc->doc;
-  NOKOGIRI_ROOT_NODE(node);
+  nokogiri_root_node(node);
 
   rb_node = Nokogiri_wrap_xml_node(
       klass == cNokogiriXmlNode ? (VALUE)NULL : klass,
@@ -1320,6 +1320,7 @@ VALUE Nokogiri_wrap_xml_node(VALUE klass, xmlNodePtr node)
   VALUE node_cache = Qnil ;
   VALUE rb_node = Qnil ;
   nokogiriTuplePtr node_has_a_document;
+  xmlDocPtr doc;
   void (*mark_method)(xmlNodePtr) = NULL ;
 
   assert(node);
@@ -1330,7 +1331,9 @@ VALUE Nokogiri_wrap_xml_node(VALUE klass, xmlNodePtr node)
   /* It's OK if the node doesn't have a fully-realized document (as in XML::Reader). */
   /* see https://github.com/tenderlove/nokogiri/issues/95 */
   /* and https://github.com/tenderlove/nokogiri/issues/439 */
-  node_has_a_document = DOC_RUBY_OBJECT_TEST(node->doc);
+  doc = node->doc;
+  if (doc->type == XML_DOCUMENT_FRAG_NODE) doc = doc->doc;
+  node_has_a_document = DOC_RUBY_OBJECT_TEST(doc);
 
   if(node->_private && node_has_a_document)
     return (VALUE)node->_private;
@@ -1385,8 +1388,8 @@ VALUE Nokogiri_wrap_xml_node(VALUE klass, xmlNodePtr node)
   node->_private = (void *)rb_node;
 
   if (node_has_a_document) {
-    document = DOC_RUBY_OBJECT(node->doc);
-    node_cache = DOC_NODE_CACHE(node->doc);
+    document = DOC_RUBY_OBJECT(doc);
+    node_cache = DOC_NODE_CACHE(doc);
     rb_ary_push(node_cache, rb_node);
     rb_funcall(document, decorate, 1, rb_node);
   }

ext/nokogiri/xml_processing_instruction.c

@@ -27,7 +27,7 @@ static VALUE new(int argc, VALUE *argv, VALUE klass)
       (const xmlChar *)StringValuePtr(content)
   );
 
-  NOKOGIRI_ROOT_NODE(node);
+  nokogiri_root_node(node);
 
   rb_node = Nokogiri_wrap_xml_node(klass, node);
   rb_obj_call_init(rb_node, argc, argv);

ext/nokogiri/xml_text.c

@@ -22,7 +22,7 @@ static VALUE new(int argc, VALUE *argv, VALUE klass)
   node = xmlNewText((xmlChar *)StringValuePtr(string));
   node->doc = doc->doc;
 
-  NOKOGIRI_ROOT_NODE(node);
+  nokogiri_root_node(node);
 
   rb_node = Nokogiri_wrap_xml_node(klass, node) ;
   rb_obj_call_init(rb_node, argc, argv);

test/xml/test_document_fragment.rb

@@ -176,6 +176,11 @@ def awesome!
         assert fragment.children.respond_to?(:awesome!), fragment.children.class
       end
 
+      def test_add_node_to_doc_fragment_segfault
+        frag = Nokogiri::XML::DocumentFragment.new(@xml, '<p>hello world</p>')
+        Nokogiri::XML::Comment.new(frag,'moo')
+      end
+
       if Nokogiri.uses_libxml?
         def test_for_libxml_in_context_fragment_parsing_bug_workaround
           10.times do