From 5f52a41cbf0439c13124810d6a4af9a39a35600d Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Tue, 10 Dec 2024 08:39:13 -0500 Subject: [PATCH] fix: Node#dup adds the new node to the document's node cache to make sure it's marked properly. Also, we make sure the new node is decorated if necessary with its document's decorators. Fixes #3359 --- ext/nokogiri/xml_node.c | 5 +++++ test/xml/test_document_fragment.rb | 12 ++++++++++++ test/xml/test_node.rb | 9 +++++++-- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/ext/nokogiri/xml_node.c b/ext/nokogiri/xml_node.c index 5b62d134e67..111e8f7ec8b 100644 --- a/ext/nokogiri/xml_node.c +++ b/ext/nokogiri/xml_node.c @@ -969,6 +969,7 @@ rb_xml_node_initialize_copy_with_args(VALUE rb_self, VALUE rb_other, VALUE rb_le xmlNodePtr c_self, c_other; int c_level; xmlDocPtr c_new_parent_doc; + VALUE rb_node_cache; Noko_Node_Get_Struct(rb_other, xmlNode, c_other); c_level = (int)NUM2INT(rb_level); @@ -980,6 +981,10 @@ rb_xml_node_initialize_copy_with_args(VALUE rb_self, VALUE rb_other, VALUE rb_le _xml_node_data_ptr_set(rb_self, c_self); noko_xml_document_pin_node(c_self); + rb_node_cache = DOC_NODE_CACHE(c_new_parent_doc); + rb_ary_push(rb_node_cache, rb_self); + rb_funcall(rb_new_parent_doc, id_decorate, 1, rb_self); + return rb_self; } diff --git a/test/xml/test_document_fragment.rb b/test/xml/test_document_fragment.rb index 28407a4b2df..4b883531458 100644 --- a/test/xml/test_document_fragment.rb +++ b/test/xml/test_document_fragment.rb @@ -301,6 +301,18 @@ def test_dup_creates_tree_with_identical_structure assert_equal(original.to_html, duplicate.to_html) end + def test_dup_creates_tree_with_identical_structure_stress + # https://github.com/sparklemotion/nokogiri/issues/3359 + original = Nokogiri::XML::DocumentFragment.parse("

hello

") + duplicate = original.dup + + stress_memory_while do + duplicate.to_html + end + + assert_equal(original.to_html, duplicate.to_html) + end + def test_dup_creates_mutable_tree original = Nokogiri::XML::DocumentFragment.parse("

hello

") duplicate = original.dup diff --git a/test/xml/test_node.rb b/test/xml/test_node.rb index abb11161a34..543e34c08c6 100644 --- a/test/xml/test_node.rb +++ b/test/xml/test_node.rb @@ -243,15 +243,20 @@ def test_dup_different_parent_document doc1 = XML::Document.parse("

hello

") doc2 = XML::Document.parse("
") + x = Module.new { def awesome!; end } + doc2.decorators(XML::Node) << x + doc2.decorate! + div = doc1.at_css("div") copy = div.dup(1, doc2) assert_same(doc2, copy.document) - assert_equal(1, copy.children.length) # deep copy + assert_equal(1, copy.children.length, "expected a deep copy") + assert_respond_to(copy, :awesome!, "expected decorators to be copied") copy = div.dup(0, doc2) - assert_equal(0, copy.children.length) # shallow copy + assert_equal(0, copy.children.length, "expected a shallow copy") end def test_subclass_dup