Do not let CPDF_Dictionary::SetNewFor() take a CPDF_Stream
Use std::enable_if() to disallow generating the templated version of
SetNewFor() that takes a CPDF_Stream. Then either update affected tests
to pass in CPDF_Stream properly using CPDF_Reference, or delete the test
code that no longer makes sense.
Along the way, rename a few variables to follow the style guide.
Bug: pdfium:2119
Change-Id: If1069133a11ed72f765a1e63a85c35326b427ea0
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/115630
Auto-Submit: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/parser/cpdf_dictionary.h b/core/fpdfapi/parser/cpdf_dictionary.h
index 671973f..dcb89f5 100644
--- a/core/fpdfapi/parser/cpdf_dictionary.h
+++ b/core/fpdfapi/parser/cpdf_dictionary.h
@@ -10,6 +10,7 @@
#include <functional>
#include <map>
#include <set>
+#include <type_traits>
#include <utility>
#include <vector>
@@ -86,8 +87,11 @@
// Prefer using these templates over calls to SetFor(), since by creating
// a new object with no previous references, they ensure cycles can not be
// introduced.
+ // A stream must be indirect and added as a `CPDF_Reference` instead.
template <typename T, typename... Args>
- typename std::enable_if<!CanInternStrings<T>::value, RetainPtr<T>>::type
+ typename std::enable_if<!CanInternStrings<T>::value &&
+ !std::is_same<T, CPDF_Stream>::value,
+ RetainPtr<T>>::type
SetNewFor(const ByteString& key, Args&&... args) {
return pdfium::WrapRetain(static_cast<T*>(SetForInternal(
key, pdfium::MakeRetain<T>(std::forward<Args>(args)...))));
diff --git a/core/fpdfapi/parser/cpdf_dictionary_unittest.cpp b/core/fpdfapi/parser/cpdf_dictionary_unittest.cpp
index 84acc3a..508623c 100644
--- a/core/fpdfapi/parser/cpdf_dictionary_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_dictionary_unittest.cpp
@@ -8,14 +8,12 @@
#include "core/fpdfapi/parser/cpdf_array.h"
#include "core/fpdfapi/parser/cpdf_number.h"
-#include "core/fpdfapi/parser/cpdf_stream.h"
#include "testing/gtest/include/gtest/gtest.h"
TEST(DictionaryTest, Iterators) {
auto dict = pdfium::MakeRetain<CPDF_Dictionary>();
dict->SetNewFor<CPDF_Dictionary>("the-dictionary");
dict->SetNewFor<CPDF_Array>("the-array");
- dict->SetNewFor<CPDF_Stream>("the-stream");
dict->SetNewFor<CPDF_Number>("the-number", 42);
CPDF_DictionaryLocker locked_dict(dict);
@@ -35,10 +33,5 @@
EXPECT_TRUE(it->second->IsNumber());
++it;
- EXPECT_NE(it, locked_dict.end());
- EXPECT_EQ(it->first, ByteString("the-stream"));
- EXPECT_TRUE(it->second->IsStream());
-
- ++it;
EXPECT_EQ(it, locked_dict.end());
}
diff --git a/core/fpdfapi/parser/cpdf_object_unittest.cpp b/core/fpdfapi/parser/cpdf_object_unittest.cpp
index a57b85e..cf1f1b1 100644
--- a/core/fpdfapi/parser/cpdf_object_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_object_unittest.cpp
@@ -946,21 +946,6 @@
dict_obj->RemoveFor("arr"); // Break deliberate cycle for cleanup.
}
{
- // Create a dictionary/stream pair with a reference loop.
- auto dict_obj = pdfium::MakeRetain<CPDF_Dictionary>();
- auto stream_obj = dict_obj->SetNewFor<CPDF_Stream>("stream", dict_obj);
- // Clone this object to see whether stack overflow will be triggered.
- RetainPtr<CPDF_Stream> cloned_stream = ToStream(stream_obj->Clone());
- // Cloned object should be the same as the original.
- ASSERT_TRUE(cloned_stream);
- RetainPtr<const CPDF_Object> cloned_dict = cloned_stream->GetDict();
- ASSERT_TRUE(cloned_dict);
- ASSERT_TRUE(cloned_dict->IsDictionary());
- // Recursively referenced object is not cloned.
- EXPECT_FALSE(cloned_dict->AsDictionary()->GetObjectFor("stream"));
- dict_obj->RemoveFor("stream"); // Break deliberate cycle for cleanup.
- }
- {
CPDF_IndirectObjectHolder objects_holder;
// Create an object with a reference loop.
auto dict_obj = objects_holder.NewIndirect<CPDF_Dictionary>();
diff --git a/core/fpdfdoc/cpdf_filespec_unittest.cpp b/core/fpdfdoc/cpdf_filespec_unittest.cpp
index 7c517a9..e898076 100644
--- a/core/fpdfdoc/cpdf_filespec_unittest.cpp
+++ b/core/fpdfdoc/cpdf_filespec_unittest.cpp
@@ -10,8 +10,10 @@
#include "build/build_config.h"
#include "core/fpdfapi/parser/cpdf_dictionary.h"
+#include "core/fpdfapi/parser/cpdf_indirect_object_holder.h"
#include "core/fpdfapi/parser/cpdf_name.h"
#include "core/fpdfapi/parser/cpdf_number.h"
+#include "core/fpdfapi/parser/cpdf_reference.h"
#include "core/fpdfapi/parser/cpdf_stream.h"
#include "core/fpdfapi/parser/cpdf_string.h"
#include "core/fxcrt/data_vector.h"
@@ -156,6 +158,7 @@
EXPECT_FALSE(file_spec.GetFileStream());
}
{
+ CPDF_IndirectObjectHolder object_holder;
// Dictionary object with a non-empty embedded files dictionary.
auto dict_obj = pdfium::MakeRetain<CPDF_Dictionary>();
dict_obj->SetNewFor<CPDF_Dictionary>("EF");
@@ -173,11 +176,15 @@
dict_obj->SetNewFor<CPDF_String>(keys[i], file_name);
// Set the file stream.
- auto pDict = pdfium::MakeRetain<CPDF_Dictionary>();
size_t buf_len = strlen(streams[i]) + 1;
- file_dict->SetNewFor<CPDF_Stream>(
- keys[i], DataVector<uint8_t>(streams[i], streams[i] + buf_len),
- std::move(pDict));
+ auto stream_object = object_holder.NewIndirect<CPDF_Stream>(
+ DataVector<uint8_t>(streams[i], streams[i] + buf_len),
+ pdfium::MakeRetain<CPDF_Dictionary>());
+ ASSERT_TRUE(stream_object);
+ const uint32_t stream_object_number = stream_object->GetObjNum();
+ ASSERT_GT(stream_object_number, 0u);
+ file_dict->SetNewFor<CPDF_Reference>(keys[i], &object_holder,
+ stream_object_number);
// Check that the file content stream is as expected.
EXPECT_STREQ(
@@ -200,6 +207,8 @@
EXPECT_FALSE(file_spec.GetParamsDict());
}
{
+ CPDF_IndirectObjectHolder object_holder;
+
// Dictionary object.
auto dict_obj = pdfium::MakeRetain<CPDF_Dictionary>();
dict_obj->SetNewFor<CPDF_Dictionary>("EF");
@@ -209,11 +218,15 @@
// Add a file stream to the embedded files dictionary.
RetainPtr<CPDF_Dictionary> file_dict = dict_obj->GetMutableDictFor("EF");
- auto pDict = pdfium::MakeRetain<CPDF_Dictionary>();
static constexpr char kHello[] = "hello";
- file_dict->SetNewFor<CPDF_Stream>(
- "UF", DataVector<uint8_t>(std::begin(kHello), std::end(kHello)),
- std::move(pDict));
+ auto stream_object = object_holder.NewIndirect<CPDF_Stream>(
+ DataVector<uint8_t>(std::begin(kHello), std::end(kHello)),
+ pdfium::MakeRetain<CPDF_Dictionary>());
+ ASSERT_TRUE(stream_object);
+ const uint32_t stream_object_number = stream_object->GetObjNum();
+ ASSERT_GT(stream_object_number, 0u);
+ file_dict->SetNewFor<CPDF_Reference>("UF", &object_holder,
+ stream_object_number);
// Add a params dictionary to the file stream.
RetainPtr<CPDF_Stream> stream = file_dict->GetMutableStreamFor("UF");