From e0b592236db902e3e8cbca7ec64f8e2b192e1935 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Wed, 12 Apr 2017 16:50:51 -0700 Subject: Refcount CPDF_CryptoHandler Avoid tracking ownership via m_bLocalCryptoHandler. Also remove m_bEncryptCloned, as it is always false. Replace some methods with direct calls to underlying code. Change-Id: Ifa9d6f721c59d07e3b8e258f76832ca9f2ea0fc9 Reviewed-on: https://pdfium-review.googlesource.com/4111 Reviewed-by: Lei Zhang Commit-Queue: Lei Zhang --- core/fpdfapi/edit/cpdf_creator.h | 6 +---- core/fpdfapi/edit/fpdf_edit_create.cpp | 36 +++++++-------------------- core/fpdfapi/parser/cpdf_crypto_handler.h | 16 ++++++------ core/fpdfapi/parser/cpdf_parser.cpp | 11 ++++---- core/fpdfapi/parser/cpdf_parser.h | 2 +- core/fpdfapi/parser/cpdf_security_handler.cpp | 4 --- core/fpdfapi/parser/cpdf_security_handler.h | 3 +-- core/fpdfapi/parser/cpdf_syntax_parser.cpp | 6 ++--- core/fpdfapi/parser/cpdf_syntax_parser.h | 4 +-- 9 files changed, 31 insertions(+), 57 deletions(-) diff --git a/core/fpdfapi/edit/cpdf_creator.h b/core/fpdfapi/edit/cpdf_creator.h index 0045a5929f..d3f18a82ab 100644 --- a/core/fpdfapi/edit/cpdf_creator.h +++ b/core/fpdfapi/edit/cpdf_creator.h @@ -44,7 +44,6 @@ class CPDF_Creator { friend class CPDF_XRefStream; bool Create(uint32_t flags); - void ResetStandardSecurity(); void Clear(); void InitOldObjNumOffsets(); @@ -80,10 +79,7 @@ class CPDF_Creator { bool m_bSecurityChanged; CPDF_Dictionary* m_pEncryptDict; uint32_t m_dwEncryptObjNum; - bool m_bEncryptCloned; - CPDF_CryptoHandler* m_pCryptoHandler; - // Whether this owns the crypto handler |m_pCryptoHandler|. - bool m_bLocalCryptoHandler; + CFX_RetainPtr m_pCryptoHandler; CPDF_Object* m_pMetadata; std::unique_ptr m_pXRefStream; int32_t m_ObjectStreamSize; diff --git a/core/fpdfapi/edit/fpdf_edit_create.cpp b/core/fpdfapi/edit/fpdf_edit_create.cpp index d08e4b5302..f643d3f24e 100644 --- a/core/fpdfapi/edit/fpdf_edit_create.cpp +++ b/core/fpdfapi/edit/fpdf_edit_create.cpp @@ -577,7 +577,7 @@ FX_FILESIZE CPDF_ObjectStream::End(CPDF_Creator* pCreator) { tempBuffer << m_Buffer; CPDF_FlateEncoder encoder(tempBuffer.GetBuffer(), tempBuffer.GetLength(), true, false); - CPDF_Encryptor encryptor(pCreator->m_pCryptoHandler, m_dwObjNum, + CPDF_Encryptor encryptor(pCreator->m_pCryptoHandler.Get(), m_dwObjNum, encoder.m_pData.Get(), encoder.m_dwSize); if ((len = pFile->AppendDWord(encryptor.m_dwSize)) < 0) { return -1; @@ -880,9 +880,7 @@ CPDF_Creator::CPDF_Creator(CPDF_Document* pDoc) m_bSecurityChanged(false), m_pEncryptDict(m_pParser ? m_pParser->GetEncryptDict() : nullptr), m_dwEncryptObjNum(0), - m_bEncryptCloned(false), m_pCryptoHandler(m_pParser ? m_pParser->GetCryptoHandler() : nullptr), - m_bLocalCryptoHandler(false), m_pMetadata(nullptr), m_ObjectStreamSize(200), m_dwLastObjNum(m_pDocument->GetLastObjNum()), @@ -896,11 +894,6 @@ CPDF_Creator::CPDF_Creator(CPDF_Document* pDoc) m_FileVersion(0) {} CPDF_Creator::~CPDF_Creator() { - ResetStandardSecurity(); - if (m_bEncryptCloned) { - delete m_pEncryptDict; - m_pEncryptDict = nullptr; - } Clear(); } @@ -1025,7 +1018,7 @@ int32_t CPDF_Creator::WriteIndirectObj(uint32_t objnum, m_Offset += len; if (pObj->IsStream()) { CPDF_CryptoHandler* pHandler = - pObj != m_pMetadata ? m_pCryptoHandler : nullptr; + pObj != m_pMetadata ? m_pCryptoHandler.Get() : nullptr; if (WriteStream(pObj, objnum, pHandler) < 0) return -1; } else { @@ -1086,8 +1079,8 @@ int32_t CPDF_Creator::WriteDirectObj(uint32_t objnum, m_Offset += len; break; } - CPDF_Encryptor encryptor(m_pCryptoHandler, objnum, (uint8_t*)str.c_str(), - str.GetLength()); + CPDF_Encryptor encryptor(m_pCryptoHandler.Get(), objnum, + (uint8_t*)str.c_str(), str.GetLength()); CFX_ByteString content = PDF_EncodeString( CFX_ByteString((const char*)encryptor.m_pData, encryptor.m_dwSize), bHex); @@ -1100,8 +1093,8 @@ int32_t CPDF_Creator::WriteDirectObj(uint32_t objnum, case CPDF_Object::STREAM: { CPDF_FlateEncoder encoder(const_cast(pObj->AsStream()), true); - CPDF_Encryptor encryptor(m_pCryptoHandler, objnum, encoder.m_pData.Get(), - encoder.m_dwSize); + CPDF_Encryptor encryptor(m_pCryptoHandler.Get(), objnum, + encoder.m_pData.Get(), encoder.m_dwSize); if (static_cast(encoder.m_pDict->GetIntegerFor("Length")) != encryptor.m_dwSize) { encoder.CloneDict(); @@ -1860,11 +1853,8 @@ void CPDF_Creator::InitID(bool bDefault) { CPDF_SecurityHandler handler; handler.OnCreate(m_pEncryptDict, m_pIDArray.get(), user_pass.raw_str(), user_pass.GetLength(), flag); - if (m_bLocalCryptoHandler) - delete m_pCryptoHandler; - m_pCryptoHandler = new CPDF_CryptoHandler; + m_pCryptoHandler = pdfium::MakeRetain(); m_pCryptoHandler->Init(m_pEncryptDict, &handler); - m_bLocalCryptoHandler = true; m_bSecurityChanged = true; } } @@ -1902,17 +1892,9 @@ bool CPDF_Creator::SetFileVersion(int32_t fileVersion) { m_FileVersion = fileVersion; return true; } + void CPDF_Creator::RemoveSecurity() { - ResetStandardSecurity(); + m_pCryptoHandler.Reset(); m_bSecurityChanged = true; m_pEncryptDict = nullptr; - m_pCryptoHandler = nullptr; -} -void CPDF_Creator::ResetStandardSecurity() { - if (!m_bLocalCryptoHandler) - return; - - delete m_pCryptoHandler; - m_pCryptoHandler = nullptr; - m_bLocalCryptoHandler = false; } diff --git a/core/fpdfapi/parser/cpdf_crypto_handler.h b/core/fpdfapi/parser/cpdf_crypto_handler.h index 1e3890a55b..24caff7081 100644 --- a/core/fpdfapi/parser/cpdf_crypto_handler.h +++ b/core/fpdfapi/parser/cpdf_crypto_handler.h @@ -7,6 +7,7 @@ #ifndef CORE_FPDFAPI_PARSER_CPDF_CRYPTO_HANDLER_H_ #define CORE_FPDFAPI_PARSER_CPDF_CRYPTO_HANDLER_H_ +#include "core/fxcrt/cfx_retain_ptr.h" #include "core/fxcrt/fx_basic.h" #include "core/fxcrt/fx_string.h" #include "core/fxcrt/fx_system.h" @@ -14,10 +15,10 @@ class CPDF_Dictionary; class CPDF_SecurityHandler; -class CPDF_CryptoHandler { +class CPDF_CryptoHandler : public CFX_Retainable { public: - CPDF_CryptoHandler(); - ~CPDF_CryptoHandler(); + template + friend CFX_RetainPtr pdfium::MakeRetain(Args&&... args); bool Init(CPDF_Dictionary* pEncryptDict, CPDF_SecurityHandler* pSecurityHandler); @@ -44,7 +45,11 @@ class CPDF_CryptoHandler { bool Init(int cipher, const uint8_t* key, int keylen); - protected: + private: + CPDF_CryptoHandler(); + ~CPDF_CryptoHandler() override; + + void PopulateKey(uint32_t objnum, uint32_t gennum, uint8_t* key); void CryptBlock(bool bEncrypt, uint32_t objnum, uint32_t gennum, @@ -64,9 +69,6 @@ class CPDF_CryptoHandler { int m_KeyLen; int m_Cipher; uint8_t* m_pAESContext; - - private: - void PopulateKey(uint32_t objnum, uint32_t gennum, uint8_t* key); }; #endif // CORE_FPDFAPI_PARSER_CPDF_CRYPTO_HANDLER_H_ diff --git a/core/fpdfapi/parser/cpdf_parser.cpp b/core/fpdfapi/parser/cpdf_parser.cpp index ecdeeacb19..f009798058 100644 --- a/core/fpdfapi/parser/cpdf_parser.cpp +++ b/core/fpdfapi/parser/cpdf_parser.cpp @@ -100,8 +100,8 @@ void CPDF_Parser::SetEncryptDictionary(CPDF_Dictionary* pDict) { m_pEncryptDict = pDict; } -CPDF_CryptoHandler* CPDF_Parser::GetCryptoHandler() { - return m_pSyntax->m_pCryptoHandler.get(); +CFX_RetainPtr CPDF_Parser::GetCryptoHandler() const { + return m_pSyntax->m_pCryptoHandler; } CFX_RetainPtr CPDF_Parser::GetFileAccess() const { @@ -249,17 +249,16 @@ CPDF_Parser::Error CPDF_Parser::SetEncryptHandler() { return PASSWORD_ERROR; m_pSecurityHandler = std::move(pSecurityHandler); - std::unique_ptr pCryptoHandler( - m_pSecurityHandler->CreateCryptoHandler()); + auto pCryptoHandler = pdfium::MakeRetain(); if (!pCryptoHandler->Init(m_pEncryptDict, m_pSecurityHandler.get())) return HANDLER_ERROR; - m_pSyntax->SetEncrypt(std::move(pCryptoHandler)); + m_pSyntax->SetEncrypt(pCryptoHandler); } return SUCCESS; } void CPDF_Parser::ReleaseEncryptHandler() { - m_pSyntax->m_pCryptoHandler.reset(); + m_pSyntax->m_pCryptoHandler.Reset(); m_pSecurityHandler.reset(); } diff --git a/core/fpdfapi/parser/cpdf_parser.h b/core/fpdfapi/parser/cpdf_parser.h index 47c2c9b885..7ae2d4627e 100644 --- a/core/fpdfapi/parser/cpdf_parser.h +++ b/core/fpdfapi/parser/cpdf_parser.h @@ -71,7 +71,7 @@ class CPDF_Parser { uint16_t GetObjectGenNum(uint32_t objnum) const; bool IsVersionUpdated() const { return m_bVersionUpdated; } bool IsObjectFreeOrNull(uint32_t objnum) const; - CPDF_CryptoHandler* GetCryptoHandler(); + CFX_RetainPtr GetCryptoHandler() const; CFX_RetainPtr GetFileAccess() const; FX_FILESIZE GetObjectOffset(uint32_t objnum) const; diff --git a/core/fpdfapi/parser/cpdf_security_handler.cpp b/core/fpdfapi/parser/cpdf_security_handler.cpp index e95715b966..4ea5da3497 100644 --- a/core/fpdfapi/parser/cpdf_security_handler.cpp +++ b/core/fpdfapi/parser/cpdf_security_handler.cpp @@ -84,10 +84,6 @@ CPDF_SecurityHandler::CPDF_SecurityHandler() CPDF_SecurityHandler::~CPDF_SecurityHandler() {} -CPDF_CryptoHandler* CPDF_SecurityHandler::CreateCryptoHandler() { - return new CPDF_CryptoHandler; -} - bool CPDF_SecurityHandler::OnInit(CPDF_Parser* pParser, CPDF_Dictionary* pEncryptDict) { m_pParser = pParser; diff --git a/core/fpdfapi/parser/cpdf_security_handler.h b/core/fpdfapi/parser/cpdf_security_handler.h index 1a41b67195..93a4e4ff61 100644 --- a/core/fpdfapi/parser/cpdf_security_handler.h +++ b/core/fpdfapi/parser/cpdf_security_handler.h @@ -7,6 +7,7 @@ #ifndef CORE_FPDFAPI_PARSER_CPDF_SECURITY_HANDLER_H_ #define CORE_FPDFAPI_PARSER_CPDF_SECURITY_HANDLER_H_ +#include "core/fpdfapi/parser/cpdf_crypto_handler.h" #include "core/fxcrt/fx_string.h" #include "core/fxcrt/fx_system.h" @@ -18,7 +19,6 @@ #define PDF_ENCRYPT_CONTENT 0 class CPDF_Array; -class CPDF_CryptoHandler; class CPDF_Dictionary; class CPDF_Parser; @@ -31,7 +31,6 @@ class CPDF_SecurityHandler { uint32_t GetPermissions(); bool GetCryptInfo(int& cipher, const uint8_t*& buffer, int& keylen); bool IsMetadataEncrypted() const; - CPDF_CryptoHandler* CreateCryptoHandler(); void OnCreate(CPDF_Dictionary* pEncryptDict, CPDF_Array* pIdArray, diff --git a/core/fpdfapi/parser/cpdf_syntax_parser.cpp b/core/fpdfapi/parser/cpdf_syntax_parser.cpp index 1c84b859db..ecf2cf6e5b 100644 --- a/core/fpdfapi/parser/cpdf_syntax_parser.cpp +++ b/core/fpdfapi/parser/cpdf_syntax_parser.cpp @@ -635,7 +635,7 @@ std::unique_ptr CPDF_SyntaxParser::ReadStream( const CFX_ByteStringC kEndObjStr("endobj"); CPDF_CryptoHandler* pCryptoHandler = - objnum == m_MetadataObjnum ? nullptr : m_pCryptoHandler.get(); + objnum == m_MetadataObjnum ? nullptr : m_pCryptoHandler.Get(); if (!pCryptoHandler) { bool bSearchForKeyword = true; if (len >= 0) { @@ -911,6 +911,6 @@ FX_FILESIZE CPDF_SyntaxParser::FindTag(const CFX_ByteStringC& tag, } void CPDF_SyntaxParser::SetEncrypt( - std::unique_ptr pCryptoHandler) { - m_pCryptoHandler = std::move(pCryptoHandler); + const CFX_RetainPtr& pCryptoHandler) { + m_pCryptoHandler = pCryptoHandler; } diff --git a/core/fpdfapi/parser/cpdf_syntax_parser.h b/core/fpdfapi/parser/cpdf_syntax_parser.h index 8dd9103f1b..9c2d84070d 100644 --- a/core/fpdfapi/parser/cpdf_syntax_parser.h +++ b/core/fpdfapi/parser/cpdf_syntax_parser.h @@ -51,7 +51,7 @@ class CPDF_SyntaxParser { FX_FILESIZE limit); FX_FILESIZE FindTag(const CFX_ByteStringC& tag, FX_FILESIZE limit); - void SetEncrypt(std::unique_ptr pCryptoHandler); + void SetEncrypt(const CFX_RetainPtr& pCryptoHandler); bool ReadBlock(uint8_t* pBuf, uint32_t size); bool GetCharAt(FX_FILESIZE pos, uint8_t& ch); CFX_ByteString GetNextWord(bool* bIsNumber); @@ -95,7 +95,7 @@ class CPDF_SyntaxParser { uint8_t* m_pFileBuf; uint32_t m_BufSize; FX_FILESIZE m_BufOffset; - std::unique_ptr m_pCryptoHandler; + CFX_RetainPtr m_pCryptoHandler; uint8_t m_WordBuffer[257]; uint32_t m_WordSize; CFX_WeakPtr m_pPool; -- cgit v1.2.3