From 15f92032436fea0c1cd804e440e7ad00b07e7392 Mon Sep 17 00:00:00 2001 From: oracola91 <118899664+oracola91@users.noreply.github.com> Date: Tue, 20 Oct 2026 10:30:20 +0300 Subject: [PATCH 1/2] Page-tree walks in annot, forms and organize skip repeated children annot::page_refs, the forms page_refs and organize's walk queued every /Kids entry before the seen check, and organize cloned the inherited attributes, Resources included, for each queued entry. A page tree whose /Kids lists the same child many times, or whose nodes share one /Kids array, held all of those entries at once before they were dropped as already seen. Each walk is now a small iterator with one cursor per open /Kids array, the way pdfcraft_model::pages has walked since #1926. organize carries inherited attributes as the nodes that set them and copies them only for the pages it returns. Page order, the pages found, the inherited values and the million-node cap are unchanged. --- crates/annot/src/lib.rs | 61 ++++++++++++++++++------- crates/annot/src/tests.rs | 17 +++++++ crates/forms/src/lib.rs | 61 ++++++++++++++++++------- crates/forms/src/tests.rs | 16 +++++++ crates/organize/src/lib.rs | 88 ++++++++++++++++++++++++++---------- crates/organize/src/tests.rs | 23 ++++++++++ 6 files changed, 209 insertions(+), 57 deletions(-) diff --git a/crates/annot/src/lib.rs b/crates/annot/src/lib.rs index 587cd4466..0edcb0871 100644 --- a/crates/annot/src/lib.rs +++ b/crates/annot/src/lib.rs @@ -651,25 +651,54 @@ impl ReviewState { pub fn page_refs(doc: &Document) -> Result, AnnotError> { let root = doc.root().ok_or(AnnotError::NoPageTree)?; let pages = doc.get(root).as_dict().and_then(|d| d.reference(b"Pages")).ok_or(AnnotError::NoPageTree)?; - let mut out = Vec::new(); - let mut seen = std::collections::HashSet::new(); - let mut stack = vec![pages]; - while let Some(node) = stack.pop() { - if !seen.insert(node) || seen.len() > 1_000_000 { - continue; - } - let obj = doc.get(node); - let Some(d) = obj.as_dict() else { continue }; - match d.get(b"Kids").map(|k| doc.resolve(k)) { - Some(kids) if d.name(b"Type") != Some(b"Page") => { - if let Some(a) = kids.as_array() { - stack.extend(a.iter().rev().filter_map(|k| k.as_ref())); - } + Ok(PageWalk::new(doc, pages).collect()) +} + +/// Depth-first walk over the leaf pages under `top`. It keeps one cursor per open `/Kids` array, +/// never one entry per child, so a child listed many times or shared between nodes is skipped by +/// the seen check without being queued first (`pdfcraft_model::pages` walks the same way). +/// Cycles end at the seen check and at most a million nodes are visited. +struct PageWalk<'a> { + doc: &'a Document, + seen: std::collections::HashSet, + pending: Vec<(std::sync::Arc, usize)>, +} + +impl<'a> PageWalk<'a> { + fn new(doc: &'a Document, top: ObjRef) -> Self { + let top = std::sync::Arc::new(Object::Array(vec![Object::Ref(top)])); + Self { doc, seen: Default::default(), pending: vec![(top, 0)] } + } +} + +impl Iterator for PageWalk<'_> { + type Item = ObjRef; + + fn next(&mut self) -> Option { + while let Some((kids, next)) = self.pending.last_mut() { + let Some(kid) = kids.as_array().and_then(|a| a.get(*next)) else { + self.pending.pop(); + continue; + }; + let node = kid.as_ref(); + *next += 1; + let Some(node) = node else { continue }; + if self.seen.len() >= 1_000_000 { + self.pending.clear(); + return None; + } + if !self.seen.insert(node) { + continue; + } + let obj = self.doc.get(node); + let Some(d) = obj.as_dict() else { continue }; + match d.get(b"Kids").map(|k| self.doc.resolve(k)) { + Some(kids) if d.name(b"Type") != Some(b"Page") => self.pending.push((kids, 0)), + _ => return Some(node), } - _ => out.push(node), } + None } - Ok(out) } fn page_ref(doc: &Document, page: usize) -> Result { diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index 4a4dfb106..7d5399276 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -1382,3 +1382,20 @@ fn free_text_properties_show_legacy_or_absent_border_width() { .unwrap(); assert_eq!(props(&doc, 0, index).unwrap().width, Some(4.0)); } + +#[test] +fn page_walk_skips_repeated_and_cyclic_kids_without_queueing_them() { + let mut doc = fixture(); + let root = ObjRef { num: 2, generation: 0 }; + let (first, second) = (ObjRef { num: 3, generation: 0 }, ObjRef { num: 4, generation: 0 }); + // A cycle back to the root, then both pages 128 times over. + let mut kids = vec![Object::Ref(root)]; + kids.extend([Object::Ref(first), Object::Ref(second)].into_iter().cycle().take(256)); + doc.update_dict(root, |d| d.set(b"Kids".to_vec(), Object::Array(kids))).unwrap(); + let mut walk = PageWalk::new(&doc, root); + assert_eq!(walk.next(), Some(first)); + assert!(walk.pending.len() <= 2, "repeated kids retained {} pending entries", walk.pending.len()); + assert_eq!(walk.next(), Some(second)); + assert_eq!(walk.next(), None); + assert_eq!(page_refs(&doc).unwrap(), [first, second]); +} diff --git a/crates/forms/src/lib.rs b/crates/forms/src/lib.rs index 1dfa76726..629489898 100644 --- a/crates/forms/src/lib.rs +++ b/crates/forms/src/lib.rs @@ -452,25 +452,54 @@ fn nums(doc: &Document, o: Option<&Object>) -> Option> { pub(crate) fn page_refs(doc: &Document) -> Vec { let Some(root) = doc.root() else { return Vec::new() }; let Some(pages) = doc.get(root).as_dict().and_then(|d| d.reference(b"Pages")) else { return Vec::new() }; - let mut out = Vec::new(); - let mut seen = std::collections::HashSet::new(); - let mut stack = vec![pages]; - while let Some(node) = stack.pop() { - if !seen.insert(node) || seen.len() > 1_000_000 { - continue; - } - let obj = doc.get(node); - let Some(d) = obj.as_dict() else { continue }; - match d.get(b"Kids").map(|k| doc.resolve(k)) { - Some(kids) if d.name(b"Type") != Some(b"Page") => { - if let Some(a) = kids.as_array() { - stack.extend(a.iter().rev().filter_map(|k| k.as_ref())); - } + PageWalk::new(doc, pages).collect() +} + +/// Depth-first walk over the leaf pages under `top`. It keeps one cursor per open `/Kids` array, +/// never one entry per child, so a child listed many times or shared between nodes is skipped by +/// the seen check without being queued first (`pdfcraft_model::pages` walks the same way). +/// Cycles end at the seen check and at most a million nodes are visited. +struct PageWalk<'a> { + doc: &'a Document, + seen: std::collections::HashSet, + pending: Vec<(std::sync::Arc, usize)>, +} + +impl<'a> PageWalk<'a> { + fn new(doc: &'a Document, top: ObjRef) -> Self { + let top = std::sync::Arc::new(Object::Array(vec![Object::Ref(top)])); + Self { doc, seen: Default::default(), pending: vec![(top, 0)] } + } +} + +impl Iterator for PageWalk<'_> { + type Item = ObjRef; + + fn next(&mut self) -> Option { + while let Some((kids, next)) = self.pending.last_mut() { + let Some(kid) = kids.as_array().and_then(|a| a.get(*next)) else { + self.pending.pop(); + continue; + }; + let node = kid.as_ref(); + *next += 1; + let Some(node) = node else { continue }; + if self.seen.len() >= 1_000_000 { + self.pending.clear(); + return None; + } + if !self.seen.insert(node) { + continue; + } + let obj = self.doc.get(node); + let Some(d) = obj.as_dict() else { continue }; + match d.get(b"Kids").map(|k| self.doc.resolve(k)) { + Some(kids) if d.name(b"Type") != Some(b"Page") => self.pending.push((kids, 0)), + _ => return Some(node), } - _ => out.push(node), } + None } - out } /// The AcroForm dictionary. diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index fef5bcfbd..968bfb201 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -2277,3 +2277,19 @@ fn document_scripts_read_each_name_tree_node_once() { ]); assert_eq!(document_scripts_named(&doc), [("init".to_string(), "var ready = true;".to_string())]); } + +#[test] +fn page_walk_skips_repeated_and_cyclic_kids_without_queueing_them() { + let mut doc = fixture(); + let root = ObjRef { num: 2, generation: 0 }; + let page = ObjRef { num: 3, generation: 0 }; + // A cycle back to the root, then the page 128 times over. + let mut kids = vec![Object::Ref(root)]; + kids.extend(std::iter::repeat_n(Object::Ref(page), 128)); + doc.update_dict(root, |d| d.set(b"Kids".to_vec(), Object::Array(kids))).unwrap(); + let mut walk = PageWalk::new(&doc, root); + assert_eq!(walk.next(), Some(page)); + assert!(walk.pending.len() <= 2, "repeated kids retained {} pending entries", walk.pending.len()); + assert_eq!(walk.next(), None); + assert_eq!(page_refs(&doc), [page]); +} diff --git a/crates/organize/src/lib.rs b/crates/organize/src/lib.rs index 99c228c60..fd1e4ecff 100644 --- a/crates/organize/src/lib.rs +++ b/crates/organize/src/lib.rs @@ -72,36 +72,74 @@ fn pages_root(doc: &Document) -> Result { /// All leaf pages in order, with their effective inherited attributes. fn walk(doc: &Document) -> Result, OrganizeError> { - let root = pages_root(doc)?; - let mut out = Vec::new(); - let mut seen = std::collections::HashSet::new(); - // (node, inherited attributes from ancestors) - let mut stack: Vec<(ObjRef, Dict)> = vec![(root, Dict::new())]; - while let Some((node, inherited)) = stack.pop() { - if !seen.insert(node) || seen.len() > 1_000_000 { - continue; // cycles in broken trees - } - let obj = doc.get(node); - let Some(d) = obj.as_dict() else { continue }; - let mut attrs = inherited.clone(); - for k in INHERITABLE { - if let Some(v) = d.get(k) { - attrs.set(k.to_vec(), v.clone()); + Ok(PageWalk::new(doc, pages_root(doc)?).collect()) +} + +/// For each of `INHERITABLE`, the nearest node that sets it, shared rather than copied. +type Owners = [Option>; 4]; + +/// Depth-first walk over the leaf pages under `top`. It keeps one cursor per open `/Kids` array, +/// never one entry per child, and carries inherited attributes as the nodes that set them, so a +/// child listed many times or shared between nodes is skipped by the seen check before anything +/// is queued or copied (`pdfcraft_model::pages` walks the same way). Cycles in broken trees end +/// at the seen check and at most a million nodes are visited. +struct PageWalk<'a> { + doc: &'a Document, + seen: std::collections::HashSet, + pending: Vec<(std::sync::Arc, usize, Owners)>, +} + +impl<'a> PageWalk<'a> { + fn new(doc: &'a Document, top: ObjRef) -> Self { + let top = std::sync::Arc::new(Object::Array(vec![Object::Ref(top)])); + Self { doc, seen: Default::default(), pending: vec![(top, 0, Default::default())] } + } +} + +impl Iterator for PageWalk<'_> { + type Item = (ObjRef, Dict); + + fn next(&mut self) -> Option<(ObjRef, Dict)> { + while let Some((kids, next, inherited)) = self.pending.last_mut() { + let Some(kid) = kids.as_array().and_then(|a| a.get(*next)) else { + self.pending.pop(); + continue; + }; + let node = kid.as_ref(); + *next += 1; + let Some(node) = node else { continue }; + if self.seen.len() >= 1_000_000 { + self.pending.clear(); + return None; } - } - let is_pages = d.name(b"Type") == Some(b"Pages") || (d.contains(b"Kids") && d.name(b"Type") != Some(b"Page")); - if is_pages { - let kids = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()).unwrap_or_default(); - for k in kids.iter().rev() { - if let Some(r) = k.as_ref() { - stack.push((r, attrs.clone())); + if !self.seen.insert(node) { + continue; + } + let obj = self.doc.get(node); + let Some(d) = obj.as_dict() else { continue }; + let mut owners = inherited.clone(); + for (k, owner) in INHERITABLE.iter().zip(&mut owners) { + if d.contains(k) { + *owner = Some(obj.clone()); } } - } else { - out.push((node, attrs)); + let is_pages = d.name(b"Type") == Some(b"Pages") || (d.contains(b"Kids") && d.name(b"Type") != Some(b"Page")); + if is_pages { + if let Some(kids) = d.get(b"Kids") { + self.pending.push((self.doc.resolve(kids), 0, owners)); + } + } else { + let mut attrs = Dict::new(); + for (k, owner) in INHERITABLE.iter().zip(&owners) { + if let Some(v) = owner.as_ref().and_then(|o| o.as_dict()).and_then(|o| o.get(k)) { + attrs.set(k.to_vec(), v.clone()); + } + } + return Some((node, attrs)); + } } + None } - Ok(out) } /// Leaf pages in document order. diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index 9bd3eb396..4f681367d 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -2821,3 +2821,26 @@ fn rotate_repeated_indices_accumulate_from_the_current_page_value() { } } } + +#[test] +fn page_walk_skips_repeated_and_cyclic_kids_without_copying_attributes() { + let mut doc = open(fixture()); + let (root, branch) = (ObjRef { num: 2, generation: 0 }, ObjRef { num: 3, generation: 0 }); + let (first, second) = (ObjRef { num: 4, generation: 0 }, ObjRef { num: 5, generation: 0 }); + // The branch node carries Resources for its pages: a cycle back to it, then both pages + // 128 times over. + let mut kids = vec![Object::Ref(branch)]; + kids.extend([Object::Ref(first), Object::Ref(second)].into_iter().cycle().take(256)); + doc.update_dict(branch, |d| d.set(b"Kids".to_vec(), Object::Array(kids))).unwrap(); + let mut walk = PageWalk::new(&doc, root); + let (page, attrs) = walk.next().unwrap(); + assert_eq!(page, first); + assert!(walk.pending.len() <= 3, "repeated kids retained {} pending entries", walk.pending.len()); + assert!(attrs.get(b"Resources").and_then(|r| r.as_dict()).is_some_and(|r| r.contains(b"Font")), "Resources from the branch"); + assert_eq!(attrs.get(b"Rotate").and_then(Object::as_int), Some(90), "Rotate from the root"); + assert_eq!(walk.next().map(|(p, _)| p), Some(second)); + let (_, last) = walk.next().unwrap(); + assert_eq!(last.get(b"MediaBox").and_then(Object::as_array).map(Vec::len), Some(4), "own MediaBox kept"); + assert!(walk.next().is_none()); + assert_eq!(labels(&doc), ["Page 1", "Page 2", "Page 3"]); +} From 586246d69aea65814bbdd83d43fb6dc61f5d992e Mon Sep 17 00:00:00 2001 From: oracola91 <118899664+oracola91@users.noreply.github.com> Date: Tue, 20 Oct 2026 11:23:29 +0300 Subject: [PATCH 2/2] organize: check the page's own MediaBox values in the walk test The root's MediaBox and the page's own both have four numbers, so the length check passed even if the page lost its own value. Compare the values instead. --- crates/organize/src/tests.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index 4f681367d..224863af1 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -2840,7 +2840,8 @@ fn page_walk_skips_repeated_and_cyclic_kids_without_copying_attributes() { assert_eq!(attrs.get(b"Rotate").and_then(Object::as_int), Some(90), "Rotate from the root"); assert_eq!(walk.next().map(|(p, _)| p), Some(second)); let (_, last) = walk.next().unwrap(); - assert_eq!(last.get(b"MediaBox").and_then(Object::as_array).map(Vec::len), Some(4), "own MediaBox kept"); + let media_box = last.get(b"MediaBox").and_then(Object::as_array).map(|m| m.iter().filter_map(Object::as_int).collect::>()); + assert_eq!(media_box, Some(vec![0, 0, 500, 500]), "own MediaBox kept over the root's"); assert!(walk.next().is_none()); assert_eq!(labels(&doc), ["Page 1", "Page 2", "Page 3"]); }