fix: Document#root= raises an ArgumentError for a non-Node arg

Previously this segfaulted on CRuby, and raised a TypeError on JRuby. Fixes #1900
sparklemotion · Mar 22, 2021 · 4031110 · 4031110
1 parent 2920cf4
commit 4031110
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,14 @@ Nokogiri follows [Semantic Versioning](https://semver.org/), please see the [REA
 
 ---
 
+## next / unreleased
+
+### Fixed
+
+* [CRuby] Passing non-`Node` objects to `Document#root=` now raises an `ArgumentError` exception. Previously this likely segfaulted. [[#1900](https://github.com/sparklemotion/nokogiri/issues/1900)]
+* [JRuby] Passing non-`Node` objects to `Document#root=` now raises an `ArgumentError` exception. Previously this raised a `TypeError` exception.
+
+
 ## 1.11.2 / 2021-03-11
 
 ### Fixed

diff --git a/ext/java/nokogiri/XmlDocument.java b/ext/java/nokogiri/XmlDocument.java
@@ -442,6 +442,9 @@ private static class DocumentBuilderFactoryHolder
       getDocument().getDocumentElement().setUserData(NokogiriHelpers.ROOT_NODE_INVALID, Boolean.TRUE, null);
       return new_root;
     }
+    if (!(new_root instanceof XmlNode)) {
+        throw context.runtime.newArgumentError("expected Nokogiri::XML::Node but received " + new_root.getType());
+    }
     XmlNode newRoot = asXmlNode(context, new_root);
 
     IRubyObject root = root(context);

diff --git a/ext/nokogiri/xml_document.c b/ext/nokogiri/xml_document.c
@@ -155,6 +155,12 @@ rb_xml_document_root_set(VALUE self, VALUE rb_new_root)
   }
 
   if (!NIL_P(rb_new_root)) {
+    if (!rb_obj_is_kind_of(rb_new_root, cNokogiriXmlNode)) {
+      rb_raise(rb_eArgError,
+               "expected Nokogiri::XML::Node but received %"PRIsVALUE,
+               rb_obj_class(rb_new_root));
+    }
+
     Data_Get_Struct(rb_new_root, xmlNode, c_new_root);
 
     /* If the new root's document is not the same as the current document,

diff --git a/test/xml/test_document.rb b/test/xml/test_document.rb
@@ -1150,6 +1150,22 @@ def initialize(*args)
             doc2 = Nokogiri::XML("<root>#{'x' * 5000000}</root>")
             doc2.root = doc.root
           end
+
+          it "raises an exception if passed something besides a Node" do
+            doc = Nokogiri::XML::Document.parse(<<~EOF)
+              <root>
+                <div>one</div>
+                <div>two</div>
+                <div>three</div>
+              </root>
+            EOF
+            node_set = doc.css("div")
+            assert_kind_of(Nokogiri::XML::NodeSet, node_set)
+            e = assert_raises(ArgumentError) do
+              doc.root = node_set
+            end
+            assert_equal("expected Nokogiri::XML::Node but received Nokogiri::XML::NodeSet", e.message);
+          end
         end
       end
     end