From 32ea6d0f847dab80e5fc03142ffa2238b552b357 Mon Sep 17 00:00:00 2001 From: Dan Sinclair Date: Wed, 2 May 2018 16:17:32 +0000 Subject: 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 Reviewed-by: Ryan Harrison --- xfa/fxfa/cxfa_ffdoc.cpp | 15 ++++++++++++--- 1 file 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(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(this); m_pDocument = pdfium::MakeUnique(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. -- cgit v1.2.3