Write a full save's cross-reference table as one subsection

core/fpdfapi/edit/cpdf_creator.cpp

@@ -610,15 +610,12 @@ CPDF_Creator::Stage CPDF_Creator::WriteDoc_Stage3() {
     if (!is_incremental_ || is_incremental_append_only_ ||
         !parser_->IsXRefStream()) {
       if (!is_incremental_ || parser_->GetLastXRefOffset() == 0) {
-        ByteString str;
-        str = pdfium::Contains(object_offsets_, 1)
-                  ? "xref\r\n"
-                  : "xref\r\n0 1\r\n0000000000 65535 f\r\n";
-        if (!archive_->WriteString(str.AsStringView())) {
+        // EmbedPDF(complete-xref): the subsection header is written with the
+        // entries (kWriteXrefsNotIncremental81).
+        if (!archive_->WriteString("xref\r\n")) {
           return Stage::kInvalid;
         }
 
-        cur_obj_num_ = 1;
         stage_ = Stage::kWriteXrefsNotIncremental81;
       } else {
         if (!archive_->WriteString("xref\r\n")) {
@@ -633,49 +630,52 @@ CPDF_Creator::Stage CPDF_Creator::WriteDoc_Stage3() {
     }
   }
   if (stage_ == Stage::kWriteXrefsNotIncremental81) {
-    ByteString str;
-    uint32_t i = cur_obj_num_;
-    uint32_t j;
-    while (i <= dwLastObjNum) {
-      while (i <= dwLastObjNum && !pdfium::Contains(object_offsets_, i)) {
-        i++;
-      }
-
-      if (i > dwLastObjNum) {
-        break;
+    // EmbedPDF(complete-xref): the file's only table is one subsection, from
+    // object 0 through the last object number (ISO 32000-1 7.5.4). A number
+    // no written object holds - reserved by an editing session and never
+    // used, deleted, or no longer reached - is a free entry. Entry 0 heads
+    // the list of free entries, each links to the next higher one, and the
+    // last links back to 0. A free entry's generation is 0, the generation a
+    // new object at that number starts with. Upstream wrote a subsection per
+    // run of written objects and gave the numbers between runs no entry;
+    // Acrobat then can't judge the changes a later revision makes.
+    std::vector<uint32_t> free_numbers;
+    for (uint32_t number = 1; number <= dwLastObjNum; ++number) {
+      if (!pdfium::Contains(object_offsets_, number)) {
+        free_numbers.push_back(number);
       }
-
-      j = i;
-      while (j <= dwLastObjNum && pdfium::Contains(object_offsets_, j)) {
-        j++;
-      }
-
-      if (i == 1) {
-        str = ByteString::Format("0 %d\r\n0000000000 65535 f\r\n", j);
+    }
+    FX_SAFE_UINT32 entry_count = dwLastObjNum;
+    entry_count += 1;
+    if (!entry_count.IsValid()) {
+      return Stage::kInvalid;
+    }
+    const uint32_t first_free = free_numbers.empty() ? 0 : free_numbers[0];
+    ByteString str = ByteString::Format("0 %u\r\n", entry_count.ValueOrDie()) +
+                     FormatXrefOffset10(first_free) + " 65535 f\r\n";
+    if (!archive_->WriteString(str.AsStringView())) {
+      return Stage::kInvalid;
+    }
+    size_t next_free = 0;
+    for (uint32_t objnum = 1; objnum <= dwLastObjNum; ++objnum) {
+      if (next_free < free_numbers.size() &&
+          free_numbers[next_free] == objnum) {
+        ++next_free;
+        const uint32_t link =
+            next_free < free_numbers.size() ? free_numbers[next_free] : 0;
+        str = FormatXrefOffset10(link) + " 00000 f\r\n";
       } else {
-        str = ByteString::Format("%d %d\r\n", i, j - i);
-      }
-
-      if (!archive_->WriteString(str.AsStringView())) {
-        return Stage::kInvalid;
-      }
-
-      while (i < j) {
         // EmbedPDF(save-generations, layer-save): each entry carries its
         // object's generation, and only an offset an entry can hold.
-        const uint32_t objnum = i++;
         const FX_FILESIZE offset = object_offsets_[objnum];
         if (!CheckEmittedOffset(offset)) {
           return Stage::kInvalid;
         }
         str = FormatXrefOffset10(offset) +
               ByteString::Format(" %05u n\r\n", GetObjectGeneration(objnum));
-        if (!archive_->WriteString(str.AsStringView())) {
-          return Stage::kInvalid;
-        }
       }
-      if (i > dwLastObjNum) {
-        break;
+      if (!archive_->WriteString(str.AsStringView())) {
+        return Stage::kInvalid;
       }
     }
     stage_ = Stage::kWriteTrailerAndFinish90;

docs/embedpdf/fork-ledger.md

@@ -191,6 +191,21 @@ and encryption objects leaked into saves.
 `cpdf_object_stream_cache_unittest.cpp`.
 **When upstream touches this.** The creator's traversal and the parser's object stream access.
 
+### complete-xref
+**Rule.** A full save's cross-reference table is one subsection, from object 0 through the last
+number it writes. A number no written object holds is a free entry with generation 0; entry 0
+heads the list of free entries, each links to the next higher one, and the last links back to 0.
+Object numbers are never changed to close the holes.
+**Why.** ISO 32000-1 7.5.4: a file that was never updated has one subsection, numbered from 0.
+Editing sessions reserve numbers they may not use, and upstream gave the numbers between runs of
+written objects no entry at all. Acrobat can't judge a later revision's changes against such a
+table, so a signature over a rewritten file read as indeterminate after any edit
+(`HasSparseXref`). Object numbers are the engine's identities.
+**Where.** `CPDF_Creator::WriteDoc_Stage3`, the `kWriteXrefsNotIncremental81` stage.
+**Guarded by.** `EPDFObjectNumberEmbedderTest.FullSaveListsUnusedNumbersAsFree`.
+**When upstream touches this.** The full save's cross-reference writer: keep the one subsection
+and the free list. Update sections (incremental saves) stay sparse.
+
 ### save-security
 **Rule.** Pending encryption settings belong to the document, not to a global. AES-256 encryption
 and password probing are ours.

fpdfsdk/epdf_object_number_embeddertest.cpp

@@ -9,6 +9,10 @@
 // caller may use is the caller's decision; the layer only refuses a number
 // that isn't free.
 
+#include <stdio.h>
+
+#include <algorithm>
+#include <iterator>
 #include <string>
 #include <vector>
 
@@ -27,6 +31,7 @@
 #include "public/epdf_form.h"
 #include "public/epdf_layer.h"
 #include "public/epdf_save.h"
+#include "public/epdf_signature.h"
 #include "public/epdf_view.h"
 #include "public/fpdf_annot.h"
 #include "public/fpdf_edit.h"
@@ -388,6 +393,83 @@ TEST_F(EPDFObjectNumberEmbedderTest, ArtifactKeepsTheRaise) {
   EXPECT_THAT(Annots(reopened.get(), 1), ElementsAre(20u, 12u, kRaised + 1u));
 }
 
+// A full save lists every number up to the last one it writes, in one
+// subsection from 0 (ISO 32000-1 7.5.4): a handed-out number no object took
+// is a free entry, linked from entry 0 in ascending order and back to 0. A
+// revision appended to the saved file then follows a table without holes,
+// which a signature's later changes can be judged against.
+TEST_F(EPDFObjectNumberEmbedderTest, FullSaveListsUnusedNumbersAsFree) {
+  ScopedFPDFDocument doc = OpenLayer();
+  ASSERT_TRUE(doc);
+  ASSERT_TRUE(EPDFLayer_BeginTransaction(doc.get()));
+  EXPECT_EQ(15u, CreateAnnot(doc.get(), 1, 15));
+  EXPECT_EQ(20u, CreateAnnot(doc.get(), 1, 20));
+  ASSERT_TRUE(EPDFLayer_CommitTransaction(doc.get()));
+  ClearString();
+  ASSERT_TRUE(FPDF_SaveAsCopy(doc.get(), this, FPDF_NO_INCREMENTAL));
+  const std::string saved = GetString();
+
+  // One table, 0 to 20: 20 is the last number written, so 21 to 26 aren't
+  // listed. Each entry is 20 bytes.
+  const size_t table = saved.find("\r\nxref\r\n");
+  ASSERT_NE(std::string::npos, table);
+  EXPECT_EQ(std::string::npos, saved.find("\r\nxref\r\n", table + 1));
+  const size_t trailer = saved.find("trailer\r\n", table);
+  ASSERT_NE(std::string::npos, trailer);
+  const std::string rows = saved.substr(table + 8, trailer - table - 8);
+  ASSERT_EQ(6u + 21u * 20u, rows.size());
+  EXPECT_EQ("0 21\r\n", rows.substr(0, 6));
+  EXPECT_NE(std::string::npos, saved.find("/Size 21/", trailer));
+  auto entry = [&rows](uint32_t number) {
+    return rows.substr(6 + 20 * number, 20);
+  };
+  const std::vector<uint32_t> free = {7,  8,  9,  10, 11, 12,
+                                      13, 14, 16, 17, 18, 19};
+  EXPECT_EQ("0000000007 65535 f\r\n", entry(0));
+  for (uint32_t number = 1; number <= 20; ++number) {
+    SCOPED_TRACE(number);
+    auto it = std::ranges::find(free, number);
+    if (it == free.end()) {
+      EXPECT_EQ(" 00000 n\r\n", entry(number).substr(10));
+      continue;
+    }
+    const uint32_t link = std::next(it) == free.end() ? 0 : *std::next(it);
+    char row[21];
+    snprintf(row, sizeof(row), "%010u 00000 f\r\n", link);
+    EXPECT_EQ(row, entry(number));
+  }
+
+  ScopedFPDFDocument reopened(
+      FPDF_LoadMemDocument64(saved.data(), saved.size(), nullptr));
+  ASSERT_TRUE(reopened);
+  EXPECT_FALSE(CPDFDocumentFromFPDFDocument(reopened.get())
+                   ->GetParser()
+                   ->xref_table_rebuilt());
+  EXPECT_THAT(Annots(reopened.get(), 1), ElementsAre(15u, 20u));
+  {
+    ScopedFPDFPage page(FPDF_LoadPage(reopened.get(), 0));
+    ASSERT_TRUE(page);
+    ScopedFPDFAnnotation annot(
+        FPDFPage_CreateAnnot(page.get(), FPDF_ANNOT_SQUARE));
+    ASSERT_TRUE(annot);
+  }
+  ClearString();
+  ASSERT_TRUE(FPDF_SaveAsCopy(reopened.get(), this, FPDF_INCREMENTAL));
+  const std::string updated = GetString();
+  ScopedFPDFDocument newer(
+      FPDF_LoadMemDocument64(updated.data(), updated.size(), nullptr));
+  ASSERT_TRUE(newer);
+  ScopedFPDFDocument older(EPDFDoc_OpenRevision(newer.get(), saved.size()));
+  ASSERT_TRUE(older);
+  EPDF_OBJECT_DIFF diff = EPDFDoc_CompareRevisions(older.get(), newer.get());
+  ASSERT_TRUE(diff);
+  FPDF_BOOL sparse = true;
+  EXPECT_TRUE(EPDFObjectDiff_GetRevisionHealth(diff, EPDF_DIFF_OLD, &sparse,
+                                               nullptr, nullptr));
+  EXPECT_FALSE(sparse);
+  EPDFObjectDiff_Close(diff);
+}
+
 // A blank page at a chosen number is in the page list and the page tree at
 // once, takes annotations that name it as their page, renders, and survives
 // a save and reopen.

fpdfsdk/fpdf_save_embeddertest.cpp

@@ -176,7 +176,10 @@ TEST_F(FPDFSaveEmbedderTest, SaveLinearizedDoc) {
   EXPECT_THAT(GetString(), HasSubstr("36 0 obj"));
   EXPECT_THAT(GetString(), Not(HasSubstr("37 0 obj")));
   EXPECT_THAT(GetString(), Not(HasSubstr("38 0 obj")));
-  EXPECT_EQ(7884u, GetString().size());
+  // EmbedPDF(complete-xref): one subsection; 26 and 28 (the linearization
+  // dictionary), which the save doesn't reach, are free entries in it.
+  EXPECT_THAT(GetString(), HasSubstr("xref\r\n0 37\r\n0000000026 65535 f\r\n"));
+  EXPECT_EQ(7912u, GetString().size());
 
   // Make sure new document renders the same as the old one.
   ScopedSavedDoc saved_document = OpenScopedSavedDocument();

testing/tools/check_saved_pdfs.py

@@ -92,6 +92,23 @@ def check_references(words, xref):
             previous = []
 
 
+def subsections(data, start):
+    """The (first number, count) headers of the classic table at `start`."""
+    words = tokens(data, start + len(b"xref"))
+    headers = []
+    for word in words:
+        if word == b"trailer":
+            break
+        require(INTEGER.fullmatch(word), f"Malformed xref subsection: {word}")
+        count = next(words, None)
+        require(count is not None and INTEGER.fullmatch(count),
+                "Malformed xref subsection header")
+        headers.append((int(word), int(count)))
+        for _ in range(3 * int(count)):
+            require(next(words, None) is not None, "Truncated xref subsection")
+    return headers
+
+
 def json_references(value):
     if isinstance(value, str):
         match = REFERENCE.fullmatch(value)
@@ -169,6 +186,11 @@ def inspect_pdf(path, original_size=0, password=""):
                     f"Full rewrite has orphan objects: {set(xref) - reachable}")
             require(all(gen == 0 for _, gen in xref),
                     "Full rewrite did not normalize generations")
+            if data[start:start + 4] == b"xref":
+                headers = subsections(data, start)
+                require(headers == [(0, trailer.get('/Size'))],
+                        f"Full rewrite's xref is not one subsection from 0 "
+                        f"through /Size: {headers}")
         else:
             require(written <= reachable,
                     f"Revision wrote orphan objects: {written - reachable}")

testing/tools/check_saved_pdfs_test.py

@@ -66,6 +66,16 @@ def test_missing_xref_entry_fails(self):
         with self.assertRaises((InvalidSavedPdf, pikepdf.PdfError)):
             self.check(data[:start] + b''.join(lines))
 
+    def test_full_rewrite_with_more_than_one_subsection_fails(self):
+        # The same entries, as subsections 0-2 and 3: legal for a lenient
+        # reader, but not the one subsection a file without updates has.
+        data, start = fixture()
+        lines = data[start:].splitlines(keepends=True)
+        lines[1] = b'0 3\n'
+        lines.insert(5, b'3 1\n')
+        with self.assertRaisesRegex(InvalidSavedPdf, 'one subsection'):
+            self.check(data[:start] + b''.join(lines))
+
     def test_custom_trailer_roots_are_followed(self):
         # This object is ordinary application data, not an encryption dictionary.
         data = fixture(custom_root=True)[0].replace(b'/Filter/Standard', b'/Filter/Custom  ')