Skip to content

Commit

Permalink
feat(cruby): raise RuntimeError if reparenting creates a cycle
Browse files Browse the repository at this point in the history
Fixes #1912
  • Loading branch information
flavorjones committed Aug 18, 2021
1 parent e9190ac commit 96045ca
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -15,6 +15,7 @@ Nokogiri follows [Semantic Versioning](https://semver.org/), please see the [REA

* [CRuby] Handle abruptly-closed HTML comments as WHATWG recommends for browsers. (Thanks to HackerOne user [tehryanx](https://hackerone.com/tehryanx?type=user) for reporting this!)
* [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
Expand Down
19 changes: 17 additions & 2 deletions ext/nokogiri/xml_node.c
Expand Up @@ -161,6 +161,19 @@ xmlReplaceNodeWrapper(xmlNodePtr pivot, xmlNodePtr new_node)
return retval ;
}

static void
raise_if_ancestor_of_self(xmlNodePtr self)
{
xmlNodePtr ancestor = self->parent;
while (ancestor) {
if (self == ancestor) {
rb_raise(rb_eRuntimeError, "cycle detected: node '%s' is an ancestor of itself", self->name);
}
ancestor = ancestor->parent;
}
}


/* :nodoc: */
static VALUE
reparent_node_with(VALUE pivot_obj, VALUE reparentee_obj, pivot_reparentee_func prf)
Expand Down Expand Up @@ -350,10 +363,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_self(reparented);

reparented_obj = noko_xml_node_wrap(Qnil, reparented);
relink_namespace(reparented);

rb_funcall(reparented_obj, id_decorate_bang, 0);

Expand Down
16 changes: 16 additions & 0 deletions test/xml/test_node_reparenting.rb
Expand Up @@ -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
Expand Down

0 comments on commit 96045ca

Please sign in to comment.