fix(automation): tools on a file the viewer reads fewer pages of answer instead of panicking

`link_list` and `form_set_props` indexed the viewer's pages with a page number from the
editing layer. On a file whose page tree has a dangling `/Kids` entry (#2660) the viewer
sees three pages and the editing layer six, so `link_list` panicked ("the len is 3 but
the index is 3") and so did moving a field on one of the later pages.

Rectangles on a page the viewer could not read are now reported unchanged, as `doc_info`
already reports them (`link_list`, `comment_list`, `content_list`); `comment_list` no
longer borrows the last page's geometry for them. Placing a rect on such a page is an
error (`form_set_props`, and `sign` when the viewer has no page at all).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Christoph Ortenburg
2026-10-15 20:32:20 -04:00
parent 459379ef9b
commit ca8ebb0bb7
6 changed files with 120 additions and 10 deletions
+2 -2
View File
@@ -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),
})
};
+3 -3
View File
@@ -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) => {
+1 -1
View File
@@ -405,7 +405,7 @@ impl Automation {
let r: Vec<f64> = 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]))
}
+2 -3
View File
@@ -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.
+1 -1
View File
@@ -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 {
+111
View File
@@ -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<u8> {
let pages: Vec<u32> = (0..6).map(|i| 10 + 2 * i).collect();
let mut kids: Vec<u32> = pages[..3].to_vec();
kids.push(77); // does not exist
kids.extend(&pages[3..]);
let kids: Vec<String> = 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));
}