diff --git a/crates/automation/src/comments.rs b/crates/automation/src/comments.rs index 302688f72..4effebf11 100644 --- a/crates/automation/src/comments.rs +++ b/crates/automation/src/comments.rs @@ -173,7 +173,6 @@ impl Automation { let only = a.opt_int("page")?.map(|p| (p.max(1) - 1) as usize); let all = self.comments(a)?; let view = |c: &Annotation| -> Value { - let p = &doc.info.pages[c.page.min(doc.info.pages.len().saturating_sub(1))]; json!({ "id": c.name, "page": c.page + 1, @@ -182,7 +181,8 @@ impl Automation { "author": c.author, "contents": c.contents, "modified": c.modified, - "rect": rect_to_view(p, c.rect), + // Unchanged on a page the viewer could not read (#2660), as `doc_info` reports it. + "rect": crate::view_rect(&doc.info, c.page, c.rect), "color": c.color.map(hex), }) }; diff --git a/crates/automation/src/content.rs b/crates/automation/src/content.rs index faefc5fa7..6d1c86002 100644 --- a/crates/automation/src/content.rs +++ b/crates/automation/src/content.rs @@ -73,9 +73,9 @@ impl Automation { .iter() .map(|it| { let index = per_page.entry(it.page).or_default(); - let ph = doc.info.pages[it.page].height as f64; - let mut v = - json!({ "page": it.page + 1, "index": *index + 1, "rect": to_view(it.content.rect(), ph).map(|x| (x * 100.0).round() / 100.0) }); + // Unchanged on a page the viewer could not read (#2660), like `crate::view_rect`. + let rect = doc.info.pages.get(it.page).map_or(it.content.rect(), |p| to_view(it.content.rect(), p.height as f64)); + let mut v = json!({ "page": it.page + 1, "index": *index + 1, "rect": rect.map(|x| (x * 100.0).round() / 100.0) }); *index += 1; match &it.content { AddedContent::Text(t) => { diff --git a/crates/automation/src/forms.rs b/crates/automation/src/forms.rs index f2eab5ddc..054303d4a 100644 --- a/crates/automation/src/forms.rs +++ b/crates/automation/src/forms.rs @@ -405,7 +405,7 @@ impl Automation { let r: Vec = v.as_array().map(|x| x.iter().filter_map(Value::as_f64).collect()).unwrap_or_default(); let r = <[f64; 4]>::try_from(r).map_err(|_| ToolError::InvalidArgs("rect must be 4 numbers".into()))?; let page = f.widgets.first().and_then(|w| w.page).ok_or_else(|| failed(format!("{name} is not on a page")))?; - let p = &doc.info.pages[page]; + let p = doc.info.pages.get(page).ok_or_else(|| failed(format!("{name} is on page {}, which could not be read", page + 1)))?; let (u0, u1) = (p.view_to_user(r[0] as f32, r[1] as f32), p.view_to_user(r[2] as f32, r[3] as f32)); Some((0, [u0[0].min(u1[0]) as f64, u0[1].min(u1[1]) as f64, u0[0].max(u1[0]) as f64, u0[1].max(u1[1]) as f64])) } diff --git a/crates/automation/src/links.rs b/crates/automation/src/links.rs index cd0fb9da0..8058d5639 100644 --- a/crates/automation/src/links.rs +++ b/crates/automation/src/links.rs @@ -70,9 +70,8 @@ impl Automation { .links() .iter() .map(|l| { - let p = &doc.info.pages[l.page]; - let (v0, v1) = (p.user_to_view(l.rect[0] as f32, l.rect[1] as f32), p.user_to_view(l.rect[2] as f32, l.rect[3] as f32)); - let rect = [v0[0].min(v1[0]), v0[1].min(v1[1]), v0[0].max(v1[0]), v0[1].max(v1[1])].map(|x| (x * 100.0).round() / 100.0); + // The link's page may be one the viewer could not read (#2660): rect unchanged then. + let rect = crate::view_rect(&doc.info, l.page, l.rect.map(|x| x as f32)); let mut v = json!({ "page": l.page + 1, "index": l.index + 1, "rect": rect, "visible": l.style.visible }); // Where an in-document link goes, as the viewer resolves it: named destinations // included, with the position on the page. diff --git a/crates/automation/src/signing.rs b/crates/automation/src/signing.rs index 94fc4bdec..c7a043c89 100644 --- a/crates/automation/src/signing.rs +++ b/crates/automation/src/signing.rs @@ -208,7 +208,7 @@ impl Automation { }; let rect = match a.nums::<4>("rect")? { Some(r) => { - let p = &doc.info.pages[page]; + let p = doc.info.pages.get(page).ok_or_else(|| bad("the document has no page to put the signature on"))?; let (u0, u1) = (p.view_to_user(r[0] as f32, r[1] as f32), p.view_to_user(r[2] as f32, r[3] as f32)); let u = [u0[0].min(u1[0]) as f64, u0[1].min(u1[1]) as f64, u0[0].max(u1[0]) as f64, u0[1].max(u1[1]) as f64]; if u[2] - u[0] < 4.0 || u[3] - u[1] < 4.0 { diff --git a/crates/automation/tests/hostile_args.rs b/crates/automation/tests/hostile_args.rs index 607696899..b7fd0f164 100644 --- a/crates/automation/tests/hostile_args.rs +++ b/crates/automation/tests/hostile_args.rs @@ -336,3 +336,114 @@ fn listing(dir: &Path) -> Vec<(PathBuf, u64)> { out.sort(); out } + +/// A six-page file whose fourth `/Kids` entry points at an object that does not exist (#2660), +/// with a link, a URI link and a comment on every page and a form field on the last. Readers +/// disagree on such a file: the viewer stopped at the dangling entry and saw three pages, while +/// the editing layer sees all six, so annotations come back on pages the viewer does not have. +fn dangling_kid_pdf() -> Vec { + let pages: Vec = (0..6).map(|i| 10 + 2 * i).collect(); + let mut kids: Vec = pages[..3].to_vec(); + kids.push(77); // does not exist + kids.extend(&pages[3..]); + let kids: Vec = kids.iter().map(|k| format!("{k} 0 R")).collect(); + let mut objs: Vec<(u32, String)> = vec![ + (1, "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [50 0 R] >> >>".into()), + (2, format!("<< /Type /Pages /Kids [{}] /Count 6 >>", kids.join(" "))), + (90, "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".into()), + ]; + for (i, &page) in pages.iter().enumerate() { + let text = format!("BT /F1 24 Tf 50 300 Td (Page {}) Tj ET", i + 1); + let field = if i == 5 { " 50 0 R" } else { "" }; + objs.push(( + page, + format!( + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 400 400] /Resources << /Font << /F1 90 0 R >> >> /Contents {} 0 R \ + /Annots [<< /Type /Annot /Subtype /Link /Rect [10 10 100 40] /Dest [10 0 R /Fit] >> \ + << /Type /Annot /Subtype /Link /Rect [10 50 100 80] /A << /S /URI /URI (https://example.com/) >> >> \ + << /Type /Annot /Subtype /Text /Rect [200 200 220 220] /Contents (note) /T (author) >>{field}] >>", + page + 1 + ), + )); + objs.push((page + 1, format!("<< /Length {} >>\nstream\n{text}\nendstream", text.len()))); + } + objs.push((50, "<< /FT /Tx /T (name) /Type /Annot /Subtype /Widget /Rect [50 100 250 130] /P 20 0 R >>".into())); + objs.sort_by_key(|(n, _)| *n); + + let size = 100; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = vec![None; size]; + for (num, body) in &objs { + if let Some(slot) = offsets.get_mut(*num as usize) { + *slot = Some(out.len()); + } + out.extend_from_slice(format!("{num} 0 obj\n{body}\nendobj\n").as_bytes()); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {size}\n0000000000 65535 f \n").as_bytes()); + for offset in offsets.iter().skip(1) { + out.extend_from_slice( + match offset { + Some(o) => format!("{o:010} 00000 n \n"), + None => "0000000000 65535 f \n".to_string(), + } + .as_bytes(), + ); + } + out.extend_from_slice(format!("trailer << /Size {size} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n").as_bytes()); + out +} + +#[test] +fn no_tool_panics_when_the_viewer_sees_fewer_pages_than_the_file_has() { + // #2660: `link_list` indexed the viewer's pages with a page number from the editing layer + // ("index out of bounds: the len is 3 but the index is 3"). Every tool, on a fresh copy of a + // file where the two disagree, must answer with a result or an error. + let root = tmp("dangling-kid"); + let pdf = dangling_kid_pdf(); + let mut failures = Vec::new(); + for def in tools() { + // Written again for every tool: tools that save write to `in.pdf`, the plausible path. + std::fs::write(root.join("in.pdf"), &pdf).expect("fixture"); + let mut a = Automation::new().with_root(&root).expect("root"); + let opened = a.call("doc_open", &json!({ "path": "in.pdf" })).expect("open the damaged file"); + let doc = opened + .into_iter() + .find_map(|c| match c { + pdfcraft_automation::Content::Json(v) => v["doc"].as_u64(), + pdfcraft_automation::Content::Png { .. } => None, + }) + .expect("the opened document has an id"); + if let Err(e) = guarded(&mut a, def.name, &Value::Object(plausible_with(&def, Some(doc)))) { + failures.push(e); + } + match def.name { + // The field sits on the sixth page; a rect is placed on the field's own page. + "form_set_props" => { + if let Err(e) = guarded(&mut a, def.name, &json!({ "doc": doc, "field": "name", "rect": [10, 10, 60, 30] })) { + failures.push(e); + } + } + // Listing still reports what is on the pages the viewer lacks, not just survives it. + "link_list" => { + let listed = catch_unwind(AssertUnwindSafe(|| a.call("link_list", &json!({ "doc": doc })))); + let count = match listed { + Ok(Ok(content)) => content.into_iter().find_map(|c| match c { + pdfcraft_automation::Content::Json(v) => v["count"].as_u64(), + pdfcraft_automation::Content::Png { .. } => None, + }), + _ => None, + }; + if count != Some(12) { + failures.push(format!("link_list listed {count:?} links, not the file's 12")); + } + } + _ => {} + } + } + for f in &failures { + eprintln!(" {f}"); + } + let _ = std::fs::remove_dir_all(&root); + assert!(failures.is_empty(), "{} tool(s) panicked; the first is: {}", failures.len(), failures.first().map_or("", String::as_str)); +}