diff --git a/apps/pdfcraft-cli/tests/edit.rs b/apps/pdfcraft-cli/tests/edit.rs index 2864dcfb3..b35b71ddd 100644 --- a/apps/pdfcraft-cli/tests/edit.rs +++ b/apps/pdfcraft-cli/tests/edit.rs @@ -239,6 +239,102 @@ fn the_append_destination_and_counts_grown_by_earlier_edits_are_accepted() { } } +/// Two pages whose catalog points at a page tree object the file does not have (#1656). The +/// reader finds the pages by scanning, in file order, so `edit` must work on the same pages. +/// Object 3 is written before object 2, so file order and object numbers disagree. +fn fixture_without_page_tree() -> Vec { + let stream = |content: &str| format!("<< /Length {} >>\nstream\n{content}\nendstream", content.len()).into_bytes(); + let objs: Vec<(usize, Vec)> = vec![ + (1, b"<< /Type /Catalog /Pages 6 0 R >>".to_vec()), + (3, b"<< /Type /Page /Parent 6 0 R /MediaBox [0 0 200 300] /Contents 4 0 R >>".to_vec()), + (2, b"<< /Type /Page /Parent 6 0 R /MediaBox [0 0 300 200] /Contents 5 0 R >>".to_vec()), + (4, stream("0 0 m 10 10 l S")), + (5, stream("0 0 m 20 20 l S")), + ]; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = vec![0; objs.len()]; + for (num, o) in &objs { + offsets[num - 1] = out.len(); + out.extend_from_slice(format!("{num} 0 obj\n").as_bytes()); + out.extend_from_slice(o); + out.extend_from_slice(b"\nendobj\n"); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + out +} + +#[test] +fn a_document_whose_page_tree_is_missing_edits_the_pages_the_reader_shows() { + let input = tmp("no-page-tree", "in.pdf"); + std::fs::write(&input, fixture_without_page_tree()).unwrap(); + let before = pdfcraft_render::inspect(std::sync::Arc::new(fixture_without_page_tree()), None).unwrap(); + assert_eq!(before.pages.iter().map(|p| (p.width, p.height)).collect::>(), [(200.0, 300.0), (300.0, 200.0)]); + for (name, args, expected) in [ + // Width and height are as displayed: the turned page reads 200 x 300. + ("rotate.pdf", ["--rotate", "2:90"], &[(200.0, 300.0, 0), (200.0, 300.0, 90)][..]), + ("delete.pdf", ["--delete", "1"], &[(300.0, 200.0, 0)][..]), + ] { + let output = tmp("no-page-tree", name); + let mut all = vec!["edit", input.to_str().unwrap(), "--out", output.to_str().unwrap()]; + all.extend(args); + let out = Command::new(BIN).args(&all).output().unwrap(); + assert!(out.status.success(), "{name}: {}", String::from_utf8_lossy(&out.stderr)); + let info = pdfcraft_render::inspect(std::sync::Arc::new(std::fs::read(&output).unwrap()), None).unwrap(); + let pages: Vec<_> = info.pages.iter().map(|p| (p.width, p.height, p.rotation)).collect(); + assert_eq!(pages, expected, "{name}"); + } +} + +/// Like [`fixture_without_page_tree`], but the scanning reader shows different pages from the +/// page objects: it skips the blank 200 x 200 page (no `/Contents`) and shows the Text +/// annotation (which has `/Contents`) as a page. No page tree may be rebuilt from such a file. +fn fixture_whose_scan_disagrees() -> Vec { + let stream = |content: &str| format!("<< /Length {} >>\nstream\n{content}\nendstream", content.len()).into_bytes(); + let objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 9 0 R >>".to_vec(), + b"<< /Type /Page /Parent 9 0 R /MediaBox [0 0 100 100] /Contents 5 0 R >>".to_vec(), + b"<< /Type /Page /Parent 9 0 R /MediaBox [0 0 200 200] >>".to_vec(), + b"<< /Type /Page /Parent 9 0 R /MediaBox [0 0 300 300] /Contents 6 0 R /Annots [7 0 R] >>".to_vec(), + stream("0 0 m 10 10 l S"), + stream("0 0 m 20 20 l S"), + b"<< /Type /Annot /Subtype /Text /Rect [10 10 30 30] /Contents (hello) >>".to_vec(), + ]; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n", i + 1).as_bytes()); + out.extend_from_slice(o); + out.extend_from_slice(b"\nendobj\n"); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + out +} + +#[test] +fn a_page_tree_is_not_rebuilt_when_the_reader_shows_other_pages() { + let input = tmp("scan-disagrees", "in.pdf"); + std::fs::write(&input, fixture_whose_scan_disagrees()).unwrap(); + let shown = pdfcraft_render::inspect(std::sync::Arc::new(fixture_whose_scan_disagrees()), None).unwrap(); + assert_eq!(shown.pages.len(), 3); + assert!(shown.pages.iter().all(|p| p.width != 200.0), "the reader must not show the blank page"); + let output = tmp("scan-disagrees", "out.pdf"); + let out = Command::new(BIN).args(["edit", input.to_str().unwrap(), "--out", output.to_str().unwrap(), "--delete", "2"]).output().unwrap(); + assert_eq!(out.status.code(), Some(1)); + assert_eq!(String::from_utf8_lossy(&out.stderr), "pdfcraft-cli: the document's page tree lists no pages: the file is damaged\n"); + assert!(!output.exists(), "no page may be deleted"); +} + /// A page past the end names its own option and the page count left by the earlier options (#1550). #[test] fn a_page_past_the_end_after_earlier_edits_names_that_option() { diff --git a/crates/cos/src/document.rs b/crates/cos/src/document.rs index 46b59abfc..a0b1f56ee 100644 --- a/crates/cos/src/document.rs +++ b/crates/cos/src/document.rs @@ -1355,13 +1355,9 @@ impl Document { None => return Ok(None), } } - Err(e) => { - if sections.is_empty() { - return Err(e); - } - log.push(format!("older cross-reference section at {off} is unreadable: {e}")); - break; - } + // An unreadable older section loses every object only it locates: the + // 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 }); let Some(rows) = retained_rows.checked_add(primary.row_count).filter(|rows| *rows <= MAX_COMPACT_XREF_ROWS) else { return Ok(None) }; @@ -1649,11 +1645,12 @@ impl Document { self.read_section(off as usize + self.header_offset, &mut sect, &mut odd)? } Err(e) => { - if revisions.is_empty() { - return Err(e); + // An unreadable older section loses every object only it locates (the page + // tree, often): rebuild from object headers, as readers do (#1656). + if !revisions.is_empty() { + log.push(format!("older cross-reference section at {off} is unreadable: {e}")); } - log.push(format!("older cross-reference section at {off} is unreadable: {e}")); - break; + return Err(e); } }; revisions.push(Revision { xref_offset: off, is_stream }); @@ -2188,6 +2185,32 @@ mod tests { } } + #[test] + fn objects_only_an_unreadable_older_section_locates_are_found_by_scanning() { + // The newest section lists the catalog only; its /Prev points past the end of the file, + // so the page tree is reachable only by scanning object headers, as readers do (#1656). + let mut bytes = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (num, body) in + [(1, "<< /Type /Catalog /Pages 2 0 R >>"), (2, "<< /Type /Pages /Kids [3 0 R] /Count 1 >>"), (3, "<< /Type /Page /Parent 2 0 R >>")] + { + offsets.push(bytes.len()); + bytes.extend_from_slice(format!("{num} 0 obj\n{body}\nendobj\n").as_bytes()); + } + let xref = bytes.len(); + bytes.extend_from_slice( + format!( + "xref\n0 2\n0000000000 65535 f \n{:010} 00000 n \ntrailer\n<< /Root 1 0 R /Size 4 /Prev 999999 >>\nstartxref\n{xref}\n%%EOF\n", + offsets[0] + ) + .as_bytes(), + ); + let doc = Document::open(Arc::new(bytes)).unwrap(); + assert_eq!(doc.get(ObjRef::new(2, 0)).as_dict().and_then(|d| d.name(b"Type")), Some(b"Pages".as_slice())); + assert_eq!(doc.get(ObjRef::new(3, 0)).as_dict().and_then(|d| d.name(b"Type")), Some(b"Page".as_slice())); + assert!(doc.repair_log().iter().any(|l| l.contains("999999")), "{:?}", doc.repair_log()); + } + #[test] fn self_referential_length_does_not_overflow() { let bytes = build( diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 7a858581f..01f162ae7 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -875,6 +875,12 @@ fn is_placeholder_title(title: &str) -> bool { /// What is displayed: the working file, plus (in memory only, never saved) the appearances /// PdfCraft draws for comments that have none ([`pdfcraft_annot::with_missing_appearances`]). /// The working file is unchanged, and so is everything that reads it (saving, signatures, …). +/// The reader shows the same pages in `a` and `b`: count, displayed size and rotation. +fn same_pages(a: &[pdfcraft_render::PageInfo], b: &[pdfcraft_render::PageInfo]) -> bool { + a.len() == b.len() + && a.iter().zip(b).all(|(x, y)| (x.width - y.width).abs() < 0.01 && (x.height - y.height).abs() < 0.01 && x.rotation == y.rotation) +} + fn display_bytes(editor: Option<&mut Editor>, bytes: &Arc>) -> Arc> { let Some(display) = editor.and_then(|e| pdfcraft_annot::with_missing_appearances_cached(&e.cos, &mut e.appearances)) else { return bytes.clone(); @@ -3452,6 +3458,31 @@ impl Session { }, (_, cos) => (bytes, info, cos), }; + // The catalog leads to no page, but the reader found pages by scanning: gather the page + // objects into a page tree as one more revision, or page edits address none of them + // (#1656). Kept only when the reader then shows the same pages, and never on a signed + // document, where writing the repair could rewrite the file under its signatures. + let (bytes, info, cos) = match cos { + Ok(Ok(cos)) if !info.pages.is_empty() && pdfcraft_organize::page_count(&cos).unwrap_or(0) == 0 => { + let mut work = cos.clone(); + let signed = || pdfcraft_sign::pdf::list(&cos, &bytes, &context.trust).iter().any(|s| s.signed); + let rebuilt = match pdfcraft_organize::recover_page_tree(&mut work) { + Ok(n) if n > 0 && !signed() => rebase(&work).ok().filter(|(_, i, _)| same_pages(&i.pages, &info.pages)), + _ => None, + }; + match rebuilt { + Some((b, i, mut c)) => { + for line in cos.repair_log() { + c.note_repair(line.clone()); + } + c.note_repair(format!("the page tree is missing; its {} pages were found by scanning the file", i.pages.len())); + (b, i, Ok(Ok(c))) + } + None => (bytes, info, Ok(Ok(cos))), + } + } + cos => (bytes, info, cos), + }; // XFA notes survive the document being re-read after edits. let xfa_warnings: Vec = if info.xfa.is_some() { info.warnings.clone() } else { Vec::new() }; let mut document = Self::build_document(&context.trust, name, path, bytes, info, cos, render_password, password, xfa); @@ -3690,8 +3721,9 @@ impl Session { } // Signed documents are only ever saved incrementally: an edit that needs a full rewrite // (applying redactions, changing security, sanitizing) would invalidate the signatures. + // So does any edit to a file rebuilt from object headers: it has no chain to append to. let rewrites = |c: &pdfcraft_cos::Document| c.full_save_required() || c.encryption_changed(); - if signed && rewrites(&next) && !rewrites(&editor.cos) { + if signed && ((rewrites(&next) && !rewrites(&editor.cos)) || (next.is_modified() && next.revisions().is_empty())) { return Err(EditError::SignedRewrite(edit.label().to_string())); } // What this edit drew with `?` (a value or comment text its font can't draw, stored as diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index d61c82d9d..ed90eea41 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -5126,6 +5126,50 @@ fn copy_image_finds_a_picture_inside_form_artwork_under_text() { assert!(d.page_image_copy_file(0, 1).is_err()); } +/// `fixture(2)` with the catalog pointing at a page tree object the file does not have (#1656). +fn fixture_without_page_tree() -> Vec { + let bytes = fixture(2); + let catalog = b"<< /Type /Catalog /Pages 2 0 R >>"; + let at = bytes.windows(catalog.len()).position(|w| w == catalog).unwrap(); + let mut out = bytes.clone(); + out[at..at + catalog.len()].copy_from_slice(b"<< /Type /Catalog /Pages 9 0 R >>"); + out +} + +#[test] +fn a_recovered_page_tree_opens_unmodified_and_edits_the_pages_shown() { + let mut s = Session::new(); + let id = s.open("damaged.pdf", None, Arc::new(fixture_without_page_tree()), None).unwrap(); + let doc = s.get(id).unwrap(); + assert_eq!(doc.info.pages.len(), 2); + assert!(!doc.editor.as_ref().unwrap().cos.is_modified(), "the repair is a revision of the opened file, not an unsaved edit"); + assert!(doc.repair_log().iter().any(|l| l.contains("page tree is missing")), "{:?}", doc.repair_log()); + s.apply(id, Edit::RotatePages { pages: vec![1], degrees: 90 }).unwrap(); + assert_eq!(s.get(id).unwrap().info.pages.iter().map(|p| p.rotation).collect::>(), [0, 90]); +} + +#[test] +fn a_signed_document_rebuilt_from_object_headers_refuses_edits() { + let p12 = std::fs::read(concat!(env!("CARGO_MANIFEST_DIR"), "/../sign/tests/data/ec-p256.p12")).unwrap(); + let digital_id = sign::pkcs12::open(&p12, "test").unwrap(); + let mut s = Session::new().with_clock(|| 1_800_000_000); + let id = s.open("fixture.pdf", None, Arc::new(fixture(2)), None).unwrap(); + let opts = SignOptions { page: 1, rect: Some([20.0, 20.0, 180.0, 60.0]), ..SignOptions::default() }; + let signed = s.sign(id, &digital_id, opts).unwrap(); + s.mark_signed(id, signed, None).unwrap(); + // A broken startxref: the file opens only by rebuilding from object headers, and any save + // of it is a full rewrite. + let mut bytes = s.get(id).unwrap().bytes.as_ref().clone(); + let sx = bytes.windows(10).rposition(|w| w == b"startxref\n").unwrap() + 10; + let end = sx + bytes[sx..].iter().position(|b| !b.is_ascii_digit()).unwrap(); + bytes.splice(sx..end, b"1".iter().copied()); + let id = s.open("damaged.pdf", None, Arc::new(bytes), None).unwrap(); + assert!(s.get(id).unwrap().is_signed()); + assert!(s.get(id).unwrap().editor.as_ref().unwrap().cos.revisions().is_empty(), "the fixture must be rebuilt"); + assert!(matches!(s.apply(id, rect_comment(0, [80.0, 80.0, 120.0, 120.0])), Err(EditError::SignedRewrite(_)))); + assert!(!s.get(id).unwrap().dirty); +} + /// #1324: marking a figure decorative rewrites that figure only. The header, added text and an /// unknown entry on the original stream stay attached to the page. This lives in the engine tests /// because edit and a11y are the same layer and cannot depend on each other. diff --git a/crates/organize/src/lib.rs b/crates/organize/src/lib.rs index 1d8a1e92d..3b2873abe 100644 --- a/crates/organize/src/lib.rs +++ b/crates/organize/src/lib.rs @@ -11,7 +11,7 @@ #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] -use pdfcraft_cos::{Dict, Document, ObjRef, Object, PdfString}; +use pdfcraft_cos::{Dict, Document, ObjRef, Object, PdfString, XrefEntry}; mod boxes; mod dedupe; @@ -148,8 +148,44 @@ fn rebuild(doc: &mut Document, order: &[(ObjRef, Dict)]) -> Result<(), OrganizeE Ok(()) } +/// Gather the document's `/Type /Page` objects into a new page tree when the catalog leads to no +/// page (its `/Pages` is missing or points at a lost object). Readers then find the pages by +/// scanning the file, in file order, and edits must address those same pages. Returns how many +/// pages were gathered; 0 leaves the document unchanged. +pub fn recover_page_tree(doc: &mut Document) -> Result { + if walk(doc).is_ok_and(|pages| !pages.is_empty()) { + return Ok(0); + } + let catalog = doc.root().ok_or(OrganizeError::NoPageTree)?; + let position = |num: u32| match doc.xref_entry(num) { + Some(XrefEntry::InFile { offset, .. }) => (offset, 0), + Some(XrefEntry::InStream { stream, index }) => match doc.xref_entry(stream) { + Some(XrefEntry::InFile { offset, .. }) => (offset, index), + _ => (u64::MAX, index), + }, + _ => (u64::MAX, 0), + }; + let mut found: Vec<((u64, u32), ObjRef)> = doc + .scan_objects() + .filter(|(_, o)| o.as_dict().is_some_and(|d| d.name(b"Type") == Some(b"Page"))) + .map(|(r, _)| (position(r.num), r)) + .collect(); + if found.is_empty() { + return Ok(0); + } + found.sort(); + let root = doc.add(Dict::new()); + doc.update_dict(catalog, |d| d.set(b"Pages".to_vec(), Object::Ref(root)))?; + let order: Vec<(ObjRef, Dict)> = found.into_iter().map(|(_, r)| (r, Dict::new())).collect(); + rebuild(doc, &order)?; + Ok(order.len()) +} + fn check(indices: &[usize], n: usize) -> Result<(), OrganizeError> { match indices.iter().find(|i| **i >= n) { + // A viewer still shows pages a lost page tree leaves out (#1656): "page 1 does not + // exist" would contradict it. + Some(_) if n == 0 => Err(OrganizeError::Invalid("the document's page tree lists no pages: the file is damaged".into())), Some(i) => Err(OrganizeError::NoSuchPage(*i)), None => Ok(()), }