Fix nodes ownership after cloning a Xml/HtmlDocument

fixes #1060
sparklemotion · Jan 21, 2019 · 2fd4158 · 2fd4158
1 parent 1647bd0
commit 2fd4158
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Nokogiri Changelog
 
+## unreleased
+
+### Bug fixes
+
+* [JRuby] Fix node ownership in duplicated documents. [#1060]
+
 ## 1.10.1 / 2019-01-13
 
 ### Features

diff --git a/ext/java/nokogiri/XmlDocument.java b/ext/java/nokogiri/XmlDocument.java
@@ -397,6 +397,20 @@ public IRubyObject root(ThreadContext context) {
             return getCachedNodeOrCreate(context.getRuntime(), rootNode);
     }
 
+    protected IRubyObject dup_implementation(Ruby runtime, boolean deep) {
+        XmlDocument doc = (XmlDocument) super.dup_implementation(runtime, deep);
+        // Avoid creating a new XmlDocument since we cloned one
+        // already. Otherwise the following test will fail:
+        //
+        //   dup = doc.dup
+        //   dup.equal?(dup.children[0].document)
+        //
+        // Since `dup.children[0].document' will end up creating a new
+        // XmlDocument.  See #1060.
+        doc.node.setUserData(NokogiriHelpers.CACHED_NODE, doc, null);
+        return doc;
+    }
+
     @JRubyMethod(name="root=")
     public IRubyObject root_set(ThreadContext context, IRubyObject newRoot_) {
         // in case of document fragment, temporary root node should be deleted.

diff --git a/test/html/test_document.rb b/test/html/test_document.rb
@@ -532,6 +532,17 @@ def test_dup
         assert_equal found.document, dup.document
       end
 
+      # issue 1060
+      def test_node_ownership_after_dup
+        html = "<html><head></head><body><div>replace me</div></body></html>"
+        doc = Nokogiri::HTML::Document.parse(html)
+        dup = doc.dup
+        assert_same dup, dup.at_css('div').document
+
+        # should not raise an exception
+        dup.at_css('div').parse('<div>replaced</div>')
+      end
+
       def test_inner_html
         html = Nokogiri::HTML <<-EOHTML
         <html>

diff --git a/test/xml/test_document.rb b/test/xml/test_document.rb
@@ -50,6 +50,17 @@ def test_document_with_refentity
         assert_equal '', doc.content
       end
 
+      # issue 1060
+      def test_node_ownership_after_dup
+        html = "<html><head></head><body><div>replace me</div></body></html>"
+        doc = Nokogiri::XML::Document.parse(html)
+        dup = doc.dup
+        assert_same dup, dup.at_css('div').document
+
+        # should not raise an exception
+        dup.at_css('div').parse('<div>replaced</div>')
+      end
+
       # issue #835
       def test_manually_adding_reference_entities
         d = Nokogiri::XML::Document.new