Skip to content

Commit

Permalink
fix: Node#dup adds the new node to the document's node cache (backpor…
Browse files Browse the repository at this point in the history
…t to v1.17.x) (#3364)

Backport of #3363 addressing #3359.
  • Loading branch information
flavorjones authored Dec 10, 2024
2 parents 765754c + 7bf2fc1 commit 12244fe
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 2 deletions.
5 changes: 5 additions & 0 deletions ext/nokogiri/xml_node.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}

Expand Down
14 changes: 14 additions & 0 deletions test/xml/test_document_fragment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,20 @@ 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
skip_unless_libxml2("this is testing CRuby GC behavior")

original = Nokogiri::XML::DocumentFragment.parse("<div><p>hello</p></div>")
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("<div><p>hello</p></div>")
duplicate = original.dup
Expand Down
9 changes: 7 additions & 2 deletions test/xml/test_node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -243,15 +243,20 @@ def test_dup_different_parent_document
doc1 = XML::Document.parse("<root><div><p>hello</p></div></root>")
doc2 = XML::Document.parse("<div></div>")

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
Expand Down

0 comments on commit 12244fe

Please sign in to comment.