From 2fd4158ddef7ab9895194ac04dad68623939fb40 Mon Sep 17 00:00:00 2001 From: John Shahid Date: Mon, 21 Jan 2019 10:57:16 -0500 Subject: [PATCH] Fix nodes ownership after cloning a Xml/HtmlDocument fixes #1060 --- CHANGELOG.md | 6 ++++++ ext/java/nokogiri/XmlDocument.java | 14 ++++++++++++++ test/html/test_document.rb | 11 +++++++++++ test/xml/test_document.rb | 11 +++++++++++ 4 files changed, 42 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d6580f3ea9..46f724b155 100644 --- 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 index 0765f97cb6..ada5222283 100644 --- 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 index c8c68ff44b..62b3fc6b1f 100644 --- 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 = "
replace me
" + 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('
replaced
') + end + def test_inner_html html = Nokogiri::HTML <<-EOHTML diff --git a/test/xml/test_document.rb b/test/xml/test_document.rb index fd748ec21c..fe81b04852 100644 --- 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 = "
replace me
" + 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('
replaced
') + end + # issue #835 def test_manually_adding_reference_entities d = Nokogiri::XML::Document.new