Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Node#dup adds the new node to the document's node cache (backport to v1.17.x) #3364

Merged
merged 1 commit into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading