Fix node_declaration copying with empty name

node_copy_string relied on the fact that target node had an empty name and
value. Normally this is a safe assumption (and a good one to make since it
makes copying faster), however it was not checked and there was one case when
it did not hold.

Since we're reusing the logic for inserting nodes, newly inserted declaration
nodes had the name set automatically to xml, which in our case violates the
assumption and is counter-productive since we'll override the name right after
setting it.

For now the best solution is to do the same insertion manually - that results
in some code duplication that we can refactor later (same logic is partially
shared by _move variants anyway so on a level duplicating is not that bad).

2 files changed, 48 insertions, 12 deletions

diff --git a/src/pugixml.cpp b/src/pugixml.cpp
index 420ac1f..ff84d44 100644
--- a/src/pugixml.cpp
+++ b/src/pugixml.cpp
@@ -3722,6 +3722,8 @@ PUGI__NS_BEGIN
 
 	PUGI__FN void node_copy_string(char_t*& dest, uintptr_t& header, uintptr_t header_mask, char_t* source, uintptr_t& source_header, xml_allocator* alloc)
 	{
+		assert(!dest && (header & header_mask) == 0);
+
 		if (source)
 		{
 			if (alloc && (source_header & header_mask) == 0)
@@ -5069,38 +5071,60 @@ namespace pugi
 
 	PUGI__FN xml_node xml_node::append_copy(const xml_node& proto)
 	{
-		xml_node result = append_child(proto.type());
+		xml_node_type type_ = proto.type();
+		if (!impl::allow_insert_child(type(), type_)) return xml_node();
+
+		xml_node n(impl::allocate_node(impl::get_allocator(_root), type_));
+		if (!n) return xml_node();
 
-		if (result) impl::node_copy_tree(result._root, proto._root);
+		impl::append_node(n._root, _root);
+		impl::node_copy_tree(n._root, proto._root);
 
-		return result;
+		return n;
 	}
 
 	PUGI__FN xml_node xml_node::prepend_copy(const xml_node& proto)
 	{
-		xml_node result = prepend_child(proto.type());
+		xml_node_type type_ = proto.type();
+		if (!impl::allow_insert_child(type(), type_)) return xml_node();
 
-		if (result) impl::node_copy_tree(result._root, proto._root);
+		xml_node n(impl::allocate_node(impl::get_allocator(_root), type_));
+		if (!n) return xml_node();
 
-		return result;
+		impl::prepend_node(n._root, _root);
+		impl::node_copy_tree(n._root, proto._root);
+
+		return n;
 	}
 
 	PUGI__FN xml_node xml_node::insert_copy_after(const xml_node& proto, const xml_node& node)
 	{
-		xml_node result = insert_child_after(proto.type(), node);
+		xml_node_type type_ = proto.type();
+		if (!impl::allow_insert_child(type(), type_)) return xml_node();
+		if (!node._root || node._root->parent != _root) return xml_node();
 
-		if (result) impl::node_copy_tree(result._root, proto._root);
+		xml_node n(impl::allocate_node(impl::get_allocator(_root), type_));
+		if (!n) return xml_node();
 
-		return result;
+		impl::insert_node_after(n._root, node._root);
+		impl::node_copy_tree(n._root, proto._root);
+
+		return n;
 	}
 
 	PUGI__FN xml_node xml_node::insert_copy_before(const xml_node& proto, const xml_node& node)
 	{
-		xml_node result = insert_child_before(proto.type(), node);
+		xml_node_type type_ = proto.type();
+		if (!impl::allow_insert_child(type(), type_)) return xml_node();
+		if (!node._root || node._root->parent != _root) return xml_node();
 
-		if (result) impl::node_copy_tree(result._root, proto._root);
+		xml_node n(impl::allocate_node(impl::get_allocator(_root), type_));
+		if (!n) return xml_node();
 
-		return result;
+		impl::insert_node_before(n._root, node._root);
+		impl::node_copy_tree(n._root, proto._root);
+
+		return n;
 	}
 
 	PUGI__FN xml_node xml_node::append_move(const xml_node& moved)
diff --git a/tests/test_dom_modify.cpp b/tests/test_dom_modify.cpp
index 8665af9..7863718 100644
--- a/tests/test_dom_modify.cpp
+++ b/tests/test_dom_modify.cpp
@@ -1426,3 +1426,15 @@ TEST_XML(dom_node_set_deallocate, "<node attr='value'>text</node>")
 
 	CHECK_NODE(doc, STR("<:anonymous :anonymous=\"\"></:anonymous>"));
 }
+
+TEST(dom_node_copy_declaration_empty_name)
+{
+	xml_document doc1;
+	xml_node decl1 = doc1.append_child(node_declaration);
+	decl1.set_name(STR(""));
+
+	xml_document doc2;
+	xml_node decl2 = doc2.append_copy(decl1);
+
+	CHECK_STRING(decl2.name(), STR(""));
+}