Cleanup XFA document properly on failed load

When we fail to parse an XFA document we would free the XML document
that is created immediately. This causes issues because the XML nodes
may have been set into the CXFA_Document already. This CL changes
ParseDoc to always save the XMLDocument and then triggers the CloseDoc()
logic if the ParseDoc method fails.

This should properly cleanup any resources on a failed document load.

Bug: chromium:837578
Change-Id: I8af7e6e34e3b756455c58ea50b22af414ffa6cbf
Reviewed-on: https://pdfium-review.googlesource.com/31710
Commit-Queue: dsinclair <dsinclair@chromium.org>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>

1 files changed, 12 insertions, 3 deletions

diff --git a/xfa/fxfa/cxfa_ffdoc.cpp b/xfa/fxfa/cxfa_ffdoc.cpp
index fb2abe3a72..12e2a7b5f6 100644
--- a/xfa/fxfa/cxfa_ffdoc.cpp
+++ b/xfa/fxfa/cxfa_ffdoc.cpp
@@ -59,11 +59,18 @@ bool CXFA_FFDoc::ParseDoc(CPDF_Object* pElementXFA) {
     return false;
 
   auto stream = pdfium::MakeRetain<CFX_SeekableMultiStream>(xfaStreams);
+
   CXFA_DocumentParser parser(m_pDocument.get());
-  if (!parser.Parse(stream, XFA_PacketType::Xdp))
-    return false;
+  bool parsed = parser.Parse(stream, XFA_PacketType::Xdp);
 
+  // We have to set the XML document before we return so that we can clean
+  // up in the OpenDoc method. If we don't, the XMLDocument will get free'd
+  // when this method returns and UnownedPtrs get unhappy.
   m_pXMLDoc = parser.GetXMLDoc();
+
+  if (!parsed)
+    return false;
+
   m_pDocument->SetRoot(parser.GetRootNode());
   return true;
 }
@@ -104,8 +111,10 @@ bool CXFA_FFDoc::OpenDoc(CPDF_Document* pPDFDoc) {
 
   m_pNotify = pdfium::MakeUnique<CXFA_FFNotify>(this);
   m_pDocument = pdfium::MakeUnique<CXFA_Document>(m_pNotify.get());
-  if (!ParseDoc(pElementXFA))
+  if (!ParseDoc(pElementXFA)) {
+    CloseDoc();
     return false;
+  }
 
   // At this point we've got an XFA document and we want to always return
   // true to signify the load succeeded.