mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-10-28 03:58:45 -06:00
M1: edit finds the pages a damaged file's readers find (#1664)
* M1: edit finds the pages a damaged file's readers find (fixes #1656) * M1: rebuild a lost page tree only when the reader shows the same pages, and refuse edits to signed rebuilt files (#1656) --------- Co-authored-by: Brandon Thomas <echelon@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Marco Arnulfo
co-authored by
Brandon Thomas
Claude Opus 5.5
Brandon Thomas
Claude Opus 5.5
parent
9479561ffa
commit
cccd55fab2
@@ -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<u8> {
|
||||
let stream = |content: &str| format!("<< /Length {} >>\nstream\n{content}\nendstream", content.len()).into_bytes();
|
||||
let objs: Vec<(usize, Vec<u8>)> = 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::<Vec<_>>(), [(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<u8> {
|
||||
let stream = |content: &str| format!("<< /Length {} >>\nstream\n{content}\nendstream", content.len()).into_bytes();
|
||||
let objs: Vec<Vec<u8>> = 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() {
|
||||
|
||||
+34
-11
@@ -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(
|
||||
|
||||
@@ -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<Vec<u8>>) -> Arc<Vec<u8>> {
|
||||
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<String> = 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
|
||||
|
||||
@@ -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<u8> {
|
||||
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::<Vec<_>>(), [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.
|
||||
|
||||
@@ -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<usize, OrganizeError> {
|
||||
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(()),
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user