Prune unreachable new PDF objects during save

Make `CPDF_Creator::WriteNewObjs()` reachability-aware so newly allocated indirect objects that are no longer referenced do not get written into saved PDFs. The save reachability set now includes normal document graph references plus trailer-owned roots such as `/Info` and non-inline `/Encrypt`, avoiding dangling trailer references while still pruning true orphans.

Also refactor `EPDF_SetEncryption()` to use an inline encryption dictionary so it follows CPDF_Creator’s existing trailer-owned encrypt path, and update Bug1206 to assert render-only saves remain stable while reopened rendering still matches.

Author: bobsingor

core/fpdfapi/edit/cpdf_creator.cpp

@@ -13,6 +13,7 @@
 #include <array>
 #include <set>
 #include <utility>
+#include <vector>
 
 #include "core/fpdfapi/parser/cpdf_array.h"
 #include "core/fpdfapi/parser/cpdf_crypto_handler.h"
@@ -136,6 +137,28 @@ ByteString FormatXrefOffset10(FX_FILESIZE offset) {
   return ByteString::Format("%010" PRId64, static_cast<int64_t>(offset));
 }
 
+std::set<uint32_t> CollectSaveReachableObjects(
+    CPDF_Document* document,
+    const CPDF_Dictionary* encrypt_dict) {
+  std::set<uint32_t> objects = GetObjectsWithReferences(document);
+
+  // `GetObjectsWithReferences()` covers the normal document graph rooted at
+  // /Root. The save trailer may also reference dictionaries outside that graph.
+  // Keep those roots in sync with the trailer entries emitted in
+  // WriteDoc_Stage4().
+  RetainPtr<CPDF_Dictionary> info = document->GetInfo();
+  if (info && info->GetObjNum() != 0) {
+    objects.insert(info->GetObjNum());
+  }
+
+  if (encrypt_dict && !encrypt_dict->IsInline() &&
+      encrypt_dict->GetObjNum() != 0) {
+    objects.insert(encrypt_dict->GetObjNum());
+  }
+
+  return objects;
+}
+
 }  // namespace
 
 CPDF_Creator::CPDF_Creator(CPDF_Document* doc,
@@ -223,8 +246,15 @@ bool CPDF_Creator::WriteOldObjs() {
 }
 
 bool CPDF_Creator::WriteNewObjs() {
+  const std::set<uint32_t> objects_with_refs =
+      CollectSaveReachableObjects(document_, encrypt_dict_.Get());
+  std::vector<uint32_t> written_new_obj_nums;
   for (size_t i = cur_obj_num_; i < new_obj_num_array_.size(); ++i) {
     uint32_t objnum = new_obj_num_array_[i];
+    if (!pdfium::Contains(objects_with_refs, objnum)) {
+      continue;
+    }
+
     RetainPtr<const CPDF_Object> pObj = document_->GetIndirectObject(objnum);
     if (!pObj) {
       continue;
@@ -234,7 +264,9 @@ bool CPDF_Creator::WriteNewObjs() {
     if (!WriteIndirectObj(pObj->GetObjNum(), pObj.Get())) {
       return false;
     }
+    written_new_obj_nums.push_back(objnum);
   }
+  new_obj_num_array_ = std::move(written_new_obj_nums);
   return true;
 }
 

fpdfsdk/fpdf_annot_embeddertest.cpp

@@ -1997,26 +1997,31 @@ TEST_F(FPDFAnnotEmbedderTest, GetFormAnnotAndCheckFlagsComboBox) {
 }
 
 TEST_F(FPDFAnnotEmbedderTest, Bug1206) {
-  static constexpr size_t kExpectedMinimumOriginalSize = 1601;
-
   ASSERT_TRUE(OpenDocument("bug_1206.pdf"));
 
   ScopedPage page = LoadScopedPage(0);
   ASSERT_TRUE(page);
 
   ASSERT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
   const size_t original_size = GetString().size();
-  EXPECT_LE(kExpectedMinimumOriginalSize, original_size);  // Sanity check.
+  ASSERT_GT(original_size, 0u);
   ClearString();
 
   for (size_t i = 0; i < 10; ++i) {
     ScopedFPDFBitmap bitmap = RenderLoadedPageWithFlags(page.get(), FPDF_ANNOT);
     CompareBitmapWithExpectationSuffix(bitmap.get(), "bug_1206");
 
     ASSERT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
-    // TODO(https://crbug.com/42270200): This is wrong. The size should be
-    // equal, not bigger.
-    EXPECT_GT(GetString().size(), original_size);
+    EXPECT_EQ(original_size, GetString().size());
+
+    ScopedSavedDoc saved_doc = OpenScopedSavedDocument();
+    ASSERT_TRUE(saved_doc);
+    ScopedSavedPage saved_page = LoadScopedSavedPage(0);
+    ASSERT_TRUE(saved_page);
+    ScopedFPDFBitmap saved_bitmap =
+        RenderSavedPageWithFlags(saved_page.get(), FPDF_ANNOT);
+    CompareBitmapWithExpectationSuffix(saved_bitmap.get(), "bug_1206");
+
     ClearString();
   }
 }
@@ -2585,7 +2590,8 @@ TEST_F(FPDFAnnotEmbedderTest, GetFontSizeNegative) {
   }
 }
 
-TEST_F(FPDFAnnotEmbedderTest, DirectAnnotationObjectNumberStaysZeroAfterRender) {
+TEST_F(FPDFAnnotEmbedderTest,
+       DirectAnnotationObjectNumberStaysZeroAfterRender) {
   ASSERT_TRUE(OpenDocument("freetext_annotation_without_da.pdf"));
   ScopedPage page = LoadScopedPage(0);
   ASSERT_TRUE(page);
@@ -2600,8 +2606,8 @@ TEST_F(FPDFAnnotEmbedderTest, DirectAnnotationObjectNumberStaysZeroAfterRender)
   ASSERT_TRUE(bitmap);
   EXPECT_EQ(0u, EPDFAnnot_GetObjectNumber(annot.get()));
 
-  ASSERT_TRUE(EPDFAnnot_SetColor(annot.get(), FPDFANNOT_COLORTYPE_Color, 10,
-                                 20, 30));
+  ASSERT_TRUE(
+      EPDFAnnot_SetColor(annot.get(), FPDFANNOT_COLORTYPE_Color, 10, 20, 30));
   EXPECT_EQ(0u, EPDFAnnot_GetObjectNumber(annot.get()));
 }
 

fpdfsdk/fpdf_save_embeddertest.cpp

@@ -6,8 +6,13 @@
 #include <string>
 #include <vector>
 
+#include "core/fpdfapi/parser/cpdf_cross_ref_table.h"
+#include "core/fpdfapi/parser/cpdf_document.h"
+#include "core/fpdfapi/parser/cpdf_parser.h"
 #include "core/fxcrt/fx_string.h"
+#include "fpdfsdk/cpdfsdk_helpers.h"
 #include "public/cpp/fpdf_scopers.h"
+#include "public/fpdf_annot.h"
 #include "public/fpdf_edit.h"
 #include "public/fpdf_ppo.h"
 #include "public/fpdf_save.h"
@@ -24,6 +29,23 @@ using testing::HasSubstr;
 using testing::Not;
 using testing::StartsWith;
 
+namespace {
+
+bool HasSavedXRefEntryForObject(FPDF_DOCUMENT document, uint32_t objnum) {
+  CPDF_Document* cpdf_doc = CPDFDocumentFromFPDFDocument(document);
+  if (!cpdf_doc || !cpdf_doc->GetParser() ||
+      !cpdf_doc->GetParser()->GetCrossRefTable()) {
+    return false;
+  }
+
+  const CPDF_CrossRefTable::ObjectInfo* info =
+      cpdf_doc->GetParser()->GetCrossRefTable()->GetObjectInfo(objnum);
+  return info && (info->type == CPDF_CrossRefTable::ObjectType::kNormal ||
+                  info->type == CPDF_CrossRefTable::ObjectType::kCompressed);
+}
+
+}  // namespace
+
 class FPDFSaveEmbedderTest : public EmbedderTest {};
 
 TEST_F(FPDFSaveEmbedderTest, SaveSimpleDoc) {
@@ -85,6 +107,98 @@ TEST_F(FPDFSaveEmbedderTest, SaveSimpleDocIncremental) {
   EXPECT_GT(GetString().size(), 985u);
 }
 
+TEST_F(FPDFSaveEmbedderTest, SaveAsCopyPrunesUnlinkedNewAnnotation) {
+  ASSERT_TRUE(OpenDocument("rectangles.pdf"));
+  ScopedPage page = LoadScopedPage(0);
+  ASSERT_TRUE(page);
+  ASSERT_EQ(0, FPDFPage_GetAnnotCount(page.get()));
+
+  ScopedFPDFAnnotation annot(EPDFPage_CreateAnnot(page.get(), FPDF_ANNOT_TEXT));
+  ASSERT_TRUE(annot);
+  const uint32_t annot_objnum = EPDFAnnot_GetObjectNumber(annot.get());
+  ASSERT_GT(annot_objnum, 0u);
+  ASSERT_EQ(1, FPDFPage_GetAnnotCount(page.get()));
+
+  annot.reset();
+  ASSERT_TRUE(FPDFPage_RemoveAnnot(page.get(), 0));
+  ASSERT_EQ(0, FPDFPage_GetAnnotCount(page.get()));
+
+  ASSERT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
+  ScopedSavedDoc saved_doc = OpenScopedSavedDocument();
+  ASSERT_TRUE(saved_doc);
+  EXPECT_FALSE(HasSavedXRefEntryForObject(saved_doc.get(), annot_objnum));
+
+  ScopedSavedPage saved_page = LoadScopedSavedPage(0);
+  ASSERT_TRUE(saved_page);
+  EXPECT_EQ(0, FPDFPage_GetAnnotCount(saved_page.get()));
+}
+
+TEST_F(FPDFSaveEmbedderTest, IncrementalSavePrunesUnlinkedNewAnnotation) {
+  ASSERT_TRUE(OpenDocument("rectangles.pdf"));
+  ScopedPage page = LoadScopedPage(0);
+  ASSERT_TRUE(page);
+  ASSERT_EQ(0, FPDFPage_GetAnnotCount(page.get()));
+
+  ScopedFPDFAnnotation annot(EPDFPage_CreateAnnot(page.get(), FPDF_ANNOT_TEXT));
+  ASSERT_TRUE(annot);
+  const uint32_t annot_objnum = EPDFAnnot_GetObjectNumber(annot.get());
+  ASSERT_GT(annot_objnum, 0u);
+
+  annot.reset();
+  ASSERT_TRUE(FPDFPage_RemoveAnnot(page.get(), 0));
+  ASSERT_EQ(0, FPDFPage_GetAnnotCount(page.get()));
+
+  ASSERT_TRUE(FPDF_SaveAsCopy(document(), this, FPDF_INCREMENTAL));
+  ScopedSavedDoc saved_doc = OpenScopedSavedDocument();
+  ASSERT_TRUE(saved_doc);
+  EXPECT_FALSE(HasSavedXRefEntryForObject(saved_doc.get(), annot_objnum));
+
+  ScopedSavedPage saved_page = LoadScopedSavedPage(0);
+  ASSERT_TRUE(saved_page);
+  EXPECT_EQ(0, FPDFPage_GetAnnotCount(saved_page.get()));
+}
+
+TEST_F(FPDFSaveEmbedderTest, SaveAsCopyKeepsLinkedNewAnnotation) {
+  ASSERT_TRUE(OpenDocument("rectangles.pdf"));
+  ScopedPage page = LoadScopedPage(0);
+  ASSERT_TRUE(page);
+  ASSERT_EQ(0, FPDFPage_GetAnnotCount(page.get()));
+
+  ScopedFPDFAnnotation annot(EPDFPage_CreateAnnot(page.get(), FPDF_ANNOT_TEXT));
+  ASSERT_TRUE(annot);
+  const uint32_t annot_objnum = EPDFAnnot_GetObjectNumber(annot.get());
+  ASSERT_GT(annot_objnum, 0u);
+  ASSERT_EQ(1, FPDFPage_GetAnnotCount(page.get()));
+
+  ASSERT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
+  ScopedSavedDoc saved_doc = OpenScopedSavedDocument();
+  ASSERT_TRUE(saved_doc);
+  EXPECT_TRUE(HasSavedXRefEntryForObject(saved_doc.get(), annot_objnum));
+
+  ScopedSavedPage saved_page = LoadScopedSavedPage(0);
+  ASSERT_TRUE(saved_page);
+  ASSERT_EQ(1, FPDFPage_GetAnnotCount(saved_page.get()));
+  ScopedFPDFAnnotation saved_annot(FPDFPage_GetAnnot(saved_page.get(), 0));
+  ASSERT_TRUE(saved_annot);
+  EXPECT_EQ(FPDF_ANNOT_TEXT, FPDFAnnot_GetSubtype(saved_annot.get()));
+}
+
+TEST_F(FPDFSaveEmbedderTest, SetEncryptionRoundTripsWithInlineEncryptDict) {
+  ASSERT_TRUE(OpenDocument("hello_world.pdf"));
+  ASSERT_TRUE(EPDF_SetEncryption(document(), "user", "owner",
+                                 EPDF_PERM_PRINT | EPDF_PERM_COPY));
+
+  ASSERT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
+  ScopedSavedDoc saved_doc = OpenScopedSavedDocumentWithPassword("user");
+  ASSERT_TRUE(saved_doc);
+  EXPECT_TRUE(EPDF_IsEncrypted(saved_doc.get()));
+
+  ScopedSavedPage saved_page = LoadScopedSavedPage(0);
+  ASSERT_TRUE(saved_page);
+  ScopedFPDFBitmap bitmap = RenderSavedPage(saved_page.get());
+  ASSERT_TRUE(bitmap);
+}
+
 TEST_F(FPDFSaveEmbedderTest, SaveSimpleDocNoIncremental) {
   ASSERT_TRUE(OpenDocument("hello_world.pdf"));
   EXPECT_TRUE(FPDF_SaveWithVersion(document(), this, FPDF_NO_INCREMENTAL, 14));

fpdfsdk/fpdf_view.cpp

@@ -524,8 +524,10 @@ EPDF_SetEncryption(FPDF_DOCUMENT document,
   int32_t permissions =
       static_cast<int32_t>(BuildPermissionsForRevision(allowed_flags_32));
 
-  // Create encrypt dictionary as indirect object
-  auto pEncryptDict = pDoc->NewIndirect<CPDF_Dictionary>();
+  // Create the encrypt dictionary inline. CPDF_Creator::SetEncryption() owns
+  // the trailer-only reference and writes inline encrypt dictionaries as
+  // indirect objects during save.
+  auto pEncryptDict = pDoc->New<CPDF_Dictionary>();
   pEncryptDict->SetNewFor<CPDF_Name>("Filter", "Standard");
   pEncryptDict->SetNewFor<CPDF_Number>("V", 5);
   pEncryptDict->SetNewFor<CPDF_Number>("R", 6);
@@ -552,8 +554,6 @@ EPDF_SetEncryption(FPDF_DOCUMENT document,
   // Returns false if LoadDict fails or R != 6
   if (!pSecurityHandler->OnCreateWithPasswords(pEncryptDict.Get(), user_pwd,
                                                owner_pwd)) {
-    // Cleanup the indirect object we created
-    pDoc->DeleteIndirectObject(pEncryptDict->GetObjNum());
     return false;
   }