Do not let CPDF_Array::SetNewAt() and friends take a CPDF_Stream
Use std::enable_if() to disallow generating the templated versions of
SetNewAt(), AppendNew(), and InsertNewAt() that takes a CPDF_Stream.
Then update affected tests to pass in CPDF_Stream properly using
CPDF_Reference.
Bug: pdfium:2119
Change-Id: I0bfb36f1d693f8c2a5e362fa80560e443a97db8b
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/116370
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Thomas Sepez <tsepez@google.com>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/parser/cpdf_array.h b/core/fpdfapi/parser/cpdf_array.h
index a103133..b9641bc 100644
--- a/core/fpdfapi/parser/cpdf_array.h
+++ b/core/fpdfapi/parser/cpdf_array.h
@@ -83,6 +83,9 @@
template <typename T, typename... Args>
typename std::enable_if<!CanInternStrings<T>::value, RetainPtr<T>>::type
AppendNew(Args&&... args) {
+ static_assert(!std::is_same<T, CPDF_Stream>::value,
+ "Cannot append a CPDF_Stream directly. Add it indirectly as "
+ "a `CPDF_Reference` instead.");
return pdfium::WrapRetain(static_cast<T*>(
AppendInternal(pdfium::MakeRetain<T>(std::forward<Args>(args)...))));
}
@@ -95,6 +98,9 @@
template <typename T, typename... Args>
typename std::enable_if<!CanInternStrings<T>::value, RetainPtr<T>>::type
SetNewAt(size_t index, Args&&... args) {
+ static_assert(!std::is_same<T, CPDF_Stream>::value,
+ "Cannot set a CPDF_Stream directly. Add it indirectly as a "
+ "`CPDF_Reference` instead.");
return pdfium::WrapRetain(static_cast<T*>(SetAtInternal(
index, pdfium::MakeRetain<T>(std::forward<Args>(args)...))));
}
@@ -107,6 +113,9 @@
template <typename T, typename... Args>
typename std::enable_if<!CanInternStrings<T>::value, RetainPtr<T>>::type
InsertNewAt(size_t index, Args&&... args) {
+ static_assert(!std::is_same<T, CPDF_Stream>::value,
+ "Cannot insert a CPDF_Stream directly. Add it indirectly as "
+ "a `CPDF_Reference` instead.");
return pdfium::WrapRetain(static_cast<T*>(InsertAtInternal(
index, pdfium::MakeRetain<T>(std::forward<Args>(args)...))));
}
diff --git a/core/fpdfapi/parser/cpdf_object_unittest.cpp b/core/fpdfapi/parser/cpdf_object_unittest.cpp
index ed366a2..2d1ea04 100644
--- a/core/fpdfapi/parser/cpdf_object_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_object_unittest.cpp
@@ -636,6 +636,8 @@
}
{
// Stream array.
+ CPDF_IndirectObjectHolder object_holder;
+
RetainPtr<CPDF_Dictionary> vals[3];
RetainPtr<CPDF_Stream> stream_vals[3];
auto arr = pdfium::MakeRetain<CPDF_Array>();
@@ -649,9 +651,11 @@
vals[i]->SetNewFor<CPDF_Number>(key.c_str(), value);
}
static constexpr uint8_t kContents[] = "content: this is a stream";
- stream_vals[i] = arr->AppendNew<CPDF_Stream>(
+ stream_vals[i] = object_holder.NewIndirect<CPDF_Stream>(
DataVector<uint8_t>(std::begin(kContents), std::end(kContents)),
vals[i]);
+ arr->AppendNew<CPDF_Reference>(&object_holder,
+ stream_vals[i]->GetObjNum());
}
for (size_t i = 0; i < 3; ++i) {
TestArrayAccessors(arr.Get(), i, // Array and index.
@@ -666,6 +670,8 @@
}
{
// Mixed array.
+
+ CPDF_IndirectObjectHolder object_holder;
auto arr = pdfium::MakeRetain<CPDF_Array>();
arr->InsertNewAt<CPDF_Boolean>(0, true);
arr->InsertNewAt<CPDF_Boolean>(1, false);
@@ -693,9 +699,10 @@
static constexpr uint8_t kData[] = "A stream for test";
// The data buffer will be owned by stream object, so it needs to be
// dynamically allocated.
- CPDF_Stream* stream_val = arr->InsertNewAt<CPDF_Stream>(
- 13, DataVector<uint8_t>(std::begin(kData), std::end(kData)),
- stream_dict);
+ auto stream_val = object_holder.NewIndirect<CPDF_Stream>(
+ DataVector<uint8_t>(std::begin(kData), std::end(kData)), stream_dict);
+ arr->InsertNewAt<CPDF_Reference>(13, &object_holder,
+ stream_val->GetObjNum());
const char* const expected_str[] = {
"true", "false", "0", "-1234", "2345", "0.05", "",
"It is a test!", "NAME", "test", "", "", "", ""};
