Do not let CPDF_Array::SetAt() and friends take a CPDF_Stream
Use `= delete` to disallow SetAt() from taking a CPDF_Stream directly.
Then update the unit test that does pass in a CPDF_Stream to properly
use CPDF_Reference instead. Do the same for InsertAt() and Append().
Along the way, rename a few variables to follow the style guide.
Bug: pdfium:2119
Change-Id: Idc24223bf2433dbe87167ec0ad21e2cf3b7af5b1
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/115711
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/parser/cpdf_array.cpp b/core/fpdfapi/parser/cpdf_array.cpp
index 55b82e7..3d51a9f 100644
--- a/core/fpdfapi/parser/cpdf_array.cpp
+++ b/core/fpdfapi/parser/cpdf_array.cpp
@@ -214,16 +214,16 @@
m_Objects[index] = m_Objects[index]->MakeReference(pHolder);
}
-void CPDF_Array::SetAt(size_t index, RetainPtr<CPDF_Object> pObj) {
- (void)SetAtInternal(index, std::move(pObj));
+void CPDF_Array::SetAt(size_t index, RetainPtr<CPDF_Object> object) {
+ (void)SetAtInternal(index, std::move(object));
}
-void CPDF_Array::InsertAt(size_t index, RetainPtr<CPDF_Object> pObj) {
- (void)InsertAtInternal(index, std::move(pObj));
+void CPDF_Array::InsertAt(size_t index, RetainPtr<CPDF_Object> object) {
+ (void)InsertAtInternal(index, std::move(object));
}
-void CPDF_Array::Append(RetainPtr<CPDF_Object> pObj) {
- (void)AppendInternal(std::move(pObj));
+void CPDF_Array::Append(RetainPtr<CPDF_Object> object) {
+ (void)AppendInternal(std::move(object));
}
CPDF_Object* CPDF_Array::SetAtInternal(size_t index,
diff --git a/core/fpdfapi/parser/cpdf_array.h b/core/fpdfapi/parser/cpdf_array.h
index ea9a815..2554809 100644
--- a/core/fpdfapi/parser/cpdf_array.h
+++ b/core/fpdfapi/parser/cpdf_array.h
@@ -117,19 +117,22 @@
index, pdfium::MakeRetain<T>(m_pPool, std::forward<Args>(args)...))));
}
- // Adds non-null `pObj` to the end of the array, growing as appropriate.
- void Append(RetainPtr<CPDF_Object> pObj);
+ // Adds non-null `object` to the end of the array, growing as appropriate.
+ void Append(RetainPtr<CPDF_Object> object);
+ void Append(RetainPtr<CPDF_Stream> stream) = delete;
- // Overwrites the object at `index` with non-null `pObj`, if it is
- // in bounds. Otherwise, `index` is out of bounds, and `pObj` is
+ // Overwrites the object at `index` with non-null `object`, if it is
+ // in bounds. Otherwise, `index` is out of bounds, and `object` is
// not stored.
- void SetAt(size_t index, RetainPtr<CPDF_Object> pObj);
+ void SetAt(size_t index, RetainPtr<CPDF_Object> object);
+ void SetAt(size_t index, RetainPtr<CPDF_Stream> stream) = delete;
- // Inserts non-null `pObj` at `index` and shifts by one position all of the
+ // Inserts non-null `object` at `index` and shifts by one position all of the
// objects beyond it like std::vector::insert(), if `index` is less than or
// equal to the current array size. Otherwise, `index` is out of bounds,
- // and `pObj` is not stored.
- void InsertAt(size_t index, RetainPtr<CPDF_Object> pObj);
+ // and `object` is not stored.
+ void InsertAt(size_t index, RetainPtr<CPDF_Object> object);
+ void InsertAt(size_t index, RetainPtr<CPDF_Stream> stream) = delete;
void Clear();
void RemoveAt(size_t index);
diff --git a/core/fpdfapi/parser/cpdf_object_walker_unittest.cpp b/core/fpdfapi/parser/cpdf_object_walker_unittest.cpp
index ab0c2c3..a45c111 100644
--- a/core/fpdfapi/parser/cpdf_object_walker_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_object_walker_unittest.cpp
@@ -64,26 +64,36 @@
}
TEST(ObjectWalkerTest, CombinedObject) {
+ CPDF_IndirectObjectHolder holder;
auto dict = pdfium::MakeRetain<CPDF_Dictionary>();
dict->SetFor("1", pdfium::MakeRetain<CPDF_String>());
dict->SetFor("2", pdfium::MakeRetain<CPDF_Boolean>());
auto array = pdfium::MakeRetain<CPDF_Array>();
array->Append(pdfium::MakeRetain<CPDF_Reference>(nullptr, 0));
array->Append(pdfium::MakeRetain<CPDF_Null>());
+ auto stream =
+ holder.NewIndirect<CPDF_Stream>(pdfium::MakeRetain<CPDF_Dictionary>());
+ ASSERT_TRUE(stream);
+ const uint32_t stream_object_number = stream->GetObjNum();
+ ASSERT_GT(stream_object_number, 0u);
array->Append(
- pdfium::MakeRetain<CPDF_Stream>(pdfium::MakeRetain<CPDF_Dictionary>()));
+ pdfium::MakeRetain<CPDF_Reference>(&holder, stream_object_number));
dict->SetFor("3", std::move(array));
- // The last number for stream length.
- EXPECT_EQ(Walk(dict), "Dict Str Bool Arr Ref Null Stream Dict Num");
+ EXPECT_EQ(Walk(dict), "Dict Str Bool Arr Ref Null Ref");
}
TEST(ObjectWalkerTest, GetParent) {
+ CPDF_IndirectObjectHolder holder;
auto level_4 = pdfium::MakeRetain<CPDF_Number>(0);
auto level_3 = pdfium::MakeRetain<CPDF_Dictionary>();
level_3->SetFor("Length", std::move(level_4));
- auto level_2 = pdfium::MakeRetain<CPDF_Stream>(std::move(level_3));
+ auto level_2 = holder.NewIndirect<CPDF_Stream>(std::move(level_3));
+ ASSERT_TRUE(level_2);
+ const uint32_t level_2_object_number = level_2->GetObjNum();
+ ASSERT_GT(level_2_object_number, 0u);
auto level_1 = pdfium::MakeRetain<CPDF_Array>();
- level_1->Append(std::move(level_2));
+ level_1->Append(
+ pdfium::MakeRetain<CPDF_Reference>(&holder, level_2_object_number));
auto level_0 = pdfium::MakeRetain<CPDF_Dictionary>();
level_0->SetFor("Array", std::move(level_1));
diff --git a/core/fpdfapi/render/cpdf_docrenderdata_unittest.cpp b/core/fpdfapi/render/cpdf_docrenderdata_unittest.cpp
index df7bb1c..88296e6 100644
--- a/core/fpdfapi/render/cpdf_docrenderdata_unittest.cpp
+++ b/core/fpdfapi/render/cpdf_docrenderdata_unittest.cpp
@@ -11,7 +11,9 @@
#include "core/fpdfapi/page/cpdf_transferfunc.h"
#include "core/fpdfapi/parser/cpdf_array.h"
#include "core/fpdfapi/parser/cpdf_dictionary.h"
+#include "core/fpdfapi/parser/cpdf_indirect_object_holder.h"
#include "core/fpdfapi/parser/cpdf_number.h"
+#include "core/fpdfapi/parser/cpdf_reference.h"
#include "core/fpdfapi/parser/cpdf_stream.h"
#include "core/fxcrt/data_vector.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -70,7 +72,8 @@
26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26, 26,
26, 26, 26, 26, 26, 26, 26, 26, 26};
-RetainPtr<CPDF_Stream> CreateType0FunctionStream() {
+RetainPtr<CPDF_Reference> CreateType0FunctionStreamReference(
+ CPDF_IndirectObjectHolder& holder) {
auto func_dict = pdfium::MakeRetain<CPDF_Dictionary>();
func_dict->SetNewFor<CPDF_Number>("FunctionType", 0);
func_dict->SetNewFor<CPDF_Number>("BitsPerSample", 8);
@@ -87,9 +90,10 @@
size_array->AppendNew<CPDF_Number>(4);
static constexpr uint8_t kContents[] = "1234";
- return pdfium::MakeRetain<CPDF_Stream>(
+ auto stream = holder.NewIndirect<CPDF_Stream>(
DataVector<uint8_t>(std::begin(kContents), std::end(kContents)),
std::move(func_dict));
+ return pdfium::MakeRetain<CPDF_Reference>(&holder, stream->GetObjNum());
}
RetainPtr<CPDF_Dictionary> CreateType2FunctionDict() {
@@ -114,7 +118,8 @@
return func_dict;
}
-RetainPtr<CPDF_Stream> CreateType4FunctionStream() {
+RetainPtr<CPDF_Reference> CreateType4FunctionStreamReference(
+ CPDF_IndirectObjectHolder& holder) {
auto func_dict = pdfium::MakeRetain<CPDF_Dictionary>();
func_dict->SetNewFor<CPDF_Number>("FunctionType", 4);
@@ -127,9 +132,10 @@
range_array->AppendNew<CPDF_Number>(1);
static constexpr uint8_t kContents[] = "{ 360 mul sin 2 div }";
- return pdfium::MakeRetain<CPDF_Stream>(
+ auto stream = holder.NewIndirect<CPDF_Stream>(
DataVector<uint8_t>(std::begin(kContents), std::end(kContents)),
std::move(func_dict));
+ return pdfium::MakeRetain<CPDF_Reference>(&holder, stream->GetObjNum());
}
RetainPtr<CPDF_Stream> CreateBadType4FunctionStream() {
@@ -194,10 +200,11 @@
}
TEST(CPDF_DocRenderDataTest, TransferFunctionArray) {
+ CPDF_IndirectObjectHolder holder;
auto func_array = pdfium::MakeRetain<CPDF_Array>();
- func_array->Append(CreateType0FunctionStream());
+ func_array->Append(CreateType0FunctionStreamReference(holder));
func_array->Append(CreateType2FunctionDict());
- func_array->Append(CreateType4FunctionStream());
+ func_array->Append(CreateType4FunctionStreamReference(holder));
TestDocRenderData render_data;
auto func = render_data.CreateTransferFuncForTesting(func_array);
@@ -247,10 +254,15 @@
}
{
+ CPDF_IndirectObjectHolder holder;
auto func_array = pdfium::MakeRetain<CPDF_Array>();
- func_array->Append(CreateType0FunctionStream());
+ func_array->Append(CreateType0FunctionStreamReference(holder));
func_array->Append(CreateType2FunctionDict());
- func_array->Append(CreateBadType4FunctionStream());
+ auto func_stream = CreateBadType4FunctionStream();
+ const int func_stream_object_number =
+ holder.AddIndirectObject(std::move(func_stream));
+ func_array->Append(
+ pdfium::MakeRetain<CPDF_Reference>(&holder, func_stream_object_number));
TestDocRenderData render_data;
auto func = render_data.CreateTransferFuncForTesting(func_array);
