fix(cos): an incremental save of a file with bytes before its header counts from the header

Some files carry bytes before `%PDF-`, and their offsets count from the header rather than from
the first byte. We read those, retrying a section relative to the header when the absolute
offset misses, but the incremental writer then appended a section whose offsets and `startxref`
were absolute while its `/Prev` kept the header-relative value it was given. qpdf reported "xref
not found" and reconstructed the file, so a save that reported success left a damaged-looking
update behind (#2364, found with KillerPDF's reserve3.pdf and a Subject edit).

The reader now records on each `Revision` whether it was found counting from the header, and the
writer counts the new section the same way as the one its `/Prev` points to. A file whose
offsets already include the prefix keeps absolute offsets, which the second test pins so the fix
cannot overcorrect.

Seen red with the writer change reverted: the appended section's `startxref` missed its `xref`.

Fixes storytold/pdfcraft#2364

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DsT1y8xBvkXeBxEUQqdDtj

crates/cos/src/document.rs

@@ -307,6 +307,10 @@ pub struct Revision {
     pub xref_offset: u64,
     /// `true` if the section is a cross-reference stream.
     pub is_stream: bool,
+    /// `true` if `xref_offset` counts from the `%PDF-` header rather than from the start of the
+    /// file. The two differ only in a file with bytes before its header, and an update appended
+    /// to such a file must count the same way as the section it points back to.
+    pub header_relative: bool,
 }
 
 #[derive(Clone, Debug)]
@@ -817,6 +821,11 @@ impl Document {
         &self.revisions
     }
 
+    /// Position of the `%PDF-` header in the file: 0 unless bytes come before it.
+    pub fn header_offset(&self) -> usize {
+        self.header_offset
+    }
+
     /// Where each revision of the file ends (oldest first): the byte after the `%%EOF` (and its
     /// end-of-line) that closes its cross-reference section. `data[..end]` is that revision as it
     /// was saved. A linearized file's first-page section belongs to the revision it precedes, so
@@ -1345,13 +1354,13 @@ impl Document {
                 log.push(format!("cross-reference chain loops at offset {off}; stopped"));
                 break;
             }
-            let (primary, sect_trailer, is_stream) = match self.read_compact_section(off as usize) {
-                Ok(Some(v)) => v,
+            let ((primary, sect_trailer, is_stream), header_relative) = match self.read_compact_section(off as usize) {
+                Ok(Some(v)) => (v, false),
                 Ok(None) => return Ok(None),
                 Err(e) if self.header_offset > 0 => {
                     log.push(format!("xref at {off} unreadable ({e}); retrying relative to the header"));
                     match self.read_compact_section(off as usize + self.header_offset)? {
-                        Some(v) => v,
+                        Some(v) => (v, true),
                         None => return Ok(None),
                     }
                 }
@@ -1359,7 +1368,7 @@ impl Document {
                 // materialized reader logs it and the file is rebuilt from object headers (#1656).
                 Err(e) => return Err(e),
             };
-            revisions.push(Revision { xref_offset: off, is_stream });
+            revisions.push(Revision { xref_offset: off, is_stream, header_relative });
             let Some(rows) = retained_rows.checked_add(primary.row_count).filter(|rows| *rows <= MAX_COMPACT_XREF_ROWS) else { return Ok(None) };
             let Some(bytes) = retained_bytes.checked_add(primary.retained_bytes()).filter(|bytes| *bytes <= MAX_COMPACT_XREF_BYTES) else {
                 return Ok(None);
@@ -1636,13 +1645,13 @@ impl Document {
             let mut sect = BTreeMap::new();
             // Object numbers whose entry in this section was set aside ([`Self::xref_oddities`]).
             let mut odd = Vec::new();
-            let (sect_trailer, is_stream) = match self.read_section(off as usize, &mut sect, &mut odd) {
-                Ok(v) => v,
+            let ((sect_trailer, is_stream), header_relative) = match self.read_section(off as usize, &mut sect, &mut odd) {
+                Ok(v) => (v, false),
                 Err(e) if self.header_offset > 0 => {
                     log.push(format!("xref at {off} unreadable ({e}); retrying relative to the header"));
                     sect.clear();
                     odd.clear();
-                    self.read_section(off as usize + self.header_offset, &mut sect, &mut odd)?
+                    (self.read_section(off as usize + self.header_offset, &mut sect, &mut odd)?, true)
                 }
                 Err(e) => {
                     // An unreadable older section loses every object only it locates (the page
@@ -1653,7 +1662,7 @@ impl Document {
                     return Err(e);
                 }
             };
-            revisions.push(Revision { xref_offset: off, is_stream });
+            revisions.push(Revision { xref_offset: off, is_stream, header_relative });
             // Hybrid files (§7.5.8.4): the /XRefStm entries supplement the table and replace the
             // free entries it gives objects only a cross-reference stream can locate.
             if let Some(x) = sect_trailer.int(b"XRefStm").filter(|x| *x >= 0) {

crates/cos/src/writer.rs

@@ -193,7 +193,11 @@ pub fn write_incremental(doc: &Document, opts: &SaveOptions) -> Result<Vec<u8>,
     // allocated once at its exact size: growing a buffer that holds a large original past a
     // guessed capacity would copy the whole file again and keep up to as much again unused.
     let newline = !matches!(original.last(), Some(b'\n' | b'\r'));
-    let base = (original.len() + usize::from(newline)) as u64;
+    // A file with bytes before its `%PDF-` header may count its offsets from the header. The new
+    // section then has to as well: readers take `/Prev` and `startxref` in one convention, and a
+    // mix sends them into reconstruction (#2364).
+    let shift = if doc.revisions().last().is_some_and(|r| r.header_relative) { doc.header_offset() } else { 0 };
+    let base = (original.len() + usize::from(newline)).saturating_sub(shift) as u64;
     let mut out = Vec::new();
     let mut offsets: BTreeMap<u32, (u64, u16, bool)> = BTreeMap::new(); // num → (offset, gen, in use)
     for (num, generation, obj) in doc.overlay_entries() {

crates/cos/tests/writer.rs

@@ -392,3 +392,66 @@ fn full_save_preserves_unknown_trailer_data() {
     assert_eq!(trailer_note(&back), trailer_note(&doc));
     assert_eq!(back.trailer().get(b"Version"), doc.trailer().get(b"Version"));
 }
+
+/// The `startxref` value at the end of `bytes`.
+fn last_startxref(bytes: &[u8]) -> usize {
+    let at = bytes.windows(9).rposition(|w| w == b"startxref").unwrap();
+    String::from_utf8_lossy(&bytes[at + 9..]).split_whitespace().next().unwrap().parse().unwrap()
+}
+
+#[test]
+fn incremental_saves_of_a_file_with_bytes_before_its_header_count_from_the_header() {
+    // Offsets written before the prefix was added count from `%PDF-`, as in the KillerPDF file
+    // of #2364; the appended section must count the same way as the `/Prev` it points back to.
+    let prefix = vec![b'#'; 128];
+    let as_stream = write_full(&Document::open(Arc::new(fixture())).unwrap(), &SaveOptions::default()).unwrap();
+    for (name, body, marker) in [("table", classic_table(), &b"xref"[..]), ("stream", as_stream, &b" 0 obj"[..])] {
+        let input = [prefix.as_slice(), &body].concat();
+        let mut doc = Document::open(Arc::new(input.clone())).unwrap();
+        assert!(doc.revisions().last().unwrap().header_relative, "{name}");
+        doc.update_dict(ObjRef::new(1, 0), |d| d.set(b"Lang".to_vec(), Object::String(pdfcraft_cos::PdfString::literal(b"en".to_vec())))).unwrap();
+        let saved = write_incremental(&doc, &SaveOptions::default()).unwrap();
+        assert!(saved.starts_with(&input), "{name}");
+
+        let xref = last_startxref(&saved) + prefix.len();
+        let section = saved.get(xref..).unwrap_or_default();
+        assert!(section.windows(marker.len()).position(|w| w == marker).is_some_and(|p| p < 16), "{name}: startxref misses its section");
+        let back = Document::open(Arc::new(saved.clone())).unwrap();
+        assert_eq!(back.revisions().len(), doc.revisions().len() + 1, "{name}");
+        assert!(back.revisions().iter().all(|r| r.header_relative), "{name}: one convention across the chain");
+        let edited = back.get(ObjRef::new(1, 0));
+        assert_eq!(edited.as_dict().unwrap().get(b"Lang").and_then(|o| o.as_string()).map(|s| s.bytes.as_slice()), Some(b"en".as_slice()));
+        assert!(back.xref_entry_is_exact(1), "{name}");
+        assert_qpdf_clean(&saved);
+    }
+}
+
+#[test]
+fn incremental_saves_of_a_file_whose_offsets_count_from_its_first_byte_keep_counting_so() {
+    // The other convention: the same file, but its offsets include the bytes before the header.
+    let prefix = vec![b'#'; 128];
+    let mut input = [prefix.as_slice(), &classic_table()].concat();
+    let xref = input.windows(6).rposition(|w| w == b"\nxref\n").unwrap() + 1;
+    let table: String = String::from_utf8_lossy(&input[xref..])
+        .lines()
+        .map(|line| match line.strip_suffix(" 00000 n ").and_then(|o| o.parse::<usize>().ok()) {
+            Some(o) => format!("{:010} 00000 n \n", o + prefix.len()),
+            None if line.parse::<usize>().is_ok() => format!("{xref}\n"),
+            None => format!("{line}\n"),
+        })
+        .collect();
+    input.truncate(xref);
+    input.extend_from_slice(table.as_bytes());
+
+    let mut doc = Document::open(Arc::new(input.clone())).unwrap();
+    assert!(doc.repair_log().is_empty(), "{:?}", doc.repair_log());
+    assert!(!doc.revisions().last().unwrap().header_relative);
+    doc.update_dict(ObjRef::new(1, 0), |d| d.set(b"Lang".to_vec(), Object::String(pdfcraft_cos::PdfString::literal(b"en".to_vec())))).unwrap();
+    let saved = write_incremental(&doc, &SaveOptions::default()).unwrap();
+    assert!(saved.get(last_startxref(&saved)..).unwrap_or_default().starts_with(b"xref"));
+    let back = Document::open(Arc::new(saved.clone())).unwrap();
+    assert!(back.repair_log().is_empty(), "{:?}", back.repair_log());
+    assert_eq!(back.revisions().len(), 2);
+    assert!(back.xref_entry_is_exact(1));
+    assert_qpdf_clean(&saved);
+}