commit 70df5d0cc50715f1df355dcc0022fa7edfd2e02b
author Tom Sepez <tsepez@chromium.org> Fri Sep 02 19:44:59 2022 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Sep 02 19:44:59 2022 +0000
tree 2e9533153650f8588cd83df5ee7f4566961b1312
parent 8ef3b24edc90841fccca56e92f95e4cd251b59ba

Return retained objects from the indirect object holder, part 1.

We've long considered these objects as permanent for the lifetime
of the document, but there may be ways to trigger a parse, for
example, replacing the object - either in error, or with one of a
higher generation number. So extend the lifetime of the objects
we return to callers.

Change-Id: Ia343f040b48a316d9d0c4513561f2bc0cd45fd97
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/97310
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>

core/fpdfapi/edit/cpdf_creator.cpp

diff --git a/core/fpdfapi/edit/cpdf_creator.cpp b/core/fpdfapi/edit/cpdf_creator.cpp
index 9e45400..cbb7460 100644
--- a/core/fpdfapi/edit/cpdf_creator.cpp
+++ b/core/fpdfapi/edit/cpdf_creator.cpp
@@ -181,12 +181,12 @@
 bool CPDF_Creator::WriteNewObjs() {
   for (size_t i = m_CurObjNum; i < m_NewObjNumArray.size(); ++i) {
     uint32_t objnum = m_NewObjNumArray[i];
-    CPDF_Object* pObj = m_pDocument->GetIndirectObject(objnum);
+    RetainPtr<const CPDF_Object> pObj = m_pDocument->GetIndirectObject(objnum);
     if (!pObj)
       continue;
 
     m_ObjectOffsets[objnum] = m_Archive->CurrentOffset();
-    if (!WriteIndirectObj(pObj->GetObjNum(), pObj))
+    if (!WriteIndirectObj(pObj->GetObjNum(), pObj.Get()))
       return false;
   }
   return true;

core/fpdfapi/page/cpdf_image.cpp

diff --git a/core/fpdfapi/page/cpdf_image.cpp b/core/fpdfapi/page/cpdf_image.cpp
index 806e82d..1637ea4 100644
--- a/core/fpdfapi/page/cpdf_image.cpp
+++ b/core/fpdfapi/page/cpdf_image.cpp
@@ -56,7 +56,7 @@
 
 CPDF_Image::CPDF_Image(CPDF_Document* pDoc, uint32_t dwStreamObjNum)
     : m_pDocument(pDoc),
-      m_pStream(ToStream(pDoc->GetIndirectObject(dwStreamObjNum))) {
+      m_pStream(ToStream(pDoc->GetMutableIndirectObject(dwStreamObjNum))) {
   DCHECK(m_pDocument);
   FinishInitialization();
 }

core/fpdfapi/parser/cpdf_indirect_object_holder.cpp

diff --git a/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp b/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp
index 2184e57..ab9dbeb 100644
--- a/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp
+++ b/core/fpdfapi/parser/cpdf_indirect_object_holder.cpp
@@ -16,7 +16,7 @@
 
 namespace {
 
-CPDF_Object* FilterInvalidObjNum(CPDF_Object* obj) {
+RetainPtr<CPDF_Object> FilterInvalidObjNum(RetainPtr<CPDF_Object> obj) {
   return obj && obj->GetObjNum() != CPDF_Object::kInvalidObjNum ? obj : nullptr;
 }
 
@@ -29,11 +29,19 @@
   m_pByteStringPool.DeleteObject();  // Make weak.
 }
 
-CPDF_Object* CPDF_IndirectObjectHolder::GetIndirectObject(
+RetainPtr<const CPDF_Object> CPDF_IndirectObjectHolder::GetIndirectObject(
     uint32_t objnum) const {
+  return const_cast<CPDF_IndirectObjectHolder*>(this)->GetMutableIndirectObject(
+      objnum);
+}
+
+RetainPtr<CPDF_Object> CPDF_IndirectObjectHolder::GetMutableIndirectObject(
+    uint32_t objnum) {
   auto it = m_IndirectObjs.find(objnum);
-  return (it != m_IndirectObjs.end()) ? FilterInvalidObjNum(it->second.Get())
-                                      : nullptr;
+  if (it == m_IndirectObjs.end())
+    return nullptr;
+
+  return FilterInvalidObjNum(it->second);
 }
 
 CPDF_Object* CPDF_IndirectObjectHolder::GetOrParseIndirectObject(
@@ -44,7 +52,7 @@
   // 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());
+    return FilterInvalidObjNum(insert_result.first->second).Get();
 
   RetainPtr<CPDF_Object> pNewObj = ParseIndirectObject(objnum);
   if (!pNewObj) {
@@ -81,7 +89,7 @@
     return false;
 
   auto& obj_holder = m_IndirectObjs[objnum];
-  const CPDF_Object* old_object = FilterInvalidObjNum(obj_holder.Get());
+  RetainPtr<const CPDF_Object> old_object = FilterInvalidObjNum(obj_holder);
   if (old_object && pObj->GetGenNum() <= old_object->GetGenNum())
     return false;
 
@@ -93,7 +101,7 @@
 
 void CPDF_IndirectObjectHolder::DeleteIndirectObject(uint32_t objnum) {
   auto it = m_IndirectObjs.find(objnum);
-  if (it == m_IndirectObjs.end() || !FilterInvalidObjNum(it->second.Get()))
+  if (it == m_IndirectObjs.end() || !FilterInvalidObjNum(it->second))
     return;
 
   m_IndirectObjs.erase(it);

core/fpdfapi/parser/cpdf_indirect_object_holder.h

diff --git a/core/fpdfapi/parser/cpdf_indirect_object_holder.h b/core/fpdfapi/parser/cpdf_indirect_object_holder.h
index 8b630cf..8154520 100644
--- a/core/fpdfapi/parser/cpdf_indirect_object_holder.h
+++ b/core/fpdfapi/parser/cpdf_indirect_object_holder.h
@@ -26,8 +26,10 @@
   CPDF_IndirectObjectHolder();
   virtual ~CPDF_IndirectObjectHolder();
 
-  CPDF_Object* GetIndirectObject(uint32_t objnum) const;
   virtual CPDF_Object* GetOrParseIndirectObject(uint32_t objnum);
+
+  RetainPtr<const CPDF_Object> GetIndirectObject(uint32_t objnum) const;
+  RetainPtr<CPDF_Object> GetMutableIndirectObject(uint32_t objnum);
   void DeleteIndirectObject(uint32_t objnum);
 
   // Creates and adds a new object owned by the indirect object holder,