tests: Add memory safety tests for remove_children/attributes

The newly added tests make sure that during node/attribute destruction
we deallocate a few memory pages; this makes sure that we don't read
node data after it's being destroyed.

Also clean up formatting/style in the remove_* implementation a bit.
This commit is contained in:
Arseny Kapoulkine 2019-09-17 20:34:40 -07:00
parent fd7326fb91
commit 6202519ca6
2 changed files with 36 additions and 2 deletions

View File

@ -6066,7 +6066,7 @@ namespace pugi
impl::xml_allocator& alloc = impl::get_allocator(_root); impl::xml_allocator& alloc = impl::get_allocator(_root);
if (!alloc.reserve()) return false; if (!alloc.reserve()) return false;
for (xml_attribute_struct* attr = _root->first_attribute; attr;) for (xml_attribute_struct* attr = _root->first_attribute; attr; )
{ {
xml_attribute_struct* next = attr->next_attribute; xml_attribute_struct* next = attr->next_attribute;
@ -6109,7 +6109,7 @@ namespace pugi
{ {
xml_node_struct* next = child->next_sibling; xml_node_struct* next = child->next_sibling;
destroy_node(child, alloc); impl::destroy_node(child, alloc);
child = next; child = next;
} }

View File

@ -505,6 +505,23 @@ TEST_XML(dom_node_remove_attributes, "<node a1='v1' a2='v2' a3='v3'><child a4='v
CHECK_NODE(node, STR("<node><child/></node>")); CHECK_NODE(node, STR("<node><child/></node>"));
} }
TEST_XML(dom_node_remove_attributes_lots, "<node/>")
{
xml_node node = doc.child(STR("node"));
// this test makes sure we generate at least 2 pages (64K) worth of attribute data
// so that we can trigger page deallocation to make sure code is memory safe
for (size_t i = 0; i < 10000; ++i)
node.append_attribute(STR("a")) = STR("v");
CHECK_STRING(node.attribute(STR("a")).value(), STR("v"));
CHECK(node.remove_attributes());
CHECK_STRING(node.attribute(STR("a")).value(), STR(""));
CHECK_NODE(node, STR("<node/>"));
}
TEST_XML(dom_node_prepend_child, "<node>foo<child/></node>") TEST_XML(dom_node_prepend_child, "<node>foo<child/></node>")
{ {
CHECK(xml_node().prepend_child() == xml_node()); CHECK(xml_node().prepend_child() == xml_node());
@ -733,6 +750,23 @@ TEST_XML(dom_node_remove_children, "<node><n1/><n2/><n3/><child><n4/></child></n
CHECK_NODE(node, STR("<node/>")); CHECK_NODE(node, STR("<node/>"));
} }
TEST_XML(dom_node_remove_children_lots, "<node/>")
{
xml_node node = doc.child(STR("node"));
// this test makes sure we generate at least 2 pages (64K) worth of node data
// so that we can trigger page deallocation to make sure code is memory safe
for (size_t i = 0; i < 10000; ++i)
node.append_child().set_name(STR("n"));
CHECK(node.child(STR("n")));
CHECK(node.remove_children());
CHECK(!node.child(STR("n")));
CHECK_NODE(node, STR("<node/>"));
}
TEST_XML(dom_node_remove_child_complex, "<node id='1'><n1 id1='1' id2='2'/><n2/><n3/><child><n4/></child></node>") TEST_XML(dom_node_remove_child_complex, "<node id='1'><n1 id1='1' id2='2'/><n2/><n3/><child><n4/></child></node>")
{ {
CHECK(doc.child(STR("node")).remove_child(STR("n1"))); CHECK(doc.child(STR("node")).remove_child(STR("n1")));