From ec0ff113fdf79aadf8fd77d7aedf9d6abef08da9 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 10 Nov 2026 22:33:58 +0000 Subject: [PATCH] address review: list a direct child's names once when its shared kids array is re-read --- crates/organize/src/import.rs | 23 ++++++++++++++++++----- crates/organize/src/tests.rs | 31 +++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/crates/organize/src/import.rs b/crates/organize/src/import.rs index 41dbe72c2e..e193496c23 100644 --- a/crates/organize/src/import.rs +++ b/crates/organize/src/import.rs @@ -444,9 +444,13 @@ pub(crate) const MAX_NAME_TREE_KEY_BYTES: usize = 64 << 20; /// named by many entries is copied once per entry). pub(crate) fn name_tree_entries(src: &Document, node: &Object, depth: u8, seen: &mut Seen, out: &mut Vec<(Vec, Object)>) { let mut key_bytes = out.iter().map(|(k, _)| k.len()).fold(0usize, usize::saturating_add); - name_tree_entries_within(src, node, depth, seen, out, &mut key_bytes, &mut std::collections::HashSet::new()); + name_tree_entries_within(src, node, depth, seen, out, &mut key_bytes, &mut std::collections::HashSet::new(), (None, Vec::new())); } +/// Where a name-tree node sits: the indirect object it is (or is under), then its path of `/Kids` indices from there. +type NodeId = (Option, Vec); + +#[allow(clippy::too_many_arguments)] fn name_tree_entries_within( src: &Document, node: &Object, @@ -454,7 +458,8 @@ fn name_tree_entries_within( seen: &mut Seen, out: &mut Vec<(Vec, Object)>, key_bytes: &mut usize, - emitted: &mut std::collections::HashSet, + emitted: &mut std::collections::HashSet, + place: NodeId, ) { if depth > 32 || node.as_ref().is_some_and(|r| !first_at(seen, r, depth)) { return; @@ -464,7 +469,10 @@ fn name_tree_entries_within( // A node read again from a shallower path (where `first_at` lets it through) has already // listed its entries: listing them again would copy each name and value twice, and the // shallower revisit is only there to reach the descendants the depth limit cut. - let names_once = node.as_ref().is_none_or(|r| emitted.insert(r)); + // An indirect node is identified by its reference; a direct one by where it sits under the + // nearest indirect object above it, which a re-read of that object repeats. + let place = node.as_ref().map_or(place, |r| (Some(r), Vec::new())); + let names_once = emitted.insert(place.clone()); let node = src.resolve(node); let Some(d) = node.as_dict() else { return }; let names = d.get(b"Names").map(|n| src.resolve(n)); @@ -482,8 +490,13 @@ fn name_tree_entries_within( } } let kids = name_tree_kids(src, d, depth, seen); - for k in kids.as_deref().and_then(Object::as_array).into_iter().flatten() { - name_tree_entries_within(src, k, depth + 1, seen, out, key_bytes, emitted); + let kids_ref = d.get(b"Kids").and_then(Object::as_ref); + for (i, k) in kids.as_deref().and_then(Object::as_array).into_iter().flatten().enumerate() { + let at = match kids_ref { + Some(r) => (Some(r), vec![i]), + None => (place.0, [place.1.as_slice(), &[i]].concat()), + }; + name_tree_entries_within(src, k, depth + 1, seen, out, key_bytes, emitted, at); } } diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index f6dbe594f8..4e798b6668 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -3871,3 +3871,34 @@ fn a_shared_name_tree_kids_array_first_met_deep_is_read_again_shallower() { crate::import::name_tree_entries(&doc, &Object::Dict(root), 0, &mut crate::import::Seen::new(), &mut out); assert_eq!(out, vec![(b"deep".to_vec(), Object::Int(7))]); } + +/// A direct child of a shared indirect `/Kids` array, read at depth 32 (its own `/Kids` cut), lists +/// its `/Names` once when a shallower node reads the array again, and that revisit still reaches +/// the `/Kids` the limit cut. +#[test] +fn a_shared_kids_array_lists_a_direct_childs_names_once() { + let key = |k: &[u8]| Object::String(pdfcraft_cos::PdfString::literal(k.to_vec())); + let mut doc = Document::new_empty(); + let mut below = Dict::new(); + below.set(b"Names".to_vec(), Object::Array(vec![key(b"below"), Object::Int(2)])); + let mut child = Dict::new(); + child.set(b"Names".to_vec(), Object::Array(vec![key(b"child"), Object::Int(1)])); + child.set(b"Kids".to_vec(), Object::Array(vec![Object::Dict(below)])); + let shared = doc.add(Object::Array(vec![Object::Dict(child)])); + let mut node = Dict::new(); + node.set(b"Kids".to_vec(), Object::Ref(shared)); + // The innermost node sits at depth 31: the array's child at depth 32 is read, its kids are cut. + for _ in 0..30 { + let mut up = Dict::new(); + up.set(b"Kids".to_vec(), Object::Array(vec![Object::Dict(node)])); + node = up; + } + let mut near = Dict::new(); + near.set(b"Kids".to_vec(), Object::Ref(shared)); + let mut root = Dict::new(); + root.set(b"Kids".to_vec(), Object::Array(vec![Object::Dict(node), Object::Dict(near)])); + let mut out = Vec::new(); + crate::import::name_tree_entries(&doc, &Object::Dict(root), 0, &mut crate::import::Seen::new(), &mut out); + out.sort_by(|a, b| a.0.cmp(&b.0)); + assert_eq!(out, vec![(b"below".to_vec(), Object::Int(2)), (b"child".to_vec(), Object::Int(1))]); +}