Fix raw memory leak in XML Error copy-assignment

aws · Mar 25, 2024 · 832ce62 · 832ce62
1 parent 00902aa
commit 832ce62
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 3 deletions.
diff --git a/src/aws-cpp-sdk-core/include/aws/core/utils/xml/XmlSerializer.h b/src/aws-cpp-sdk-core/include/aws/core/utils/xml/XmlSerializer.h
@@ -142,8 +142,8 @@ namespace Aws
                 //we do not own these.... I just had to change it from ref because the compiler was
                 //confused about which assignment operator to call. Do not... I repeat... do not delete
                 //these pointers in your destructor.
-                Aws::External::tinyxml2::XMLNode* m_node;
-                const XmlDocument* m_doc;
+                Aws::External::tinyxml2::XMLNode* m_node = nullptr;
+                const XmlDocument* m_doc = nullptr;
 
                 friend class XmlDocument;
             };
@@ -200,7 +200,7 @@ namespace Aws
             private:
                 void InitDoc();
 
-                Aws::External::tinyxml2::XMLDocument* m_doc;
+                Aws::External::tinyxml2::XMLDocument* m_doc = nullptr;
 
                 friend class XmlNode;
 

diff --git a/src/aws-cpp-sdk-core/source/utils/xml/XmlSerializer.cpp b/src/aws-cpp-sdk-core/source/utils/xml/XmlSerializer.cpp
@@ -134,19 +134,22 @@ void XmlNode::SetText(const Aws::String& textValue)
 {
     if (m_node != nullptr)
     {
+        assert(m_doc && m_doc->m_doc == m_node->GetDocument());
         Aws::External::tinyxml2::XMLText* text = m_doc->m_doc->NewText(textValue.c_str());
         m_node->InsertEndChild(text);
     }
 }
 
 XmlNode XmlNode::CreateChildElement(const Aws::String& name)
 {
+    assert(m_doc && m_doc->m_doc == m_node->GetDocument());
     Aws::External::tinyxml2::XMLElement* element = m_doc->m_doc->NewElement(name.c_str());
     return XmlNode(m_node->InsertEndChild(element), *m_doc);
 }
 
 XmlNode XmlNode::CreateSiblingElement(const Aws::String& name)
 {
+    assert(m_doc && m_doc->m_doc == m_node->GetDocument());
     Aws::External::tinyxml2::XMLElement* element = m_doc->m_doc->NewElement(name.c_str());
     return XmlNode(m_node->Parent()->InsertEndChild(element), *m_doc);
 }
@@ -193,6 +196,7 @@ XmlDocument& XmlDocument::operator=(const XmlDocument& other)
         if (m_doc != nullptr)
         {
             m_doc->Clear();
+            Aws::Delete(m_doc);
             m_doc = nullptr;
         }
     }
@@ -228,11 +232,13 @@ XmlDocument::~XmlDocument()
     if (m_doc)
     {
         Aws::Delete(m_doc);
+        m_doc = nullptr;
     }
 }
 
 void XmlDocument::InitDoc()
 {
+    assert(!m_doc);
     m_doc = Aws::New<Aws::External::tinyxml2::XMLDocument>(XML_SERIALIZER_ALLOCATION_TAG, true, Aws::External::tinyxml2::Whitespace::PRESERVE_WHITESPACE);
 }
 

diff --git a/tests/aws-cpp-sdk-core-tests/aws/client/AWSErrorMashallerTest.cpp b/tests/aws-cpp-sdk-core-tests/aws/client/AWSErrorMashallerTest.cpp
@@ -183,6 +183,9 @@ TEST_F(AWSErrorMarshallerTest, TestXmlErrorPayload)
     ASSERT_EQ("", error.GetMessage());
     ASSERT_EQ("", error.GetRequestId());
     ASSERT_TRUE(error.ShouldRetry());
+
+    AWSError<CoreErrors> emptyError;
+    error = emptyError; // ASAN must not complain.
 }
 
 TEST_F(AWSErrorMarshallerTest, TestCombinationsOfJsonErrorPayload)