From 265ddca58c8f123021e324f62b82363c0ab0ca18 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Sat, 10 Oct 2026 21:46:52 +0200 Subject: [PATCH] organize: drop named destinations and their links when a page is deleted (#817) (#851) --- crates/organize/src/lib.rs | 134 ++++++++++++++++++++++++++++++++++- crates/organize/src/tests.rs | 36 ++++++++++ 2 files changed, 169 insertions(+), 1 deletion(-) diff --git a/crates/organize/src/lib.rs b/crates/organize/src/lib.rs index 4d6cacb17e..fc383faf1b 100644 --- a/crates/organize/src/lib.rs +++ b/crates/organize/src/lib.rs @@ -197,6 +197,7 @@ pub fn delete_pages(doc: &mut Document, indices: &[usize]) -> Result<(), Organiz /// Bookmarks, and links on the `keep` pages, that go to one of the `gone` pages lose that /// destination (/Dest, or a GoTo /A), rather than pointing at nothing; so does the catalog's /OpenAction. fn drop_destinations_to(doc: &mut Document, gone: &[ObjRef], keep: &[(ObjRef, Dict)]) -> Result<(), OrganizeError> { + let dead_names = drop_named_destinations(doc, gone)?; let mut holders = outline::items(doc); for (p, _) in keep { let annots = doc.get(*p).as_dict().and_then(|d| d.get(b"Annots").cloned()).map(|a| doc.resolve(&a).as_array().cloned().unwrap_or_default()); @@ -209,7 +210,12 @@ fn drop_destinations_to(doc: &mut Document, gone: &[ObjRef], keep: &[(ObjRef, Di ); } let to_gone = |o: Option<&Object>| { - o.map(|o| doc.resolve(o)).and_then(|o| o.as_array().and_then(|a| a.first()).and_then(Object::as_ref)).is_some_and(|r| gone.contains(&r)) + let Some(o) = o.map(|o| doc.resolve(o)) else { return false }; + match &*o { + Object::Name(n) => dead_names.contains(n), + Object::String(s) => dead_names.contains(&s.bytes), + _ => goes_to(doc, &o, gone), + } }; let mut dead: Vec<(ObjRef, &[u8])> = Vec::new(); for h in holders { @@ -242,6 +248,132 @@ fn drop_destinations_to(doc: &mut Document, gone: &[ObjRef], keep: &[(ObjRef, Di Ok(()) } +/// True when a destination (an array, or a dictionary whose /D is one) opens one of the `gone` pages. +fn goes_to(doc: &Document, dest: &Object, gone: &[ObjRef]) -> bool { + let dest = doc.resolve(dest); + let array = match dest.as_dict() { + Some(d) => d.get(b"D").map(|d| doc.resolve(d)), + None => Some(dest.clone()), + }; + array.and_then(|a| a.as_array().and_then(|a| a.first()).and_then(Object::as_ref)).is_some_and(|r| gone.contains(&r)) +} + +/// Remove the named destinations (the catalog's /Names /Dests tree and the legacy /Dests +/// dictionary) that open one of the `gone` pages. Returns the removed names, so that the links +/// using them can lose their destination too. Names of surviving pages are left alone. +fn drop_named_destinations(doc: &mut Document, gone: &[ObjRef]) -> Result>, OrganizeError> { + let mut dead: Vec> = Vec::new(); + let Some(root) = doc.root() else { return Ok(dead) }; + let Some(catalog) = doc.get(root).as_dict().cloned() else { return Ok(dead) }; + match catalog.get(b"Dests").cloned() { + Some(Object::Ref(r)) => { + if let Some(mut d) = doc.get(r).as_dict().cloned() + && prune_legacy(doc, &mut d, gone, &mut dead) + { + doc.update_dict(r, |x| *x = d)?; + } + } + Some(Object::Dict(mut d)) => { + if prune_legacy(doc, &mut d, gone, &mut dead) { + doc.update_dict(root, |c| c.set(b"Dests".to_vec(), Object::Dict(d)))?; + } + } + _ => {} + } + match catalog.get(b"Names").cloned() { + Some(Object::Ref(n)) => { + let tree = doc.get(n).as_dict().and_then(|d| d.get(b"Dests").cloned()); + if let Some(new) = tree.and_then(|t| prune_tree_slot(doc, &t, gone, &mut dead).transpose()).transpose()? { + doc.update_dict(n, |d| d.set(b"Dests".to_vec(), Object::Dict(new)))?; + } + } + Some(Object::Dict(mut names)) => { + let tree = names.get(b"Dests").cloned(); + if let Some(new) = tree.and_then(|t| prune_tree_slot(doc, &t, gone, &mut dead).transpose()).transpose()? { + names.set(b"Dests".to_vec(), Object::Dict(new)); + doc.update_dict(root, |c| c.set(b"Names".to_vec(), Object::Dict(names)))?; + } + } + _ => {} + } + Ok(dead) +} + +/// Drop the entries of a legacy /Dests dictionary that go to a `gone` page. True when any went. +fn prune_legacy(doc: &Document, dests: &mut Dict, gone: &[ObjRef], dead: &mut Vec>) -> bool { + let keys: Vec> = dests.iter().filter(|(_, v)| goes_to(doc, v, gone)).map(|(k, _)| k.clone()).collect(); + for k in &keys { + dests.remove(k); + } + dead.extend(keys.iter().cloned()); + !keys.is_empty() +} + +/// Prune a name tree held in `slot` (a reference, or a dictionary written inline). A reference is +/// updated in place and yields `None`; an inline dictionary that changed is handed back. +fn prune_tree_slot(doc: &mut Document, slot: &Object, gone: &[ObjRef], dead: &mut Vec>) -> Result, OrganizeError> { + let mut seen = Vec::new(); + match slot { + Object::Ref(r) => { + prune_tree_ref(doc, *r, gone, dead, &mut seen)?; + Ok(None) + } + Object::Dict(d) => { + let mut node = d.clone(); + Ok(prune_tree_node(doc, &mut node, gone, dead, &mut seen)?.then_some(node)) + } + _ => Ok(None), + } +} + +fn prune_tree_ref(doc: &mut Document, r: ObjRef, gone: &[ObjRef], dead: &mut Vec>, seen: &mut Vec) -> Result<(), OrganizeError> { + if seen.contains(&r) { + return Ok(()); + } + seen.push(r); + let Some(mut node) = doc.get(r).as_dict().cloned() else { return Ok(()) }; + if prune_tree_node(doc, &mut node, gone, dead, seen)? { + doc.update_dict(r, |d| *d = node)?; + } + Ok(()) +} + +/// Drop the /Names pairs of one name-tree node that go to a `gone` page, then do the same for its +/// /Kids. True when `node` itself changed (kids held by reference are written back here). +fn prune_tree_node( + doc: &mut Document, + node: &mut Dict, + gone: &[ObjRef], + dead: &mut Vec>, + seen: &mut Vec, +) -> Result { + let mut changed = false; + if let Some(pairs) = node.get(b"Names").map(|n| doc.resolve(n)).and_then(|n| n.as_array().cloned()) { + let mut kept = Vec::with_capacity(pairs.len()); + for pair in pairs.chunks(2) { + match pair { + [key, value] if goes_to(doc, value, gone) => { + if let Some(s) = doc.resolve(key).as_string() { + dead.push(s.bytes.clone()); + } + } + _ => kept.extend_from_slice(pair), + } + } + if kept.len() != pairs.len() { + node.set(b"Names".to_vec(), Object::Array(kept)); + changed = true; + } + } + let kids = node.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()).unwrap_or_default(); + for kid in kids { + if let Some(r) = kid.as_ref() { + prune_tree_ref(doc, r, gone, dead, seen)?; + } + } + Ok(changed) +} + /// Take widgets out of the form: out of their field's /Kids, or out of /AcroForm /Fields when /// they are fields themselves. A field left without kids goes too. fn drop_widgets(doc: &mut Document, mut widgets: Vec) -> Result<(), OrganizeError> { diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index baa0709984..6f139d8254 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -278,6 +278,42 @@ fn full_save_drops_unreachable_objects() { assert_eq!(hayro_syntax::Pdf::new(bytes).unwrap().pages().len(), 1); } +#[test] +fn deleting_a_page_drops_the_named_destinations_to_it() { + // Four names (two in the /Names tree, two in the legacy /Dests dictionary) go to P1 or P3; + // P2 has a link for each. Deleting P1 must drop its names and their links, keep P3's. + let b: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R /Names << /Dests 6 0 R >> /Dests 7 0 R >>".into(), // 1 + "<< /Type /Pages /Kids [3 0 R 4 0 R 5 0 R] /Count 3 /MediaBox [0 0 300 400] >>".into(), // 2 + "<< /Type /Page /Parent 2 0 R >>".into(), // 3 + "<< /Type /Page /Parent 2 0 R /Annots [8 0 R 9 0 R 10 0 R 11 0 R] >>".into(), // 4 + "<< /Type /Page /Parent 2 0 R >>".into(), // 5 + "<< /Names [(treeFirst) [3 0 R /Fit] (treeLast) [5 0 R /Fit]] >>".into(), // 6 + "<< /legacyFirst [3 0 R /Fit] /legacyLast << /D [5 0 R /Fit] >> >>".into(), // 7 + "<< /Type /Annot /Subtype /Link /Rect [0 0 9 9] /Dest (treeFirst) >>".into(), // 8 + "<< /Type /Annot /Subtype /Link /Rect [0 0 9 9] /Dest (treeLast) >>".into(), // 9 + "<< /Type /Annot /Subtype /Link /Rect [0 0 9 9] /Dest /legacyFirst >>".into(), // 10 + "<< /Type /Annot /Subtype /Link /Rect [0 0 9 9] /A << /S /GoTo /D /legacyLast >> >>".into(), // 11 + ]; + let mut doc = open(build(&b, "/Root 1 0 R")); + delete_pages(&mut doc, &[0]).unwrap(); + let out = full_roundtrip(&doc); + let cat = out.get(out.root().unwrap()).as_dict().cloned().unwrap(); + let tree = out.resolve(out.resolve(cat.get(b"Names").unwrap()).as_dict().unwrap().get(b"Dests").unwrap()); + let pairs = tree.as_dict().unwrap().get(b"Names").and_then(Object::as_array).cloned().unwrap(); + assert_eq!(pairs.len(), 2, "{pairs:?}"); + assert_eq!(pairs[0].as_string().map(|s| s.bytes.clone()), Some(b"treeLast".to_vec())); + let page3 = pages(&out).unwrap()[1].obj; + assert_eq!(out.resolve(&pairs[1]).as_array().and_then(|a| a.first()).and_then(Object::as_ref), Some(page3)); + let legacy = out.resolve(cat.get(b"Dests").unwrap()).as_dict().cloned().unwrap(); + assert!(!legacy.contains(b"legacyFirst") && legacy.contains(b"legacyLast"), "{legacy:?}"); + let links = annots(&out, 0); + assert!(!links[0].contains(b"Dest"), "{:?}", links[0]); + assert!(links[1].contains(b"Dest"), "{:?}", links[1]); + assert!(!links[2].contains(b"Dest"), "{:?}", links[2]); + assert!(links[3].contains(b"A"), "{:?}", links[3]); +} + // ── Combine / extract / split ────────────────────────────────────────────────────────────────── /// Build a classic-xref PDF from object bodies (object i+1 = bodies[i]).