From 5ae6c564d16ce8b625df3d1950abc822f9ecc987 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Wed, 17 Oct 2018 17:57:51 +0000 Subject: Add CPDF_{Array,Dictionary}Locker to catch illegal iteration patterns. Move begin/end methods onto locker object which tracks whether iterators are in existence. Change-Id: Ia869f313fce48d10a0d0180d0cc083eed6ea1584 Reviewed-on: https://pdfium-review.googlesource.com/c/44070 Reviewed-by: Lei Zhang Commit-Queue: Tom Sepez --- core/fpdfapi/edit/cpdf_creator.cpp | 3 +- core/fpdfapi/page/cpdf_allstates.cpp | 3 +- core/fpdfapi/page/cpdf_colorspace.cpp | 3 +- core/fpdfapi/page/cpdf_streamcontentparser.cpp | 51 ++++++++++++++------------ core/fpdfapi/parser/cpdf_array.cpp | 15 ++++++++ core/fpdfapi/parser/cpdf_array.h | 27 ++++++++++++-- core/fpdfapi/parser/cpdf_array_unittest.cpp | 4 +- core/fpdfapi/parser/cpdf_cross_ref_table.cpp | 6 +-- core/fpdfapi/parser/cpdf_data_avail.cpp | 3 +- core/fpdfapi/parser/cpdf_dictionary.cpp | 27 +++++++++++++- core/fpdfapi/parser/cpdf_dictionary.h | 33 +++++++++++++++-- core/fpdfapi/parser/cpdf_object.h | 11 ++---- core/fpdfapi/parser/cpdf_object_unittest.cpp | 6 +-- core/fpdfapi/parser/cpdf_object_walker.cpp | 15 +++++--- core/fpdfapi/parser/fpdf_parser_utility.cpp | 4 +- core/fpdfdoc/cpdf_formcontrol.cpp | 3 +- core/fpdfdoc/cpdf_formfield.cpp | 3 +- core/fpdfdoc/cpdf_interactiveform.cpp | 9 +++-- fpdfsdk/formfiller/cba_fontmap.cpp | 3 +- fpdfsdk/fpdf_annot.cpp | 7 ++-- fpdfsdk/fpdf_editpage.cpp | 3 +- fpdfsdk/fpdf_flatten.cpp | 6 ++- fpdfsdk/fpdf_ppo.cpp | 29 +++++++++------ fpdfsdk/fpdf_transformpage.cpp | 7 ++-- fpdfsdk/fpdf_view.cpp | 3 +- fxjs/cjs_document.cpp | 3 +- xfa/fgas/font/cfgas_pdffontmgr.cpp | 4 +- 27 files changed, 202 insertions(+), 89 deletions(-) diff --git a/core/fpdfapi/edit/cpdf_creator.cpp b/core/fpdfapi/edit/cpdf_creator.cpp index c4bc698acc..7eb7673d69 100644 --- a/core/fpdfapi/edit/cpdf_creator.cpp +++ b/core/fpdfapi/edit/cpdf_creator.cpp @@ -445,7 +445,8 @@ CPDF_Creator::Stage CPDF_Creator::WriteDoc_Stage4() { if (m_pParser) { std::unique_ptr p = m_pParser->GetCombinedTrailer(); - for (const auto& it : *p) { + CPDF_DictionaryLocker locker(p.get()); + for (const auto& it : locker) { const ByteString& key = it.first; CPDF_Object* pValue = it.second.get(); if (key == "Encrypt" || key == "Size" || key == "Filter" || diff --git a/core/fpdfapi/page/cpdf_allstates.cpp b/core/fpdfapi/page/cpdf_allstates.cpp index 9af904cb5d..3e6692b223 100644 --- a/core/fpdfapi/page/cpdf_allstates.cpp +++ b/core/fpdfapi/page/cpdf_allstates.cpp @@ -39,7 +39,8 @@ void CPDF_AllStates::SetLineDash(CPDF_Array* pArray, float phase, float scale) { void CPDF_AllStates::ProcessExtGS(CPDF_Dictionary* pGS, CPDF_StreamContentParser* pParser) { - for (const auto& it : *pGS) { + CPDF_DictionaryLocker locker(pGS); + for (const auto& it : locker) { const ByteString& key_str = it.first; CPDF_Object* pElement = it.second.get(); CPDF_Object* pObject = pElement ? pElement->GetDirect() : nullptr; diff --git a/core/fpdfapi/page/cpdf_colorspace.cpp b/core/fpdfapi/page/cpdf_colorspace.cpp index 2266dec9e8..bcfd42d72f 100644 --- a/core/fpdfapi/page/cpdf_colorspace.cpp +++ b/core/fpdfapi/page/cpdf_colorspace.cpp @@ -461,7 +461,8 @@ std::unique_ptr CPDF_ColorSpace::Load( if (!pDict) return nullptr; - for (const auto& it : *pDict) { + CPDF_DictionaryLocker locker(pDict); + for (const auto& it : locker) { std::unique_ptr pRet; CPDF_Object* pValue = it.second.get(); if (ToName(pValue)) diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.cpp b/core/fpdfapi/page/cpdf_streamcontentparser.cpp index 79fa3d2890..beba208739 100644 --- a/core/fpdfapi/page/cpdf_streamcontentparser.cpp +++ b/core/fpdfapi/page/cpdf_streamcontentparser.cpp @@ -174,36 +174,39 @@ ByteStringView FindFullName(const AbbrPair* table, void ReplaceAbbr(CPDF_Object* pObj) { switch (pObj->GetType()) { case CPDF_Object::DICTIONARY: { - CPDF_Dictionary* pDict = pObj->AsDictionary(); std::vector replacements; - for (const auto& it : *pDict) { - ByteString key = it.first; - CPDF_Object* value = it.second.get(); - ByteStringView fullname = FindFullName( - InlineKeyAbbr, FX_ArraySize(InlineKeyAbbr), key.AsStringView()); - if (!fullname.IsEmpty()) { - AbbrReplacementOp op; - op.is_replace_key = true; - op.key = std::move(key); - op.replacement = fullname; - replacements.push_back(op); - key = fullname; - } - - if (value->IsName()) { - ByteString name = value->GetString(); - fullname = - FindFullName(InlineValueAbbr, FX_ArraySize(InlineValueAbbr), - name.AsStringView()); + CPDF_Dictionary* pDict = pObj->AsDictionary(); + { + CPDF_DictionaryLocker locker(pDict); + for (const auto& it : locker) { + ByteString key = it.first; + CPDF_Object* value = it.second.get(); + ByteStringView fullname = FindFullName( + InlineKeyAbbr, FX_ArraySize(InlineKeyAbbr), key.AsStringView()); if (!fullname.IsEmpty()) { AbbrReplacementOp op; - op.is_replace_key = false; - op.key = key; + op.is_replace_key = true; + op.key = std::move(key); op.replacement = fullname; replacements.push_back(op); + key = fullname; + } + + if (value->IsName()) { + ByteString name = value->GetString(); + fullname = + FindFullName(InlineValueAbbr, FX_ArraySize(InlineValueAbbr), + name.AsStringView()); + if (!fullname.IsEmpty()) { + AbbrReplacementOp op; + op.is_replace_key = false; + op.key = key; + op.replacement = fullname; + replacements.push_back(op); + } + } else { + ReplaceAbbr(value); } - } else { - ReplaceAbbr(value); } } for (const auto& op : replacements) { diff --git a/core/fpdfapi/parser/cpdf_array.cpp b/core/fpdfapi/parser/cpdf_array.cpp index f2eca1be70..e0ac34169f 100644 --- a/core/fpdfapi/parser/cpdf_array.cpp +++ b/core/fpdfapi/parser/cpdf_array.cpp @@ -175,16 +175,19 @@ const CPDF_Array* CPDF_Array::GetArrayAt(size_t i) const { } void CPDF_Array::Clear() { + CHECK(!IsLocked()); m_Objects.clear(); } void CPDF_Array::RemoveAt(size_t i) { + CHECK(!IsLocked()); if (i < m_Objects.size()) m_Objects.erase(m_Objects.begin() + i); } void CPDF_Array::ConvertToIndirectObjectAt(size_t i, CPDF_IndirectObjectHolder* pHolder) { + CHECK(!IsLocked()); if (i >= m_Objects.size()) return; @@ -196,6 +199,7 @@ void CPDF_Array::ConvertToIndirectObjectAt(size_t i, } CPDF_Object* CPDF_Array::SetAt(size_t i, std::unique_ptr pObj) { + CHECK(!IsLocked()); ASSERT(IsArray()); ASSERT(!pObj || pObj->IsInline()); if (i >= m_Objects.size()) { @@ -209,6 +213,7 @@ CPDF_Object* CPDF_Array::SetAt(size_t i, std::unique_ptr pObj) { CPDF_Object* CPDF_Array::InsertAt(size_t index, std::unique_ptr pObj) { + CHECK(!IsLocked()); ASSERT(IsArray()); CHECK(!pObj || pObj->IsInline()); CPDF_Object* pRet = pObj.get(); @@ -224,6 +229,7 @@ CPDF_Object* CPDF_Array::InsertAt(size_t index, } CPDF_Object* CPDF_Array::Add(std::unique_ptr pObj) { + CHECK(!IsLocked()); ASSERT(IsArray()); CHECK(!pObj || pObj->IsInline()); CPDF_Object* pRet = pObj.get(); @@ -242,3 +248,12 @@ bool CPDF_Array::WriteTo(IFX_ArchiveStream* archive, } return archive->WriteString("]"); } + +CPDF_ArrayLocker::CPDF_ArrayLocker(const CPDF_Array* pArray) + : m_pArray(pArray) { + m_pArray->m_LockCount++; +} + +CPDF_ArrayLocker::~CPDF_ArrayLocker() { + m_pArray->m_LockCount--; +} diff --git a/core/fpdfapi/parser/cpdf_array.h b/core/fpdfapi/parser/cpdf_array.h index 0905251382..f7b4a1fc9e 100644 --- a/core/fpdfapi/parser/cpdf_array.h +++ b/core/fpdfapi/parser/cpdf_array.h @@ -108,17 +108,38 @@ class CPDF_Array final : public CPDF_Object { void Clear(); void RemoveAt(size_t index); void ConvertToIndirectObjectAt(size_t index, CPDF_IndirectObjectHolder* pDoc); - - const_iterator begin() const { return m_Objects.begin(); } - const_iterator end() const { return m_Objects.end(); } + bool IsLocked() const { return !!m_LockCount; } private: + friend class CPDF_ArrayLocker; + std::unique_ptr CloneNonCyclic( bool bDirect, std::set* pVisited) const override; std::vector> m_Objects; WeakPtr m_pPool; + mutable uint32_t m_LockCount = 0; +}; + +class CPDF_ArrayLocker { + public: + using const_iterator = CPDF_Array::const_iterator; + + explicit CPDF_ArrayLocker(const CPDF_Array* pArray); + ~CPDF_ArrayLocker(); + + const_iterator begin() const { + CHECK(m_pArray->IsLocked()); + return m_pArray->m_Objects.begin(); + } + const_iterator end() const { + CHECK(m_pArray->IsLocked()); + return m_pArray->m_Objects.end(); + } + + private: + UnownedPtr const m_pArray; }; inline CPDF_Array* ToArray(CPDF_Object* obj) { diff --git a/core/fpdfapi/parser/cpdf_array_unittest.cpp b/core/fpdfapi/parser/cpdf_array_unittest.cpp index a70f9910ed..fbc5c393b2 100644 --- a/core/fpdfapi/parser/cpdf_array_unittest.cpp +++ b/core/fpdfapi/parser/cpdf_array_unittest.cpp @@ -178,7 +178,9 @@ TEST(cpdf_array, Iterator) { for (size_t i = 0; i < FX_ArraySize(elems); ++i) arr->InsertNewAt(i, elems[i]); size_t index = 0; - for (const auto& it : *arr) + + CPDF_ArrayLocker locker(arr.get()); + for (const auto& it : locker) EXPECT_EQ(elems[index++], it->AsNumber()->GetInteger()); EXPECT_EQ(FX_ArraySize(elems), index); } diff --git a/core/fpdfapi/parser/cpdf_cross_ref_table.cpp b/core/fpdfapi/parser/cpdf_cross_ref_table.cpp index 4f7c482c9f..31e9fd3f62 100644 --- a/core/fpdfapi/parser/cpdf_cross_ref_table.cpp +++ b/core/fpdfapi/parser/cpdf_cross_ref_table.cpp @@ -5,6 +5,7 @@ #include "core/fpdfapi/parser/cpdf_cross_ref_table.h" #include +#include #include "core/fpdfapi/parser/cpdf_dictionary.h" #include "core/fpdfapi/parser/cpdf_parser.h" @@ -153,9 +154,6 @@ void CPDF_CrossRefTable::UpdateTrailer( new_trailer->SetFor("XRefStm", trailer_->RemoveFor("XRefStm")); new_trailer->SetFor("Prev", trailer_->RemoveFor("Prev")); - for (auto it = new_trailer->begin(); it != new_trailer->end();) { - const ByteString key = it->first; - ++it; + for (const auto& key : new_trailer->GetKeys()) trailer_->SetFor(key, new_trailer->RemoveFor(key)); - } } diff --git a/core/fpdfapi/parser/cpdf_data_avail.cpp b/core/fpdfapi/parser/cpdf_data_avail.cpp index c046996498..b685214f6f 100644 --- a/core/fpdfapi/parser/cpdf_data_avail.cpp +++ b/core/fpdfapi/parser/cpdf_data_avail.cpp @@ -310,7 +310,8 @@ bool CPDF_DataAvail::CheckPage() { } CPDF_Array* pArray = ToArray(pObj.get()); if (pArray) { - for (const auto& pArrayObj : *pArray) { + CPDF_ArrayLocker locker(pArray); + for (const auto& pArrayObj : locker) { if (CPDF_Reference* pRef = ToReference(pArrayObj.get())) UnavailObjList.push_back(pRef->GetRefObjNum()); } diff --git a/core/fpdfapi/parser/cpdf_dictionary.cpp b/core/fpdfapi/parser/cpdf_dictionary.cpp index 9c06e42804..6c005c8db2 100644 --- a/core/fpdfapi/parser/cpdf_dictionary.cpp +++ b/core/fpdfapi/parser/cpdf_dictionary.cpp @@ -72,7 +72,8 @@ std::unique_ptr CPDF_Dictionary::CloneNonCyclic( std::set* pVisited) const { pVisited->insert(this); auto pCopy = pdfium::MakeUnique(m_pPool); - for (const auto& it : *this) { + CPDF_DictionaryLocker locker(this); + for (const auto& it : locker) { if (!pdfium::ContainsKey(*pVisited, it.second.get())) { std::set visited(*pVisited); if (auto obj = it.second->CloneNonCyclic(bDirect, &visited)) @@ -195,8 +196,17 @@ bool CPDF_Dictionary::KeyExist(const ByteString& key) const { return pdfium::ContainsKey(m_Map, key); } +std::vector CPDF_Dictionary::GetKeys() const { + std::vector result; + CPDF_DictionaryLocker locker(this); + for (const auto& item : locker) + result.push_back(item.first); + return result; +} + CPDF_Object* CPDF_Dictionary::SetFor(const ByteString& key, std::unique_ptr pObj) { + CHECK(!IsLocked()); if (!pObj) { m_Map.erase(key); return nullptr; @@ -210,6 +220,7 @@ CPDF_Object* CPDF_Dictionary::SetFor(const ByteString& key, void CPDF_Dictionary::ConvertToIndirectObjectFor( const ByteString& key, CPDF_IndirectObjectHolder* pHolder) { + CHECK(!IsLocked()); auto it = m_Map.find(key); if (it == m_Map.end() || it->second->IsReference()) return; @@ -219,6 +230,7 @@ void CPDF_Dictionary::ConvertToIndirectObjectFor( } std::unique_ptr CPDF_Dictionary::RemoveFor(const ByteString& key) { + CHECK(!IsLocked()); std::unique_ptr result; auto it = m_Map.find(key); if (it != m_Map.end()) { @@ -230,6 +242,7 @@ std::unique_ptr CPDF_Dictionary::RemoveFor(const ByteString& key) { void CPDF_Dictionary::ReplaceKey(const ByteString& oldkey, const ByteString& newkey) { + CHECK(!IsLocked()); auto old_it = m_Map.find(oldkey); if (old_it == m_Map.end()) return; @@ -273,7 +286,8 @@ bool CPDF_Dictionary::WriteTo(IFX_ArchiveStream* archive, const bool is_signature = CPDF_CryptoHandler::IsSignatureDictionary(this); - for (const auto& it : *this) { + CPDF_DictionaryLocker locker(this); + for (const auto& it : locker) { const ByteString& key = it.first; CPDF_Object* pValue = it.second.get(); if (!archive->WriteString("/") || @@ -288,3 +302,12 @@ bool CPDF_Dictionary::WriteTo(IFX_ArchiveStream* archive, } return archive->WriteString(">>"); } + +CPDF_DictionaryLocker::CPDF_DictionaryLocker(const CPDF_Dictionary* pDictionary) + : m_pDictionary(pDictionary) { + m_pDictionary->m_LockCount++; +} + +CPDF_DictionaryLocker::~CPDF_DictionaryLocker() { + m_pDictionary->m_LockCount--; +} diff --git a/core/fpdfapi/parser/cpdf_dictionary.h b/core/fpdfapi/parser/cpdf_dictionary.h index 2e972d0577..e6abd26cc2 100644 --- a/core/fpdfapi/parser/cpdf_dictionary.h +++ b/core/fpdfapi/parser/cpdf_dictionary.h @@ -11,12 +11,14 @@ #include #include #include +#include #include "core/fpdfapi/parser/cpdf_object.h" #include "core/fxcrt/fx_coordinates.h" #include "core/fxcrt/fx_string.h" #include "core/fxcrt/string_pool_template.h" #include "core/fxcrt/weak_ptr.h" +#include "third_party/base/logging.h" #include "third_party/base/ptr_util.h" class CPDF_IndirectObjectHolder; @@ -41,6 +43,8 @@ class CPDF_Dictionary final : public CPDF_Object { bool WriteTo(IFX_ArchiveStream* archive, const CPDF_Encryptor* encryptor) const override; + bool IsLocked() const { return !!m_LockCount; } + size_t size() const { return m_Map.size(); } const CPDF_Object* GetObjectFor(const ByteString& key) const; CPDF_Object* GetObjectFor(const ByteString& key); @@ -65,6 +69,7 @@ class CPDF_Dictionary final : public CPDF_Object { float GetFloatFor(const ByteString& key) const { return GetNumberFor(key); } bool KeyExist(const ByteString& key) const; + std::vector GetKeys() const; // Set* functions invalidate iterators for the element with the key |key|. // Takes ownership of |pObj|, returns an unowned pointer to it. @@ -76,6 +81,7 @@ class CPDF_Dictionary final : public CPDF_Object { typename std::enable_if::value, T*>::type SetNewFor( const ByteString& key, Args&&... args) { + CHECK(!IsLocked()); return static_cast( SetFor(key, pdfium::MakeUnique(std::forward(args)...))); } @@ -83,6 +89,7 @@ class CPDF_Dictionary final : public CPDF_Object { typename std::enable_if::value, T*>::type SetNewFor( const ByteString& key, Args&&... args) { + CHECK(!IsLocked()); return static_cast(SetFor( key, pdfium::MakeUnique(m_pPool, std::forward(args)...))); } @@ -100,21 +107,41 @@ class CPDF_Dictionary final : public CPDF_Object { // Invalidates iterators for the element with the key |oldkey|. void ReplaceKey(const ByteString& oldkey, const ByteString& newkey); - const_iterator begin() const { return m_Map.begin(); } - const_iterator end() const { return m_Map.end(); } - WeakPtr GetByteStringPool() const { return m_pPool; } private: + friend class CPDF_DictionaryLocker; + ByteString MaybeIntern(const ByteString& str); std::unique_ptr CloneNonCyclic( bool bDirect, std::set* visited) const override; + mutable uint32_t m_LockCount = 0; WeakPtr m_pPool; std::map> m_Map; }; +class CPDF_DictionaryLocker { + public: + using const_iterator = CPDF_Dictionary::const_iterator; + + explicit CPDF_DictionaryLocker(const CPDF_Dictionary* pDictionary); + ~CPDF_DictionaryLocker(); + + const_iterator begin() const { + CHECK(m_pDictionary->IsLocked()); + return m_pDictionary->m_Map.begin(); + } + const_iterator end() const { + CHECK(m_pDictionary->IsLocked()); + return m_pDictionary->m_Map.end(); + } + + private: + UnownedPtr const m_pDictionary; +}; + inline CPDF_Dictionary* ToDictionary(CPDF_Object* obj) { return obj ? obj->AsDictionary() : nullptr; } diff --git a/core/fpdfapi/parser/cpdf_object.h b/core/fpdfapi/parser/cpdf_object.h index 6c6b583963..4f64aec265 100644 --- a/core/fpdfapi/parser/cpdf_object.h +++ b/core/fpdfapi/parser/cpdf_object.h @@ -115,16 +115,13 @@ class CPDF_Object { CPDF_IndirectObjectHolder* holder) const; protected: - CPDF_Object() : m_ObjNum(0), m_GenNum(0) {} + CPDF_Object() = default; + CPDF_Object(const CPDF_Object& src) = delete; std::unique_ptr CloneObjectNonCyclic(bool bDirect) const; - uint32_t m_ObjNum; - - private: - CPDF_Object(const CPDF_Object& src) {} - - uint32_t m_GenNum; + uint32_t m_ObjNum = 0; + uint32_t m_GenNum = 0; }; template diff --git a/core/fpdfapi/parser/cpdf_object_unittest.cpp b/core/fpdfapi/parser/cpdf_object_unittest.cpp index e5b322ff42..c3ec3aeddb 100644 --- a/core/fpdfapi/parser/cpdf_object_unittest.cpp +++ b/core/fpdfapi/parser/cpdf_object_unittest.cpp @@ -138,9 +138,9 @@ class PDFObjectsTest : public testing::Test { const CPDF_Dictionary* dict2 = obj2->AsDictionary(); if (dict1->size() != dict2->size()) return false; - for (CPDF_Dictionary::const_iterator it = dict1->begin(); - it != dict1->end(); ++it) { - if (!Equal(it->second.get(), dict2->GetObjectFor(it->first))) + CPDF_DictionaryLocker locker1(dict1); + for (const auto& item : locker1) { + if (!Equal(item.second.get(), dict2->GetObjectFor(item.first))) return false; } return true; diff --git a/core/fpdfapi/parser/cpdf_object_walker.cpp b/core/fpdfapi/parser/cpdf_object_walker.cpp index 6d6d489e02..f3e1062d83 100644 --- a/core/fpdfapi/parser/cpdf_object_walker.cpp +++ b/core/fpdfapi/parser/cpdf_object_walker.cpp @@ -37,11 +37,11 @@ class StreamIterator final : public CPDF_ObjectWalker::SubobjectIterator { class DictionaryIterator final : public CPDF_ObjectWalker::SubobjectIterator { public: explicit DictionaryIterator(const CPDF_Dictionary* dictionary) - : SubobjectIterator(dictionary) {} + : SubobjectIterator(dictionary), locker_(dictionary) {} ~DictionaryIterator() override {} bool IsFinished() const override { - return IsStarted() && dict_iterator_ == object()->GetDict()->end(); + return IsStarted() && dict_iterator_ == locker_.end(); } const CPDF_Object* IncrementImpl() override { @@ -55,24 +55,26 @@ class DictionaryIterator final : public CPDF_ObjectWalker::SubobjectIterator { void Start() override { ASSERT(!IsStarted()); - dict_iterator_ = object()->GetDict()->begin(); + dict_iterator_ = locker_.begin(); } ByteString dict_key() const { return dict_key_; } private: CPDF_Dictionary::const_iterator dict_iterator_; + CPDF_DictionaryLocker locker_; ByteString dict_key_; }; class ArrayIterator final : public CPDF_ObjectWalker::SubobjectIterator { public: - explicit ArrayIterator(const CPDF_Array* array) : SubobjectIterator(array) {} + explicit ArrayIterator(const CPDF_Array* array) + : SubobjectIterator(array), locker_(array) {} ~ArrayIterator() override {} bool IsFinished() const override { - return IsStarted() && arr_iterator_ == object()->AsArray()->end(); + return IsStarted() && arr_iterator_ == locker_.end(); } const CPDF_Object* IncrementImpl() override { @@ -83,10 +85,11 @@ class ArrayIterator final : public CPDF_ObjectWalker::SubobjectIterator { return result; } - void Start() override { arr_iterator_ = object()->AsArray()->begin(); } + void Start() override { arr_iterator_ = locker_.begin(); } public: CPDF_Array::const_iterator arr_iterator_; + CPDF_ArrayLocker locker_; }; } // namespace diff --git a/core/fpdfapi/parser/fpdf_parser_utility.cpp b/core/fpdfapi/parser/fpdf_parser_utility.cpp index 4cd02f009e..e6972984ea 100644 --- a/core/fpdfapi/parser/fpdf_parser_utility.cpp +++ b/core/fpdfapi/parser/fpdf_parser_utility.cpp @@ -192,9 +192,9 @@ std::ostream& operator<<(std::ostream& buf, const CPDF_Object* pObj) { break; } case CPDF_Object::DICTIONARY: { - const CPDF_Dictionary* p = pObj->AsDictionary(); + CPDF_DictionaryLocker locker(pObj->AsDictionary()); buf << "<<"; - for (const auto& it : *p) { + for (const auto& it : locker) { const ByteString& key = it.first; CPDF_Object* pValue = it.second.get(); buf << "/" << PDF_NameEncode(key); diff --git a/core/fpdfdoc/cpdf_formcontrol.cpp b/core/fpdfdoc/cpdf_formcontrol.cpp index edd5fe566c..7ff0914590 100644 --- a/core/fpdfdoc/cpdf_formcontrol.cpp +++ b/core/fpdfdoc/cpdf_formcontrol.cpp @@ -48,7 +48,8 @@ ByteString CPDF_FormControl::GetOnStateName() const { if (!pN) return csOn; - for (const auto& it : *pN) { + CPDF_DictionaryLocker locker(pN); + for (const auto& it : locker) { if (it.first != "Off") return it.first; } diff --git a/core/fpdfdoc/cpdf_formfield.cpp b/core/fpdfdoc/cpdf_formfield.cpp index 2b06161b06..89030b4132 100644 --- a/core/fpdfdoc/cpdf_formfield.cpp +++ b/core/fpdfdoc/cpdf_formfield.cpp @@ -796,7 +796,8 @@ bool CPDF_FormField::IsOptionSelected(int iOptIndex) const { if (!pArray) return false; - for (const auto& pObj : *pArray) { + CPDF_ArrayLocker locker(pArray); + for (const auto& pObj : locker) { if (pObj->GetInteger() == iOptIndex) return true; } diff --git a/core/fpdfdoc/cpdf_interactiveform.cpp b/core/fpdfdoc/cpdf_interactiveform.cpp index 403344c0ff..382d3dd314 100644 --- a/core/fpdfdoc/cpdf_interactiveform.cpp +++ b/core/fpdfdoc/cpdf_interactiveform.cpp @@ -154,7 +154,8 @@ CPDF_Font* GetNativeFont(CPDF_Dictionary* pFormDict, if (!pFonts) return nullptr; - for (const auto& it : *pFonts) { + CPDF_DictionaryLocker locker(pFonts); + for (const auto& it : locker) { const ByteString& csKey = it.first; if (!it.second) continue; @@ -194,7 +195,8 @@ bool FindFont(CPDF_Dictionary* pFormDict, if (!pFonts) return false; - for (const auto& it : *pFonts) { + CPDF_DictionaryLocker locker(pFonts); + for (const auto& it : locker) { const ByteString& csKey = it.first; if (!it.second) continue; @@ -230,7 +232,8 @@ bool FindFont(CPDF_Dictionary* pFormDict, if (csFontName.GetLength() > 0) csFontName.Remove(' '); - for (const auto& it : *pFonts) { + CPDF_DictionaryLocker locker(pFonts); + for (const auto& it : locker) { const ByteString& csKey = it.first; if (!it.second) continue; diff --git a/fpdfsdk/formfiller/cba_fontmap.cpp b/fpdfsdk/formfiller/cba_fontmap.cpp index f2e7c840f8..24d1ef9de9 100644 --- a/fpdfsdk/formfiller/cba_fontmap.cpp +++ b/fpdfsdk/formfiller/cba_fontmap.cpp @@ -117,7 +117,8 @@ CPDF_Font* CBA_FontMap::FindResFontSameCharset(const CPDF_Dictionary* pResDict, CPDF_Document* pDocument = GetDocument(); CPDF_Font* pFind = nullptr; - for (const auto& it : *pFonts) { + CPDF_DictionaryLocker locker(pFonts); + for (const auto& it : locker) { const ByteString& csKey = it.first; if (!it.second) continue; diff --git a/fpdfsdk/fpdf_annot.cpp b/fpdfsdk/fpdf_annot.cpp index e4bfab0b70..2c8e68eca7 100644 --- a/fpdfsdk/fpdf_annot.cpp +++ b/fpdfsdk/fpdf_annot.cpp @@ -277,17 +277,18 @@ FPDF_EXPORT int FPDF_CALLCONV FPDFPage_GetAnnotIndex(FPDF_PAGE page, if (!pAnnots) return -1; + CPDF_ArrayLocker locker(pAnnots); CPDF_Dictionary* pDict = pAnnot->GetAnnotDict(); auto it = - std::find_if(pAnnots->begin(), pAnnots->end(), + std::find_if(locker.begin(), locker.end(), [pDict](const std::unique_ptr& candidate) { return candidate->GetDirect() == pDict; }); - if (it == pAnnots->end()) + if (it == locker.end()) return -1; - return it - pAnnots->begin(); + return it - locker.begin(); } FPDF_EXPORT void FPDF_CALLCONV FPDFPage_CloseAnnot(FPDF_ANNOTATION annot) { diff --git a/fpdfsdk/fpdf_editpage.cpp b/fpdfsdk/fpdf_editpage.cpp index 8c9a33d816..dcb3d5e1f0 100644 --- a/fpdfsdk/fpdf_editpage.cpp +++ b/fpdfsdk/fpdf_editpage.cpp @@ -389,7 +389,8 @@ FPDFPageObjMark_GetParamKey(FPDF_PAGEOBJECTMARK mark, if (!pParams) return false; - for (auto& it : *pParams) { + CPDF_DictionaryLocker locker(pParams); + for (auto& it : locker) { if (index == 0) { *out_buflen = Utf16EncodeMaybeCopyAndReturnLength( WideString::FromUTF8(it.first.AsStringView()), buffer, buflen); diff --git a/fpdfsdk/fpdf_flatten.cpp b/fpdfsdk/fpdf_flatten.cpp index de396667f5..98529c53c8 100644 --- a/fpdfsdk/fpdf_flatten.cpp +++ b/fpdfsdk/fpdf_flatten.cpp @@ -90,7 +90,8 @@ int ParserAnnots(CPDF_Document* pSourceDoc, if (!pAnnots) return FLATTEN_NOTHINGTODO; - for (const auto& pAnnot : *pAnnots) { + CPDF_ArrayLocker locker(pAnnots); + for (const auto& pAnnot : locker) { CPDF_Dictionary* pAnnotDic = ToDictionary(pAnnot->GetDirect()); if (!pAnnotDic) continue; @@ -338,7 +339,8 @@ FPDF_EXPORT int FPDF_CALLCONV FPDFPage_Flatten(FPDF_PAGE page, int nFlag) { pAPStream = pAPDic->GetStreamFor(sAnnotState); } else { if (pAPDic->size() > 0) { - CPDF_Object* pFirstObj = pAPDic->begin()->second.get(); + CPDF_DictionaryLocker locker(pAPDic); + CPDF_Object* pFirstObj = locker.begin()->second.get(); if (pFirstObj) { if (pFirstObj->IsReference()) pFirstObj = pFirstObj->GetDirect(); diff --git a/fpdfsdk/fpdf_ppo.cpp b/fpdfsdk/fpdf_ppo.cpp index fd270d997c..5c5930e097 100644 --- a/fpdfsdk/fpdf_ppo.cpp +++ b/fpdfsdk/fpdf_ppo.cpp @@ -360,18 +360,22 @@ bool CPDF_PageOrganizer::UpdateReference(CPDF_Object* pObj, } case CPDF_Object::DICTIONARY: { CPDF_Dictionary* pDict = pObj->AsDictionary(); - auto it = pDict->begin(); - while (it != pDict->end()) { - const ByteString& key = it->first; - CPDF_Object* pNextObj = it->second.get(); - ++it; - if (key == "Parent" || key == "Prev" || key == "First") - continue; - if (!pNextObj) - return false; - if (!UpdateReference(pNextObj, pObjNumberMap)) - pDict->RemoveFor(key); + std::vector bad_keys; + { + CPDF_DictionaryLocker locker(pDict); + for (auto it = locker.begin(); it != locker.end(); ++it) { + const ByteString& key = it->first; + if (key == "Parent" || key == "Prev" || key == "First") + continue; + CPDF_Object* pNextObj = it->second.get(); + if (!pNextObj) + return false; + if (!UpdateReference(pNextObj, pObjNumberMap)) + bad_keys.push_back(key); + } } + for (const auto& key : bad_keys) + pDict->RemoveFor(key); break; } case CPDF_Object::ARRAY: { @@ -471,7 +475,8 @@ bool CPDF_PageExporter::ExportPage(const std::vector& pageNums, return false; // Clone the page dictionary - for (const auto& it : *pSrcPageDict) { + CPDF_DictionaryLocker locker(pSrcPageDict); + for (const auto& it : locker) { const ByteString& cbSrcKeyStr = it.first; if (cbSrcKeyStr == pdfium::page_object::kType || cbSrcKeyStr == pdfium::page_object::kParent) { diff --git a/fpdfsdk/fpdf_transformpage.cpp b/fpdfsdk/fpdf_transformpage.cpp index 8e8c9d5b70..debf38b47e 100644 --- a/fpdfsdk/fpdf_transformpage.cpp +++ b/fpdfsdk/fpdf_transformpage.cpp @@ -219,11 +219,12 @@ FPDFPage_TransFormWithClip(FPDF_PAGE page, if (!pRes) return true; - CPDF_Dictionary* pPattenDict = pRes->GetDictFor("Pattern"); - if (!pPattenDict) + CPDF_Dictionary* pPatternDict = pRes->GetDictFor("Pattern"); + if (!pPatternDict) return true; - for (const auto& it : *pPattenDict) { + CPDF_DictionaryLocker locker(pPatternDict); + for (const auto& it : locker) { CPDF_Object* pObj = it.second.get(); if (pObj->IsReference()) pObj = pObj->GetDirect(); diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp index 83d7e4a6cb..136841d566 100644 --- a/fpdfsdk/fpdf_view.cpp +++ b/fpdfsdk/fpdf_view.cpp @@ -1152,7 +1152,8 @@ FPDF_EXPORT FPDF_DEST FPDF_CALLCONV FPDF_GetNamedDest(FPDF_DOCUMENT document, index -= count; int i = 0; ByteString bsName; - for (const auto& it : *pDest) { + CPDF_DictionaryLocker locker(pDest); + for (const auto& it : locker) { bsName = it.first; pDestObj = it.second.get(); if (!pDestObj) diff --git a/fxjs/cjs_document.cpp b/fxjs/cjs_document.cpp index adf36be403..2ecfd424d2 100644 --- a/fxjs/cjs_document.cpp +++ b/fxjs/cjs_document.cpp @@ -742,7 +742,8 @@ CJS_Result CJS_Document::get_info(CJS_Runtime* pRuntime) { // PutObjectProperty() calls below may re-enter JS and change info dict. auto pCopy = pDictionary->Clone(); - for (const auto& it : *ToDictionary(pCopy.get())) { + CPDF_DictionaryLocker locker(ToDictionary(pCopy.get())); + for (const auto& it : locker) { const ByteString& bsKey = it.first; CPDF_Object* pValueObj = it.second.get(); WideString wsKey = WideString::FromUTF8(bsKey.AsStringView()); diff --git a/xfa/fgas/font/cfgas_pdffontmgr.cpp b/xfa/fgas/font/cfgas_pdffontmgr.cpp index fe4b83ce4c..c1d6ada7a9 100644 --- a/xfa/fgas/font/cfgas_pdffontmgr.cpp +++ b/xfa/fgas/font/cfgas_pdffontmgr.cpp @@ -48,7 +48,9 @@ RetainPtr CFGAS_PDFFontMgr::FindFont(const ByteString& strPsName, ByteString name = strPsName; name.Remove(' '); - for (const auto& it : *pFontSetDict) { + + CPDF_DictionaryLocker locker(pFontSetDict); + for (const auto& it : locker) { const ByteString& key = it.first; CPDF_Object* pObj = it.second.get(); if (!PsNameMatchDRFontName(name.AsStringView(), bBold, bItalic, key, -- cgit v1.2.3