commit 75e77041a822f7ac7e73fb94b24e8c7771ff7f36
author Lei Zhang <thestig@chromium.org> Fri Jan 19 06:08:35 2024 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Jan 19 06:08:35 2024 +0000
tree 6d93d03f1c7663581516407fde075ac9e4778c5d
parent 40dd828603597b67b50d6be336365723a7f1ff78

Do not let CPDF_Dictionary::SetFor() take a CPDF_Stream

Use `= delete` to disallow SetFor() from taking a CPDF_Stream directly.
Then update the unit test that does pass in a CPDF_Stream to properly
use CPDF_Reference instead.

Along the way, rename a few variables to follow the style guide.

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

core/fpdfapi/font/cpdf_simplefont_unittest.cpp

diff --git a/core/fpdfapi/font/cpdf_simplefont_unittest.cpp b/core/fpdfapi/font/cpdf_simplefont_unittest.cpp
index f54a97d..93aad8b 100644
--- a/core/fpdfapi/font/cpdf_simplefont_unittest.cpp
+++ b/core/fpdfapi/font/cpdf_simplefont_unittest.cpp
@@ -11,6 +11,7 @@
 #include "core/fpdfapi/page/test_with_page_module.h"
 #include "core/fpdfapi/parser/cpdf_dictionary.h"
 #include "core/fpdfapi/parser/cpdf_name.h"
+#include "core/fpdfapi/parser/cpdf_reference.h"
 #include "core/fpdfapi/parser/cpdf_stream.h"
 #include "core/fpdfapi/parser/cpdf_test_document.h"
 #include "core/fxcrt/retain_ptr.h"
@@ -38,13 +39,17 @@
   CPDF_TestDocument doc;
 
   // The code being exercised requires valid font data.
-  auto font_file_stream = pdfium::MakeRetain<CPDF_Stream>(
+  auto font_file_stream = doc.NewIndirect<CPDF_Stream>(
       DataVector<uint8_t>(std::begin(kFoxitFixedFontData),
                           std::end(kFoxitFixedFontData)),
       pdfium::MakeRetain<CPDF_Dictionary>());
+  ASSERT_TRUE(font_file_stream);
+  const uint32_t stream_object_number = font_file_stream->GetObjNum();
+  ASSERT_GT(stream_object_number, 0u);
 
   auto font_descriptor_dict = pdfium::MakeRetain<CPDF_Dictionary>();
-  font_descriptor_dict->SetFor("FontFile", std::move(font_file_stream));
+  font_descriptor_dict->SetFor("FontFile", pdfium::MakeRetain<CPDF_Reference>(
+                                               &doc, stream_object_number));
 
   auto font_dict = pdfium::MakeRetain<CPDF_Dictionary>();
   font_dict->SetNewFor<CPDF_Name>("BaseFont", "CHEESE+Swiss");

core/fpdfapi/parser/cpdf_dictionary.cpp

diff --git a/core/fpdfapi/parser/cpdf_dictionary.cpp b/core/fpdfapi/parser/cpdf_dictionary.cpp
index bf6f593..81d20f6 100644
--- a/core/fpdfapi/parser/cpdf_dictionary.cpp
+++ b/core/fpdfapi/parser/cpdf_dictionary.cpp
@@ -265,8 +265,8 @@
 }
 
 void CPDF_Dictionary::SetFor(const ByteString& key,
-                             RetainPtr<CPDF_Object> pObj) {
-  (void)SetForInternal(key, std::move(pObj));
+                             RetainPtr<CPDF_Object> object) {
+  (void)SetForInternal(key, std::move(object));
 }
 
 CPDF_Object* CPDF_Dictionary::SetForInternal(const ByteString& key,

core/fpdfapi/parser/cpdf_dictionary.h

diff --git a/core/fpdfapi/parser/cpdf_dictionary.h b/core/fpdfapi/parser/cpdf_dictionary.h
index 34be276..671973f 100644
--- a/core/fpdfapi/parser/cpdf_dictionary.h
+++ b/core/fpdfapi/parser/cpdf_dictionary.h
@@ -99,10 +99,13 @@
         key, pdfium::MakeRetain<T>(m_pPool, std::forward<Args>(args)...))));
   }
 
-  // If |pObj| is null, then |key| is erased from the map. Otherwise, takes
-  // ownership of |pObj| and stores in in the map. Invalidates iterators for
-  // the element with the key |key|.
-  void SetFor(const ByteString& key, RetainPtr<CPDF_Object> pObj);
+  // If `object` is null, then `key` is erased from the map. Otherwise, takes
+  // ownership of `object` and stores in in the map. Invalidates iterators for
+  // the element with the key `key`.
+  // TODO(crbug.com/pdfium/2119): Ensure `object` is not a `CPDF_Stream`.
+  void SetFor(const ByteString& key, RetainPtr<CPDF_Object> object);
+  // A stream must be indirect and added as a `CPDF_Reference` instead.
+  void SetFor(const ByteString& key, RetainPtr<CPDF_Stream> stream) = delete;
 
   // Convenience functions to convert native objects to array form.
   void SetRectFor(const ByteString& key, const CFX_FloatRect& rect);