From d936d2931d86eb029feadd8aefbd3c7e51264a64 Mon Sep 17 00:00:00 2001 From: Dan Sinclair Date: Mon, 9 Apr 2018 17:27:55 +0000 Subject: Move the CFX_XMLParser out of CXFA_SimpleParser This CL removes the CFX_XMLParser from CXFA_SimpleParser and constructs it in the CFX_XMLDoc. Change-Id: Id69b396f091892170a4c4654ac1d0fd65d1efbb6 Reviewed-on: https://pdfium-review.googlesource.com/29970 Reviewed-by: Henrique Nakashima Commit-Queue: dsinclair --- core/fxcrt/xml/cfx_xmldoc.cpp | 20 ++++++-------------- core/fxcrt/xml/cfx_xmldoc.h | 5 ++--- testing/libfuzzer/pdf_xml_fuzzer.cc | 5 +---- xfa/fxfa/parser/cxfa_simple_parser.cpp | 28 ++++++++++------------------ xfa/fxfa/parser/cxfa_simple_parser.h | 2 -- 5 files changed, 19 insertions(+), 41 deletions(-) diff --git a/core/fxcrt/xml/cfx_xmldoc.cpp b/core/fxcrt/xml/cfx_xmldoc.cpp index 236ab05c37..64ed5a98d0 100644 --- a/core/fxcrt/xml/cfx_xmldoc.cpp +++ b/core/fxcrt/xml/cfx_xmldoc.cpp @@ -18,24 +18,17 @@ #include "third_party/base/ptr_util.h" #include "third_party/base/stl_util.h" -CFX_XMLDoc::CFX_XMLDoc() - : m_iStatus(0), m_pRoot(pdfium::MakeUnique()) { +CFX_XMLDoc::CFX_XMLDoc(const RetainPtr& pStream) + : m_iStatus(0), + m_pRoot(pdfium::MakeUnique()), + m_pXMLParser(pdfium::MakeUnique(m_pRoot.get(), pStream)) { + ASSERT(pStream); + m_pRoot->AppendChild(new CFX_XMLInstruction(L"xml")); } CFX_XMLDoc::~CFX_XMLDoc() {} -bool CFX_XMLDoc::LoadXML(std::unique_ptr pXMLParser) { - if (!pXMLParser) - return false; - - m_iStatus = 0; - m_pStream.Reset(); - m_pRoot->DeleteChildren(); - m_pXMLParser = std::move(pXMLParser); - return true; -} - int32_t CFX_XMLDoc::DoLoad() { if (m_iStatus < 100) m_iStatus = m_pXMLParser->DoParser(); @@ -46,4 +39,3 @@ int32_t CFX_XMLDoc::DoLoad() { void CFX_XMLDoc::CloseXML() { m_pXMLParser.reset(); } - diff --git a/core/fxcrt/xml/cfx_xmldoc.h b/core/fxcrt/xml/cfx_xmldoc.h index 8ab6b8fea4..07be2d4e84 100644 --- a/core/fxcrt/xml/cfx_xmldoc.h +++ b/core/fxcrt/xml/cfx_xmldoc.h @@ -16,20 +16,19 @@ class CFX_XMLDoc { public: - CFX_XMLDoc(); + explicit CFX_XMLDoc(const RetainPtr& pStream); ~CFX_XMLDoc(); - bool LoadXML(std::unique_ptr pXMLParser); int32_t DoLoad(); void CloseXML(); CFX_XMLNode* GetRoot() const { return m_pRoot.get(); } + CFX_XMLParser* GetParser() const { return m_pXMLParser.get(); } private: int32_t m_iStatus; std::unique_ptr m_pRoot; std::unique_ptr m_pXMLParser; - RetainPtr m_pStream; }; #endif // CORE_FXCRT_XML_CFX_XMLDOC_H_ diff --git a/testing/libfuzzer/pdf_xml_fuzzer.cc b/testing/libfuzzer/pdf_xml_fuzzer.cc index 3a2889421f..072f86c702 100644 --- a/testing/libfuzzer/pdf_xml_fuzzer.cc +++ b/testing/libfuzzer/pdf_xml_fuzzer.cc @@ -22,10 +22,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { RetainPtr stream = pdfium::MakeRetain(const_cast(data), size); - auto doc = pdfium::MakeUnique(); - if (!doc->LoadXML(pdfium::MakeUnique(doc->GetRoot(), stream))) - return 0; - + auto doc = pdfium::MakeUnique(stream); if (doc->DoLoad() < 100) return 0; diff --git a/xfa/fxfa/parser/cxfa_simple_parser.cpp b/xfa/fxfa/parser/cxfa_simple_parser.cpp index f30ab6b631..de9f74f291 100644 --- a/xfa/fxfa/parser/cxfa_simple_parser.cpp +++ b/xfa/fxfa/parser/cxfa_simple_parser.cpp @@ -350,12 +350,8 @@ int32_t CXFA_SimpleParser::StartParse( wCodePage != FX_CODEPAGE_UTF8) { m_pStream->SetCodePage(FX_CODEPAGE_UTF8); } - m_pXMLDoc = pdfium::MakeUnique(); - auto pNewParser = - pdfium::MakeUnique(m_pXMLDoc->GetRoot(), m_pStream); - m_pXMLParser = pNewParser.get(); - if (!m_pXMLDoc->LoadXML(std::move(pNewParser))) - return XFA_PARSESTATUS_StatusErr; + + m_pXMLDoc = pdfium::MakeUnique(m_pStream); m_bParseStarted = true; m_ePacketID = ePacketID; @@ -373,7 +369,6 @@ int32_t CXFA_SimpleParser::DoParse() { return iRet / 2; m_pRootNode = ParseAsXDPPacket(GetDocumentNode(m_pXMLDoc.get()), m_ePacketID); - m_pXMLParser.Release(); m_pXMLDoc->CloseXML(); m_pStream.Reset(); @@ -385,15 +380,11 @@ int32_t CXFA_SimpleParser::DoParse() { CFX_XMLNode* CXFA_SimpleParser::ParseXMLData(const ByteString& wsXML) { CloseParser(); - m_pXMLDoc = pdfium::MakeUnique(); auto pStream = pdfium::MakeRetain( const_cast(wsXML.raw_str()), wsXML.GetLength()); - auto pParser = - pdfium::MakeUnique(m_pXMLDoc->GetRoot(), pStream); - pParser->m_dwCheckStatus = 0x03; - if (!m_pXMLDoc->LoadXML(std::move(pParser))) - return nullptr; + m_pXMLDoc = pdfium::MakeUnique(pStream); + m_pXMLDoc->GetParser()->m_dwCheckStatus = 0x03; int32_t iRet = m_pXMLDoc->DoLoad(); if (iRet < 0 || iRet >= 100) @@ -662,16 +653,17 @@ CXFA_Node* CXFA_SimpleParser::ParseAsXDPPacket_Form( CFX_XMLElement* pXMLDocumentElement = static_cast(pXMLDocumentNode); WideString wsChecksum = pXMLDocumentElement->GetString(L"checksum"); - if (wsChecksum.GetLength() != 28 || m_pXMLParser->m_dwCheckStatus != 0x03) { + if (wsChecksum.GetLength() != 28 || + m_pXMLDoc->GetParser()->m_dwCheckStatus != 0x03) { return nullptr; } auto pChecksum = pdfium::MakeUnique(); pChecksum->StartChecksum(); - pChecksum->UpdateChecksum(m_pFileRead, m_pXMLParser->m_nStart[0], - m_pXMLParser->m_nSize[0]); - pChecksum->UpdateChecksum(m_pFileRead, m_pXMLParser->m_nStart[1], - m_pXMLParser->m_nSize[1]); + pChecksum->UpdateChecksum(m_pFileRead, m_pXMLDoc->GetParser()->m_nStart[0], + m_pXMLDoc->GetParser()->m_nSize[0]); + pChecksum->UpdateChecksum(m_pFileRead, m_pXMLDoc->GetParser()->m_nStart[1], + m_pXMLDoc->GetParser()->m_nSize[1]); pChecksum->FinishChecksum(); ByteString bsCheck = pChecksum->GetChecksum(); if (bsCheck != wsChecksum.UTF8Encode()) diff --git a/xfa/fxfa/parser/cxfa_simple_parser.h b/xfa/fxfa/parser/cxfa_simple_parser.h index c1015469c9..1300a8c4f2 100644 --- a/xfa/fxfa/parser/cxfa_simple_parser.h +++ b/xfa/fxfa/parser/cxfa_simple_parser.h @@ -16,7 +16,6 @@ class CXFA_Node; class CFX_XMLDoc; class CFX_XMLInstruction; class CFX_XMLNode; -class CFX_XMLParser; class IFX_SeekableStream; class CFX_SeekableStreamProxy; @@ -73,7 +72,6 @@ class CXFA_SimpleParser { XFA_PacketType ePacketID); std::unique_ptr m_pXMLDoc; - UnownedPtr m_pXMLParser; // Owned by |m_pXMLDoc| RetainPtr m_pStream; RetainPtr m_pFileRead; UnownedPtr m_pFactory; -- cgit v1.2.3