From 125aa55061ccde4ae7351a9a6c7270a15c9e0204 Mon Sep 17 00:00:00 2001 From: Arseny Kapoulkine Date: Thu, 20 Nov 2014 23:39:40 -0800 Subject: 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). --- src/pugixml.cpp | 48 +++++++++++++++++++++++++++++++++++------------ tests/test_dom_modify.cpp | 12 ++++++++++++ 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, "text") CHECK_NODE(doc, STR("<: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("")); +} -- cgit v1.2.3