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");