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
diff --git a/core/fpdfapi/fpdf_parser/cpdf_parser.cpp b/core/fpdfapi/fpdf_parser/cpdf_parser.cpp index e2bab45..9d26104 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_parser.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_parser.cpp
@@ -51,7 +51,8 @@ } // 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() { - 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 @@ 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 @@ 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 @@ 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 d6a5d57..3d2408f 100644 --- a/core/fpdfapi/fpdf_parser/include/cpdf_parser.h +++ b/core/fpdfapi/fpdf_parser/include/cpdf_parser.h
@@ -95,14 +95,37 @@ uint16_t gennum; }; - void CloseParser(); + std::unique_ptr<CPDF_SyntaxParser> m_pSyntax; + std::map<uint32_t, ObjectInfo> 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 @@ bool VerifyCrossRefV4(); CPDF_Document* m_pDocument; // not owned - std::unique_ptr<CPDF_SyntaxParser> m_pSyntax; + bool m_bHasParsed; bool m_bOwnFileRead; int m_FileVersion; CPDF_Dictionary* m_pTrailer; @@ -127,7 +150,6 @@ FX_BOOL m_bXRefStream; std::unique_ptr<CPDF_SecurityHandler> m_pSecurityHandler; CFX_ByteString m_Password; - std::map<uint32_t, ObjectInfo> m_ObjectInfo; std::set<FX_FILESIZE> m_SortedOffset; CFX_ArrayTemplate<CPDF_Dictionary*> m_Trailers; bool m_bVersionUpdated; @@ -149,25 +171,7 @@ // All indirect object numbers that are being parsed. std::set<uint32_t> 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 0c3a95d..dee71ac 100644 --- a/fpdfsdk/fpdfview.cpp +++ b/fpdfsdk/fpdfview.cpp
@@ -367,7 +367,8 @@ std::unique_ptr<CPDF_Document> 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;