diff --git a/CHANGELOG.md b/CHANGELOG.md index fdd157d862..e43e1aa89b 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/ext/nokogiri/xml_node.c b/ext/nokogiri/xml_node.c index 545a39b106..2130e933a9 100644 --- a/ext/nokogiri/xml_node.c +++ b/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); diff --git a/test/xml/test_node_reparenting.rb b/test/xml/test_node_reparenting.rb index 77fc1ebe42..7ef7e30236 100644 --- a/test/xml/test_node_reparenting.rb +++ b/test/xml/test_node_reparenting.rb @@ -768,6 +768,22 @@ def coerce(data) assert_match(//, insert_point.children.to_xml) end end + + describe "creating a cycle in the graph" do + it "raises an exception" do + doc = Nokogiri::XML("") + 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