Merge pull request #498 from zeux/fix-oom-safer

Fix memory leak during OOM in convert_buffer

src/pugixml.cpp

@@ -4707,16 +4707,18 @@ PUGI__NS_BEGIN
 		// get actual encoding
 		xml_encoding buffer_encoding = impl::get_buffer_encoding(encoding, contents, size);
 
+		// if convert_buffer below throws bad_alloc, we still need to deallocate contents if we own it
+		auto_deleter<void> contents_guard(own ? contents : 0, xml_memory::deallocate);
+
 		// get private buffer
 		char_t* buffer = 0;
 		size_t length = 0;
 
 		// coverity[var_deref_model]
-		if (!impl::convert_buffer(buffer, length, buffer_encoding, contents, size, is_mutable))
+		if (!impl::convert_buffer(buffer, length, buffer_encoding, contents, size, is_mutable)) return impl::make_parse_result(status_out_of_memory);
-		{
+
-			if (own && contents) impl::xml_memory::deallocate(contents);
+		// after this we either deallocate contents (below) or hold on to it via doc->buffer, so we don't need to guard it
-			return impl::make_parse_result(status_out_of_memory);
+		contents_guard.release();
-		}
 
 		// delete original buffer if we performed a conversion
 		if (own && buffer != contents && contents) impl::xml_memory::deallocate(contents);

tests/test_document.cpp

@@ -1817,3 +1817,39 @@ TEST(document_move_assign_empty)
 	CHECK_NODE(doc, STR("<node2/>"));
 }
 #endif
+
+TEST(document_load_buffer_convert_out_of_memory)
+{
+	const char* source = "<node>\xe7</node>";
+	size_t size = strlen(source);
+
+	test_runner::_memory_fail_threshold = 1;
+
+	xml_document doc;
+
+	xml_parse_result result;
+	result.status = status_out_of_memory;
+	CHECK_ALLOC_FAIL(result = doc.load_buffer(source, size, pugi::parse_default, pugi::encoding_latin1));
+
+	CHECK(result.status == status_out_of_memory);
+}
+
+TEST(document_load_buffer_own_convert_out_of_memory)
+{
+	const char* source = "<node>\xe7</node>";
+	size_t size = strlen(source);
+
+	void* buffer = pugi::get_memory_allocation_function()(size);
+	CHECK(buffer);
+	memcpy(buffer, source, size);
+
+	test_runner::_memory_fail_threshold = 1;
+
+	xml_document doc;
+
+	xml_parse_result result;
+	result.status = status_out_of_memory;
+	CHECK_ALLOC_FAIL(result = doc.load_buffer_inplace_own(buffer, size, pugi::parse_default, pugi::encoding_latin1));
+
+	CHECK(result.status == status_out_of_memory);
+}