From 156de02596e91490bf2432686d0d3c91a5c1a26e Mon Sep 17 00:00:00 2001 From: dsinclair Date: Wed, 24 Aug 2016 11:58:24 -0700 Subject: Removing CPDF_Parser::CloseParser. Currently the only calls to CloseParser() happend in the destructor or the start*Parse methods. The Start*Parse methods are currently only called on freshly constructed parsers in fpdf_dataavail and fpdfview. This CL removes the CloseParser() method and puts the contents in the destructor. We then add an ASSERT that we don't re-enter the parser after it has already completed the parse. Review-Url: https://codereview.chromium.org/2267173005 --- core/fpdfapi/fpdf_parser/cpdf_parser.cpp | 66 +++++++++++--------------- core/fpdfapi/fpdf_parser/include/cpdf_parser.h | 50 ++++++++++--------- fpdfsdk/fpdfview.cpp | 3 +- 3 files changed, 56 insertions(+), 63 deletions(-) diff --git a/core/fpdfapi/fpdf_parser/cpdf_parser.cpp b/core/fpdfapi/fpdf_parser/cpdf_parser.cpp index e2bab450be..9d26104caa 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_parser.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_parser.cpp @@ -51,7 +51,8 @@ int32_t GetStreamFirst(CPDF_StreamAcc* pObjStream) { } // namespace CPDF_Parser::CPDF_Parser() - : m_bOwnFileRead(true), + : m_bHasParsed(false), + m_bOwnFileRead(true), m_FileVersion(0), m_pTrailer(nullptr), m_pEncryptDict(nullptr), @@ -63,7 +64,25 @@ CPDF_Parser::CPDF_Parser() } CPDF_Parser::~CPDF_Parser() { - CloseParser(); + if (m_pTrailer) + m_pTrailer->Release(); + + ReleaseEncryptHandler(); + SetEncryptDictionary(nullptr); + + if (m_bOwnFileRead && m_pSyntax->m_pFileAccess) { + m_pSyntax->m_pFileAccess->Release(); + m_pSyntax->m_pFileAccess = nullptr; + } + + int32_t iLen = m_Trailers.GetSize(); + for (int32_t i = 0; i < iLen; ++i) { + if (CPDF_Dictionary* trailer = m_Trailers.GetAt(i)) + trailer->Release(); + } + + if (m_pLinearized) + m_pLinearized->Release(); } uint32_t CPDF_Parser::GetLastObjNum() const { @@ -124,43 +143,10 @@ void CPDF_Parser::ShrinkObjectMap(uint32_t objnum) { m_ObjectInfo[objnum - 1].pos = 0; } -void CPDF_Parser::CloseParser() { - m_bVersionUpdated = false; - m_pDocument = nullptr; - - if (m_pTrailer) { - m_pTrailer->Release(); - m_pTrailer = nullptr; - } - ReleaseEncryptHandler(); - SetEncryptDictionary(nullptr); - - if (m_bOwnFileRead && m_pSyntax->m_pFileAccess) { - m_pSyntax->m_pFileAccess->Release(); - m_pSyntax->m_pFileAccess = nullptr; - } - - m_ObjectStreamMap.clear(); - m_ObjCache.clear(); - m_SortedOffset.clear(); - m_ObjectInfo.clear(); - - int32_t iLen = m_Trailers.GetSize(); - for (int32_t i = 0; i < iLen; ++i) { - if (CPDF_Dictionary* trailer = m_Trailers.GetAt(i)) - trailer->Release(); - } - m_Trailers.RemoveAll(); - - if (m_pLinearized) { - m_pLinearized->Release(); - m_pLinearized = nullptr; - } -} - CPDF_Parser::Error CPDF_Parser::StartParse(IFX_FileRead* pFileAccess, CPDF_Document* pDocument) { - CloseParser(); + ASSERT(!m_bHasParsed); + m_bHasParsed = true; m_bXRefStream = FALSE; m_LastXRefOffset = 0; @@ -1550,7 +1536,8 @@ FX_BOOL CPDF_Parser::IsLinearizedFile(IFX_FileRead* pFileAccess, CPDF_Parser::Error CPDF_Parser::StartLinearizedParse(IFX_FileRead* pFileAccess, CPDF_Document* pDocument) { - CloseParser(); + ASSERT(!m_bHasParsed); + m_bXRefStream = FALSE; m_LastXRefOffset = 0; m_bOwnFileRead = true; @@ -1563,8 +1550,9 @@ CPDF_Parser::Error CPDF_Parser::StartLinearizedParse(IFX_FileRead* pFileAccess, m_pSyntax->m_pFileAccess = nullptr; return StartParse(pFileAccess, std::move(pDocument)); } - + m_bHasParsed = true; m_pDocument = pDocument; + FX_FILESIZE dwFirstXRefOffset = m_pSyntax->SavePos(); FX_BOOL bXRefRebuilt = FALSE; diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_parser.h b/core/fpdfapi/fpdf_parser/include/cpdf_parser.h index d6a5d5703b..3d2408fad1 100644 --- a/core/fpdfapi/fpdf_parser/include/cpdf_parser.h +++ b/core/fpdfapi/fpdf_parser/include/cpdf_parser.h @@ -95,14 +95,37 @@ class CPDF_Parser { uint16_t gennum; }; - void CloseParser(); + std::unique_ptr m_pSyntax; + std::map m_ObjectInfo; + + bool LoadCrossRefV4(FX_FILESIZE pos, FX_FILESIZE streampos, FX_BOOL bSkip); + FX_BOOL RebuildCrossRef(); + + private: + friend class CPDF_DataAvail; + + enum class ParserState { + kDefault, + kComment, + kWhitespace, + kString, + kHexString, + kEscapedString, + kXref, + kObjNum, + kPostObjNum, + kGenNum, + kPostGenNum, + kTrailer, + kBeginObj, + kEndObj + }; + CPDF_Object* ParseDirect(CPDF_Object* pObj); FX_BOOL LoadAllCrossRefV4(FX_FILESIZE pos); FX_BOOL LoadAllCrossRefV5(FX_FILESIZE pos); - bool LoadCrossRefV4(FX_FILESIZE pos, FX_FILESIZE streampos, FX_BOOL bSkip); FX_BOOL LoadCrossRefV5(FX_FILESIZE* pos, FX_BOOL bMainXRef); CPDF_Dictionary* LoadTrailerV4(); - FX_BOOL RebuildCrossRef(); Error SetEncryptHandler(); void ReleaseEncryptHandler(); FX_BOOL LoadLinearizedAllCrossRefV4(FX_FILESIZE pos, uint32_t dwObjCount); @@ -118,7 +141,7 @@ class CPDF_Parser { bool VerifyCrossRefV4(); CPDF_Document* m_pDocument; // not owned - std::unique_ptr m_pSyntax; + bool m_bHasParsed; bool m_bOwnFileRead; int m_FileVersion; CPDF_Dictionary* m_pTrailer; @@ -127,7 +150,6 @@ class CPDF_Parser { FX_BOOL m_bXRefStream; std::unique_ptr m_pSecurityHandler; CFX_ByteString m_Password; - std::map m_ObjectInfo; std::set m_SortedOffset; CFX_ArrayTemplate m_Trailers; bool m_bVersionUpdated; @@ -149,25 +171,7 @@ class CPDF_Parser { // All indirect object numbers that are being parsed. std::set m_ParsingObjNums; - friend class CPDF_DataAvail; - private: - enum class ParserState { - kDefault, - kComment, - kWhitespace, - kString, - kHexString, - kEscapedString, - kXref, - kObjNum, - kPostObjNum, - kGenNum, - kPostGenNum, - kTrailer, - kBeginObj, - kEndObj - }; }; #endif // CORE_FPDFAPI_FPDF_PARSER_INCLUDE_CPDF_PARSER_H_ diff --git a/fpdfsdk/fpdfview.cpp b/fpdfsdk/fpdfview.cpp index 0c3a95d094..dee71ac9e2 100644 --- a/fpdfsdk/fpdfview.cpp +++ b/fpdfsdk/fpdfview.cpp @@ -367,7 +367,8 @@ DLLEXPORT FPDF_DOCUMENT STDCALL FPDF_LoadDocument(FPDF_STRING file_path, std::unique_ptr pDocument( new CPDF_Document(std::move(pParser))); - CPDF_Parser::Error error = pParser->StartParse(pFileAccess, pDocument.get()); + CPDF_Parser::Error error = + pDocument->GetParser()->StartParse(pFileAccess, pDocument.get()); if (error != CPDF_Parser::SUCCESS) { ProcessParseError(error); return nullptr; -- cgit v1.2.3