diff options
Diffstat (limited to 'core')
| -rw-r--r-- | core/fpdfapi/parser/cpdf_indirect_object_holder.cpp | 61 | ||||
| -rw-r--r-- | core/fpdfapi/parser/cpdf_indirect_object_holder_unittest.cpp | 81 |
2 files changed, 120 insertions, 22 deletions
diff --git a/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp b/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp index aee527618f..434470abeb 100644 --- a/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp +++ b/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp @@ -13,6 +13,14 @@ #include "core/fpdfapi/parser/cpdf_parser.h" #include "third_party/base/logging.h" +namespace { + +CPDF_Object* FilterInvalidObjNum(CPDF_Object* obj) { + return obj && obj->GetObjNum() != CPDF_Object::kInvalidObjNum ? obj : nullptr; +} + +} // namespace + CPDF_IndirectObjectHolder::CPDF_IndirectObjectHolder() : m_LastObjNum(0), m_pByteStringPool(pdfium::MakeUnique<CFX_ByteStringPool>()) {} @@ -24,26 +32,30 @@ CPDF_IndirectObjectHolder::~CPDF_IndirectObjectHolder() { CPDF_Object* CPDF_IndirectObjectHolder::GetIndirectObject( uint32_t objnum) const { auto it = m_IndirectObjs.find(objnum); - return it != m_IndirectObjs.end() ? it->second.get() : nullptr; + return (it != m_IndirectObjs.end()) ? FilterInvalidObjNum(it->second.get()) + : nullptr; } CPDF_Object* CPDF_IndirectObjectHolder::GetOrParseIndirectObject( uint32_t objnum) { - if (objnum == 0) + if (objnum == 0 || objnum == CPDF_Object::kInvalidObjNum) return nullptr; - CPDF_Object* pObj = GetIndirectObject(objnum); - if (pObj) - return pObj->GetObjNum() != CPDF_Object::kInvalidObjNum ? pObj : nullptr; + // Add item anyway to prevent recursively parsing of same object. + auto insert_result = m_IndirectObjs.insert(std::make_pair(objnum, nullptr)); + if (!insert_result.second) + return FilterInvalidObjNum(insert_result.first->second.get()); std::unique_ptr<CPDF_Object> pNewObj = ParseIndirectObject(objnum); - if (!pNewObj) + if (!pNewObj) { + m_IndirectObjs.erase(insert_result.first); return nullptr; + } pNewObj->m_ObjNum = objnum; m_LastObjNum = std::max(m_LastObjNum, objnum); - m_IndirectObjs[objnum] = std::move(pNewObj); - return m_IndirectObjs[objnum].get(); + insert_result.first->second = std::move(pNewObj); + return insert_result.first->second.get(); } std::unique_ptr<CPDF_Object> CPDF_IndirectObjectHolder::ParseIndirectObject( @@ -54,38 +66,43 @@ std::unique_ptr<CPDF_Object> CPDF_IndirectObjectHolder::ParseIndirectObject( CPDF_Object* CPDF_IndirectObjectHolder::AddIndirectObject( std::unique_ptr<CPDF_Object> pObj) { CHECK(!pObj->m_ObjNum); - CPDF_Object* pUnowned = pObj.get(); pObj->m_ObjNum = ++m_LastObjNum; - if (m_IndirectObjs[m_LastObjNum]) - m_OrphanObjs.push_back(std::move(m_IndirectObjs[m_LastObjNum])); - m_IndirectObjs[m_LastObjNum] = std::move(pObj); - return pUnowned; + auto& obj_holder = m_IndirectObjs[m_LastObjNum]; + if (obj_holder) + m_OrphanObjs.push_back(std::move(obj_holder)); + + obj_holder = std::move(pObj); + return obj_holder.get(); } bool CPDF_IndirectObjectHolder::ReplaceIndirectObjectIfHigherGeneration( uint32_t objnum, std::unique_ptr<CPDF_Object> pObj) { ASSERT(objnum); - if (!pObj) + if (!pObj || objnum == CPDF_Object::kInvalidObjNum) return false; - CPDF_Object* pOldObj = GetIndirectObject(objnum); - if (pOldObj && pObj->GetGenNum() <= pOldObj->GetGenNum()) + auto& obj_holder = m_IndirectObjs[objnum]; + + const CPDF_Object* old_object = FilterInvalidObjNum(obj_holder.get()); + + if (old_object && pObj->GetGenNum() <= old_object->GetGenNum()) return false; pObj->m_ObjNum = objnum; - if (m_IndirectObjs[objnum]) - m_OrphanObjs.push_back(std::move(m_IndirectObjs[objnum])); - m_IndirectObjs[objnum] = std::move(pObj); + if (obj_holder) + m_OrphanObjs.push_back(std::move(obj_holder)); + + obj_holder = std::move(pObj); m_LastObjNum = std::max(m_LastObjNum, objnum); return true; } void CPDF_IndirectObjectHolder::DeleteIndirectObject(uint32_t objnum) { - CPDF_Object* pObj = GetIndirectObject(objnum); - if (!pObj || pObj->GetObjNum() == CPDF_Object::kInvalidObjNum) + auto it = m_IndirectObjs.find(objnum); + if (it == m_IndirectObjs.end() || !FilterInvalidObjNum(it->second.get())) return; - m_IndirectObjs.erase(objnum); + m_IndirectObjs.erase(it); } diff --git a/core/fpdfapi/parser/cpdf_indirect_object_holder_unittest.cpp b/core/fpdfapi/parser/cpdf_indirect_object_holder_unittest.cpp new file mode 100644 index 0000000000..666264f50b --- /dev/null +++ b/core/fpdfapi/parser/cpdf_indirect_object_holder_unittest.cpp @@ -0,0 +1,81 @@ +// Copyright 2017 PDFium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "core/fpdfapi/parser/cpdf_indirect_object_holder.h" + +#include <memory> +#include <utility> + +#include "core/fpdfapi/parser/cpdf_null.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "third_party/base/ptr_util.h" + +namespace { + +class MockIndirectObjectHolder : public CPDF_IndirectObjectHolder { + public: + MockIndirectObjectHolder() {} + ~MockIndirectObjectHolder() override {} + + MOCK_METHOD1(ParseIndirectObject, + std::unique_ptr<CPDF_Object>(uint32_t objnum)); +}; + +} // namespace + +TEST(CPDF_IndirectObjectHolderTest, RecursiveParseOfSameObject) { + MockIndirectObjectHolder mock_holder; + // ParseIndirectObject should not be called again on recursively same object + // parse request. + EXPECT_CALL(mock_holder, ParseIndirectObject(::testing::_)) + .WillOnce(::testing::WithArg<0>(::testing::Invoke( + [&mock_holder](uint32_t objnum) -> std::unique_ptr<CPDF_Object> { + const CPDF_Object* same_parse = + mock_holder.GetOrParseIndirectObject(objnum); + CHECK(!same_parse); + return pdfium::MakeUnique<CPDF_Null>(); + }))); + + EXPECT_TRUE(mock_holder.GetOrParseIndirectObject(1000)); +} + +TEST(CPDF_IndirectObjectHolderTest, GetObjectMethods) { + static constexpr uint32_t kObjNum = 1000; + MockIndirectObjectHolder mock_holder; + + EXPECT_CALL(mock_holder, ParseIndirectObject(::testing::_)).Times(0); + EXPECT_FALSE(mock_holder.GetIndirectObject(kObjNum)); + ::testing::Mock::VerifyAndClearExpectations(&mock_holder); + + EXPECT_CALL(mock_holder, ParseIndirectObject(::testing::_)) + .WillOnce(::testing::WithArg<0>(::testing::Invoke( + [](uint32_t objnum) -> std::unique_ptr<CPDF_Object> { + return pdfium::MakeUnique<CPDF_Null>(); + }))); + EXPECT_TRUE(mock_holder.GetOrParseIndirectObject(kObjNum)); + ::testing::Mock::VerifyAndClearExpectations(&mock_holder); + + EXPECT_CALL(mock_holder, ParseIndirectObject(::testing::_)).Times(0); + ASSERT_TRUE(mock_holder.GetIndirectObject(kObjNum)); + ::testing::Mock::VerifyAndClearExpectations(&mock_holder); + + EXPECT_EQ(kObjNum, mock_holder.GetIndirectObject(kObjNum)->GetObjNum()); +} + +TEST(CPDF_IndirectObjectHolderTest, ParseInvalidObjNum) { + MockIndirectObjectHolder mock_holder; + + EXPECT_CALL(mock_holder, ParseIndirectObject(::testing::_)).Times(0); + EXPECT_FALSE( + mock_holder.GetOrParseIndirectObject(CPDF_Object::kInvalidObjNum)); +} + +TEST(CPDF_IndirectObjectHolderTest, ReplaceObjectWithInvalidObjNum) { + MockIndirectObjectHolder mock_holder; + + EXPECT_CALL(mock_holder, ParseIndirectObject(::testing::_)).Times(0); + EXPECT_FALSE(mock_holder.ReplaceIndirectObjectIfHigherGeneration( + CPDF_Object::kInvalidObjNum, pdfium::MakeUnique<CPDF_Null>())); +} |