commit 54f25ce7a46c49c420b95ab209aecad995c0c98a
author Lei Zhang <thestig@chromium.org> Tue Jan 23 01:24:55 2024 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue Jan 23 01:24:55 2024 +0000
tree a95d08b87db358ed3d310fc1c8c7aa1f0ade294a
parent 720ffea6a75c11dc67ec51fa986c6b74f8ede747

Ensure CPDF_Stream always has a dictionary

Now that CPDF_Stream is never uninitialized, tweak its semantics to
guarantee its `dict_` member is never null and CHECK() that is the case.
Then remove conditionals that check if `dict_` is null, and update known
CPDF_Stream users to avoid passing in null dictionaries.

Change-Id: I3446698401cbc29c7100a18084997e02e8030de4
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/115692
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Thomas Sepez <tsepez@google.com>

core/fpdfapi/parser/cpdf_object_stream_unittest.cpp

diff --git a/core/fpdfapi/parser/cpdf_object_stream_unittest.cpp b/core/fpdfapi/parser/cpdf_object_stream_unittest.cpp
index 43306f4..513ca9b 100644
--- a/core/fpdfapi/parser/cpdf_object_stream_unittest.cpp
+++ b/core/fpdfapi/parser/cpdf_object_stream_unittest.cpp
@@ -80,11 +80,11 @@
   EXPECT_FALSE(obj_stream->ParseObject(&holder, 12, 3));
 }
 
-TEST(ObjectStreamTest, StreamNoDict) {
+TEST(ObjectStreamTest, StreamEmptyDict) {
   ByteStringView contents_view(kNormalStreamContent);
   auto stream = pdfium::MakeRetain<CPDF_Stream>(
       DataVector<uint8_t>(contents_view.begin(), contents_view.end()),
-      /*pDict=*/nullptr);
+      pdfium::MakeRetain<CPDF_Dictionary>());
   EXPECT_FALSE(CPDF_ObjectStream::Create(std::move(stream)));
 }
 

core/fpdfapi/parser/cpdf_stream.cpp

diff --git a/core/fpdfapi/parser/cpdf_stream.cpp b/core/fpdfapi/parser/cpdf_stream.cpp
index acda825..cfa7dfc 100644
--- a/core/fpdfapi/parser/cpdf_stream.cpp
+++ b/core/fpdfapi/parser/cpdf_stream.cpp
@@ -23,6 +23,7 @@
 #include "core/fxcrt/data_vector.h"
 #include "core/fxcrt/fx_stream.h"
 #include "core/fxcrt/span_util.h"
+#include "third_party/base/check.h"
 #include "third_party/base/containers/contains.h"
 #include "third_party/base/numerics/safe_conversions.h"
 
@@ -52,6 +53,7 @@
 CPDF_Stream::CPDF_Stream(RetainPtr<IFX_SeekableReadStream> file,
                          RetainPtr<CPDF_Dictionary> dict)
     : data_(std::move(file)), dict_(std::move(dict)) {
+  CHECK(dict_);
   SetLengthInDict(pdfium::base::checked_cast<int>(
       absl::get<RetainPtr<IFX_SeekableReadStream>>(data_)->GetSize()));
 }
@@ -59,14 +61,16 @@
 CPDF_Stream::CPDF_Stream(DataVector<uint8_t> data,
                          RetainPtr<CPDF_Dictionary> dict)
     : data_(std::move(data)), dict_(std::move(dict)) {
+  CHECK(dict_);
   SetLengthInDict(pdfium::base::checked_cast<int>(
       absl::get<DataVector<uint8_t>>(data_).size()));
 }
 
 CPDF_Stream::~CPDF_Stream() {
   m_ObjNum = kInvalidObjNum;
-  if (dict_ && dict_->GetObjNum() == kInvalidObjNum)
+  if (dict_->GetObjNum() == kInvalidObjNum) {
     dict_.Leak();  // lowercase release, release ownership.
+  }
 }
 
 CPDF_Object::Type CPDF_Stream::GetType() const {
@@ -161,7 +165,7 @@
 }
 
 bool CPDF_Stream::HasFilter() const {
-  return dict_ && dict_->KeyExist("Filter");
+  return dict_->KeyExist("Filter");
 }
 
 WideString CPDF_Stream::GetUnicodeText() const {
@@ -209,7 +213,5 @@
 }
 
 void CPDF_Stream::SetLengthInDict(int length) {
-  if (!dict_)
-    dict_ = pdfium::MakeRetain<CPDF_Dictionary>();
   dict_->SetNewFor<CPDF_Number>("Length", length);
 }

core/fpdfapi/parser/cpdf_stream.h

diff --git a/core/fpdfapi/parser/cpdf_stream.h b/core/fpdfapi/parser/cpdf_stream.h
index 08b72ed..42f5683 100644
--- a/core/fpdfapi/parser/cpdf_stream.h
+++ b/core/fpdfapi/parser/cpdf_stream.h
@@ -68,7 +68,7 @@
   friend class CPDF_Dictionary;
 
   // Initializes with empty data and /Length set to 0 in `dict`.
-  // If `dict` is null, then a new dictionary will be created instead.
+  // `dict` must be non-null.
   explicit CPDF_Stream(RetainPtr<CPDF_Dictionary> dict);
 
   // Copies `span` and `stream`, respectively. Creates a new dictionary with the
@@ -77,12 +77,12 @@
   explicit CPDF_Stream(fxcrt::ostringstream* stream);
 
   // Reads data from `file`. `dict` will have its /Length set based on `file`.
-  // If `dict` is null, then a new dictionary will be created instead.
+  // `dict` must be non-null.
   CPDF_Stream(RetainPtr<IFX_SeekableReadStream> file,
               RetainPtr<CPDF_Dictionary> dict);
 
   // Takes `data`.
-  // If `dict` is null, then a new dictionary will be created instead.
+  // `dict` must be non-null.
   CPDF_Stream(DataVector<uint8_t> data, RetainPtr<CPDF_Dictionary> dict);
   ~CPDF_Stream() override;