From 87b448b664e1b3c960b479cc0f2c2e7a82c7ac71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rk=20Bartalos?= Date: Mon, 12 Oct 2026 14:57:37 +0200 Subject: [PATCH] M1: write a valid xref when an incremental save has no objects (#1339) (#1424) An unchanged classic-table file used to append xref immediately followed by trailer, which qpdf treats as a damaged file. A zero-length subsection keeps trailer-only updates without rewriting any object entry. Co-authored-by: Brandon Thomas --- ROADMAP.md | 2 + crates/cos/src/writer.rs | 9 +++ crates/cos/tests/writer.rs | 110 +++++++++++++++++++++++++++++++++++++ docs/roadmap.md | 5 +- 4 files changed, 125 insertions(+), 1 deletion(-) diff --git a/ROADMAP.md b/ROADMAP.md index 8c3c70da..44a26604 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,6 +2,7 @@ **Stage: alpha** · next: beta, ~26% (ready for real work 49% → 75%) and ~400–700 h away +> **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-12 · **Change:** minor (M1 #1339: an unchanged incremental save of a classic cross-reference table writes a zero-length subsection; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-12 · **Change:** minor (M14 Windows low-DPI font-hinting candidate measured on macOS; no Windows runtime claim; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-12 · **Change:** minor (M3: Windows' Animation effects setting is followed, and Preferences has Animation effects: system, on or off); earlier: exclusive image-crop resize follow-up; M2 short same-line redraws split before placement; estimates unchanged · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-12 · **Change:** minor (M7 #1200 candidate: local GoToR links and confined automation; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) @@ -140,6 +141,7 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md ## Progress log +- **2026-10-12 (M1, #1339):** An incremental save of an unchanged classic-table file used to append `xref` and then `trailer` with no subsection, which qpdf reports as a damaged file. The section now has a zero-length subsection, so no object entry changes and a trailer-only edit is still written. Estimates unchanged. - **2026-10-12 (M12, auto-tag preview):** The engine and `accessibility_autotag_preview` propose paragraph and large-heading roles for bounded text blocks on one page, with locations and reasons. Every proposal needs review; images remain unclassified. No PDF tags are written yet. Synthetic tool tests cover normal text, a larger heading, bounds and unchanged source bytes. Estimates unchanged. - **2026-10-12 (M10, editing OCR text on scans):** Editing a recognised word on a scanned page now changes what the page shows. Before, the rewrite stayed invisible OCR text, so the scan still showed the old word; on a form, the whole OCR layer also grouped into one paragraph that rewrapped when one word changed. OCR words now group into phrases (a table cell or a tab stop splits them, and lines stay apart); an edit draws the phrase visibly in the scan's sampled ink colour over a cover in the sampled paper colour, keeping each word's width and growing rightwards. The editor covers the scanned word while typing. OCR words are now sized from their letters (a capitals-only word's box spans the cap height, not the full ascender-descender span), so recognised and rewritten text is as tall as the scanned word and sits on its baseline. Five engine tests on synthetic scans and a kittest UI test; `ocr.editable-text-output` is partial. - **2026-10-12 (community, M14, recovery metadata):** A recovery snapshot whose metadata is missing, damaged or names another snapshot is no longer deleted when the folder is listed: it is offered for recovery without an original path (Save asks for a destination), and encrypted snapshots still ask for their password. Recovering one writes fresh metadata in place of the damaged file, and clearing the folder also removes metadata left without a snapshot. Before, a failed metadata write meant the next start deleted the finished snapshot. Synthetic regressions cover the failed write, missing, corrupt, mismatched and encrypted cases. Estimates unchanged. diff --git a/crates/cos/src/writer.rs b/crates/cos/src/writer.rs index 26dcfc46..34254bf3 100644 --- a/crates/cos/src/writer.rs +++ b/crates/cos/src/writer.rs @@ -431,6 +431,15 @@ fn write_xref_table(out: &mut Vec, offsets: &BTreeMap if include_zero { all.insert(0, (0, (0, 65535, false))); } + // A section is one or more subsections. `xref` followed immediately by `trailer` is not a + // table (qpdf: "xref syntax invalid") and is what an incremental save of a classic-table + // file used to append when nothing was edited. A zero-length subsection names no objects, + // so the free list and every in-use entry stay as they were, while a trailer-only change + // (a missing `/ID`, or a caller edit of the trailer) is still written. + if all.is_empty() { + let _ = writeln!(out, "0 0"); + return; + } let mut i = 0; while i < all.len() { let start = all[i].0; diff --git a/crates/cos/tests/writer.rs b/crates/cos/tests/writer.rs index 448cf357..b7514cc0 100644 --- a/crates/cos/tests/writer.rs +++ b/crates/cos/tests/writer.rs @@ -135,6 +135,116 @@ fn full_saves_are_allocated_at_their_exact_size() { } } +/// A one-page classic-table file with no `/ID`, the case that used to append `xref` and then +/// `trailer` with no subsection (#1339). +fn classic_table() -> Vec { + let objects = [ + "<< /Type /Catalog /Pages 2 0 R >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>", + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Resources << >> /Contents 4 0 R >>", + "<< /Length 0 >>\nstream\n\nendstream", + ]; + let mut bytes = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, object) in objects.iter().enumerate() { + offsets.push(bytes.len()); + bytes.extend_from_slice(format!("{} 0 obj\n{object}\nendobj\n", i + 1).as_bytes()); + } + let xref = bytes.len(); + bytes.extend_from_slice(b"xref\n0 5\n0000000000 65535 f \n"); + for offset in offsets { + bytes.extend_from_slice(format!("{offset:010} 00000 n \n").as_bytes()); + } + bytes.extend_from_slice(format!("trailer\n<< /Size 5 /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n").as_bytes()); + bytes +} + +fn qpdf_check(bytes: &[u8]) -> Option { + let Ok(version) = std::process::Command::new("qpdf").arg("--version").output() else { + return None; + }; + assert!(version.status.success()); + static NEXT: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); + let n = NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let path = std::env::temp_dir().join(format!("pdfcraft-xref-{}-{n}.pdf", std::process::id())); + std::fs::write(&path, bytes).unwrap(); + let check = std::process::Command::new("qpdf").arg("--check").arg(&path).output().unwrap(); + let _ = std::fs::remove_file(&path); + Some(check) +} + +fn page(doc: &Document) -> Dict { + let root = doc.resolve(doc.trailer().get(b"Root").unwrap()); + let pages = doc.resolve(root.as_dict().unwrap().get(b"Pages").unwrap()); + let kids = pages.as_dict().unwrap().get(b"Kids").unwrap().as_array().unwrap(); + doc.resolve(kids.first().unwrap()).as_dict().cloned().unwrap() +} + +fn assert_qpdf_clean(bytes: &[u8]) { + let Some(check) = qpdf_check(bytes) else { + eprintln!("qpdf not installed: skipped"); + return; + }; + let report = format!("{}{}", String::from_utf8_lossy(&check.stdout), String::from_utf8_lossy(&check.stderr)); + assert!(check.status.success() && !report.contains("WARNING"), "{report}"); +} + +#[test] +fn unchanged_incremental_save_of_a_classic_table_is_a_valid_xref() { + let input = classic_table(); + let doc = Document::open(Arc::new(input.clone())).unwrap(); + assert!(!doc.is_modified()); + assert!(doc.repair_log().is_empty()); + assert_eq!(doc.revisions().len(), 1); + assert!(!doc.revisions()[0].is_stream); + + let unchanged = write_incremental(&doc, &SaveOptions::default()).unwrap(); + assert!(unchanged.starts_with(&input)); + let appended = &unchanged[input.len()..]; + assert!(appended.windows(8).any(|w| w == b"xref\n0 0"), "a section with no entries still needs a subsection"); + assert!(!appended.windows(13).any(|w| w == b"xref\ntrailer")); + + let back = Document::open(Arc::new(unchanged.clone())).unwrap(); + assert!(back.repair_log().is_empty(), "{:?}", back.repair_log()); + assert_eq!(back.revisions().len(), 2); + assert_eq!(page(&back).name(b"Type"), Some(b"Page".as_slice())); + assert!(back.trailer().get(b"ID").is_some(), "a file without an identifier still receives one"); + assert_eq!(hayro_pages(&unchanged), 1); + assert_qpdf_clean(&unchanged); + + let mut edited = doc.clone(); + edited.update_dict(ObjRef::new(3, 0), |d| d.set(b"Rotate".to_vec(), Object::Int(90))).unwrap(); + let edited = write_incremental(&edited, &SaveOptions::default()).unwrap(); + let edited_back = Document::open(Arc::new(edited.clone())).unwrap(); + assert!(edited_back.repair_log().is_empty(), "{:?}", edited_back.repair_log()); + assert_eq!(edited_back.get(ObjRef::new(3, 0)).as_dict().unwrap().int(b"Rotate"), Some(90)); + assert_qpdf_clean(&edited); + + let stamped = write_incremental(&doc, &SaveOptions { mod_date: Some("D:20260101000000Z".into()), ..SaveOptions::default() }).unwrap(); + let stamped_back = Document::open(Arc::new(stamped.clone())).unwrap(); + assert!(stamped_back.repair_log().is_empty(), "{:?}", stamped_back.repair_log()); + let info = stamped_back.resolve(stamped_back.trailer().get(b"Info").unwrap()); + assert_eq!( + info.as_dict().unwrap().get(b"ModDate").and_then(|d| d.as_string()).map(|s| s.bytes.as_slice()), + Some(b"D:20260101000000Z".as_slice()) + ); + assert_qpdf_clean(&stamped); +} + +#[test] +fn incremental_save_keeps_a_trailer_only_edit() { + let input = classic_table(); + let mut doc = Document::open(Arc::new(input)).unwrap(); + assert!(!doc.is_modified()); + doc.trailer_mut().set(b"Custom".to_vec(), Object::String(pdfcraft_cos::PdfString::literal(b"keep me".to_vec()))); + let saved = write_incremental(&doc, &SaveOptions::default()).unwrap(); + let back = Document::open(Arc::new(saved.clone())).unwrap(); + assert!(back.repair_log().is_empty(), "{:?}", back.repair_log()); + assert_eq!(back.trailer().get(b"Custom").and_then(|o| o.as_string()).map(|s| s.bytes.as_slice()), Some(b"keep me".as_slice())); + assert_eq!(hayro_pages(&saved), 1); + assert_qpdf_clean(&saved); +} + #[test] fn qpdf_accepts_object_stream_output() { let Ok(out) = std::process::Command::new("qpdf").arg("--version").output() else { diff --git a/docs/roadmap.md b/docs/roadmap.md index 0f8dc062..9c6d39c9 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -1,5 +1,7 @@ # PdfCraft roadmap: milestones and what's next +> **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-12 · **Change:** minor (M1 #1339: unchanged incremental saves of a classic cross-reference table write a zero-length subsection; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) +> **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-12 · **Change:** minor (M3: Windows' Animation effects setting is followed, and Preferences has Animation effects; earlier: exclusive image-crop resize follow-up; M2: short same-line redraws split before placement; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-12 · **Change:** minor (M7: preserve Arabic ActualText typing order; earlier: M3: Windows' Animation effects setting is followed, and Preferences has Animation effects; earlier: exclusive image-crop resize follow-up; M2: short same-line redraws split before placement; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-12 · **Change:** minor (M7: local GoToR links and confined automation candidate; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) > **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-12 · **Change:** exclusive image-crop resize follow-up; minor (M2: short same-line redraws split before placement; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS) @@ -78,7 +80,7 @@ total matches the parity estimate. | M | Milestone | Plan size (h) | Done | Remaining (h, 2026-10-10) | Notes | |---|---|---|---|---|---| | M0 | Skeleton: workspace, xtask gates, CI | 15–30 | 98% | 1–2 | GitHub workflow, `deny.toml` (licence audit of every dependency), parity checklist (826 features, `xtask parity`) done. Missing: remaining crate stubs, testkit/oracle crates | -| M1 | COS: filters, crypt, parser, xref, writer | 120–200 | 80% | 20–40 | Done:
- filters and crypt: every standard-security revision R2–R6 (RC4, AES-128/256), SASLprep, permissions, creating encryption;
- cos: parse and repair (including invalid xref object-number ranges), decoded-stream limits including unfiltered data, decrypt on load, re-encrypt on save, incremental and full writing.
Corpus: open/edit/save passes on 958 files, and all 7 password-protected files open.
Full saves now pack objects into compressed object streams. Fuzzing runs nightly (`xtask fuzz`). Missing: ≥ 250 tests, own image codecs | +| M1 | COS: filters, crypt, parser, xref, writer | 120–200 | 80% | 20–40 | Done:
- filters and crypt: every standard-security revision R2–R6 (RC4, AES-128/256), SASLprep, permissions, creating encryption;
- cos: parse and repair (including invalid xref object-number ranges), decoded-stream limits including unfiltered data, decrypt on load, re-encrypt on save, incremental and full writing.
Corpus: open/edit/save passes on 958 files, and all 7 password-protected files open.
Full saves now pack objects into compressed object streams. An unchanged incremental save of a classic table writes a zero-length cross-reference subsection instead of a section with no subsections (#1339). Fuzzing runs nightly (`xtask fuzz`). Missing: ≥ 250 tests, own image codecs | | M2 | Model, render, text | 200–350 | 15% | 130–230 | hayro bootstrap renderer (vendored patches); inspector predictor scratch is bounded by available input, and JPX sample rescaling handles 1–32 bits safely. Text extraction reaches word-F1 0.98 against pdftotext. Short same-line redraws split before placement rather than interleaving. Missing: model crate, fonts, DisplayList, renderer independent of hayro. Inspector page-label traversal and output are resource-bounded, with physical-number fallback warnings | | M3 | Viewer app (native + web) | 80–150 | 90% | 10–20 | Acrobat-style shell, find, select, panels, tiles, web build, UI control channel for agents (opt-in). Missing: 60 fps test on a 500-page document, snapshot tests of every panel
Done since: fit visible, document title in the window; Linux middle-button scrolling; shared three-state theme preferences in the toolbar, View menu and control channel; numeric Linux portal events and idle wakeups completing #881, with native appearance inheritance retained (#446); late browser startup downloads preserve the current document and Save target (#171). | | M4 | Engine, history, save, organize | 100–180 | 95% | 8–15 | Done:
- command registry (menus, shortcuts and palette all use it);
- undo/redo; incremental, atomic and encrypted saves;
- autosave and crash recovery;
- organize, combine, extract, split and insert-from-file, with identical fonts and images stored once;
- CLI `edit/combine/extract/split`.
Done since: Organize opens directly into the page overview, with checkbox selection, an editable page-range selection (#1092), and per-page rotate/delete controls; Combine files in its own tab (sortable, resizable columns; folders; early warnings; unlocking protected files), bookmark editing, page labels (Number pages), CLI `run`, Set Page Boxes, Crop tool, Duplicate pages, Ctrl+A / Cmd+A to select all pages in Organize, Replace pages, bookmarks from tagged headings. Missing: page transitions, recovery on the web | @@ -143,6 +145,7 @@ M2 (renderer), M7 (content editing), M10 (OCR and Office), M11 (Preflight and st | Date | Change | Summary | |---|---|---| +| 2026-10-12 | minor | M1 #1339: unchanged incremental saves of a classic cross-reference table write a zero-length subsection. Estimates unchanged. | | 2026-10-12 | minor | M12: bounded text-only auto-tag preview through the engine and automation; actual PDF structure writing and visual review remain open. Estimates unchanged. | | 2026-10-12 | minor | M7: keep added Arabic copy/search in typing order with per-line ActualText; preserve that order in the editor and keep visual runs separate. Font-independent regressions and tool save/reopen coverage; font fallback and bidi typing remain partial. Estimates unchanged. | | 2026-10-12 | minor | M3: case-insensitive Find matches normal, final and capital Greek sigma; exact search, word boundaries, glyph ranges and match limits have regression coverage. The tab-menu test uses Finder on macOS. Estimates unchanged. |