feat(cruby): raise RuntimeError if reparenting creates a cycle

Fixes #1912

CHANGELOG.md

@@ -9,6 +9,7 @@ Nokogiri follows [Semantic Versioning](https://semver.org/), please see the [REA
 ### Improved
 
 * [CRuby] `Node#line` is no longer capped at 65535. libxml v2.9.0 and later support a new parse option, exposed as `Nokogiri::XML::ParseOptions::PARSE_BIG_LINES` and set in `ParseOptions::DEFAULT_XML`, `::DEFAULT_XSLT`, `::DEFAULT_HTML`, and `::DEFAULT_SCHEMA`. (Note that JRuby never had this problem.) [[#1764](https://github.com/sparklemotion/nokogiri/issues/1764), [#1493](https://github.com/sparklemotion/nokogiri/issues/1493), [#1617](https://github.com/sparklemotion/nokogiri/issues/1617), [#1505](https://github.com/sparklemotion/nokogiri/issues/1505), [#1003](https://github.com/sparklemotion/nokogiri/issues/1003), [#533](https://github.com/sparklemotion/nokogiri/issues/533)]
+* [CRuby] If a cycle is introduced when reparenting a node (i.e., the node becomes its own ancestor), a `RuntimeError` is raised. libxml2 does no checking for this, which means cycles would otherwise result in infinite loops on subsequent operations. (Note: JRuby/Xerces already does this.) [[#1912](https://github.com/sparklemotion/nokogiri/issues/1912)]
 
 
 ## 1.12.3 / 2021-08-10

ext/nokogiri/xml_node.c

@@ -161,6 +161,17 @@ xmlReplaceNodeWrapper(xmlNodePtr pivot, xmlNodePtr new_node)
   return retval ;
 }
 
+static void
+raise_if_ancestor_of(xmlNodePtr self, xmlNodePtr subject)
+{
+  if (subject == NULL) { return; }
+  if (self == subject) {
+    rb_raise(rb_eRuntimeError, "cycle detected: node '%s' is an ancestor of itself", self->name);
+  }
+  raise_if_ancestor_of(self, subject->parent);
+}
+
+
 /* :nodoc: */
 static VALUE
 reparent_node_with(VALUE pivot_obj, VALUE reparentee_obj, pivot_reparentee_func prf)
@@ -350,10 +361,12 @@ reparent_node_with(VALUE pivot_obj, VALUE reparentee_obj, pivot_reparentee_func
    *  adjacent text nodes.
    */
   DATA_PTR(reparentee_obj) = reparented ;
+  reparented_obj = noko_xml_node_wrap(Qnil, reparented);
 
-  relink_namespace(reparented);
+  /* if we've created a cycle, raise an exception */
+  raise_if_ancestor_of(reparented, reparented->parent);
 
-  reparented_obj = noko_xml_node_wrap(Qnil, reparented);
+  relink_namespace(reparented);
 
   rb_funcall(reparented_obj, id_decorate_bang, 0);
 

test/xml/test_node_reparenting.rb

@@ -768,6 +768,22 @@ def coerce(data)
             assert_match(/<Component>/, insert_point.children.to_xml)
           end
         end
+
+        describe "creating a cycle in the graph" do
+          it "raises an exception" do
+            doc = Nokogiri::XML("<root><a><b/></a></root>")
+            a = doc.at_css("a")
+            b = doc.at_css("b")
+            exception = assert_raise(RuntimeError) do
+              a.parent = b
+            end
+            if Nokogiri.jruby?
+              assert_match(/HIERARCHY_REQUEST_ERR/, exception.message)
+            else
+              assert_match(/cycle detected/, exception.message)
+            end
+          end
+        end
       end
     end
   end