diff --git a/crates/render/src/inspect.rs b/crates/render/src/inspect.rs index 438932f99..1343c2b51 100644 --- a/crates/render/src/inspect.rs +++ b/crates/render/src/inspect.rs @@ -466,6 +466,7 @@ fn inspect_structure(info: &mut DocInfo, fill: impl FnOnce(&mut DocInfo) -> Resu struct Inspector<'a> { doc: &'a dyn Structure, + page_ids: Vec, page_index: HashMap, /// Named destinations (`/Names /Dests` tree), keyed by raw string bytes, built once. /// Looking each name up by walking the tree was O(links × names): minutes on manuals. @@ -474,8 +475,9 @@ struct Inspector<'a> { impl<'a> Inspector<'a> { fn new(doc: &'a dyn Structure) -> Self { - let page_index = doc.get_pages().into_iter().map(|(n, id)| (id, n as usize - 1)).collect(); - let mut me = Self { doc, page_index, named: HashMap::new() }; + let page_ids = doc.page_ids(); + let page_index = page_ids.iter().copied().enumerate().map(|(index, id)| (id, index)).collect(); + let mut me = Self { doc, page_ids, page_index, named: HashMap::new() }; let mut named = HashMap::new(); if let Some(tree) = doc.catalog().ok().and_then(|c| c.get(b"Names").ok()).and_then(|o| me.dict(o)).and_then(|n| n.get(b"Dests").ok()).and_then(|o| me.dict(o)) @@ -523,9 +525,7 @@ impl<'a> Inspector<'a> { } // Widgets no `/Fields` entry reaches are fields too (Acrobat and the browsers fill them); // each is adopted through its topmost unlisted `/Parent`, so a split field stays one. - let mut pages: Vec<_> = self.page_index.iter().collect(); - pages.sort_by_key(|(_, i)| **i); - for (&pid, _) in pages { + for &pid in &self.page_ids { let Ok(page_dict) = self.doc.get_dictionary(pid) else { continue }; let Ok(Object::Array(annots)) = page_dict.get(b"Annots").map(|o| self.resolve(o)) else { continue }; for a in annots { @@ -1185,9 +1185,7 @@ impl<'a> Inspector<'a> { fn fonts(&self, out: &mut Vec) { let mut seen = HashSet::new(); - let mut pages: Vec<_> = self.page_index.iter().collect(); - pages.sort_by_key(|(_, i)| **i); - for (&pid, _) in pages { + for &pid in &self.page_ids { let Ok(fonts) = self.doc.get_page_fonts(pid) else { continue }; for (_, f) in fonts { let base = f diff --git a/crates/render/src/structure.rs b/crates/render/src/structure.rs index 018de5a6e..f6a0ad5d4 100644 --- a/crates/render/src/structure.rs +++ b/crates/render/src/structure.rs @@ -37,8 +37,8 @@ pub(crate) trait Structure { self.trailer().get(b"Root").and_then(Object::as_reference).and_then(|id| self.get_dictionary(id)) } - fn get_pages(&self) -> BTreeMap { - let mut pages = BTreeMap::new(); + fn page_ids(&self) -> Vec { + let mut pages = Vec::new(); let Ok(root) = self.catalog().and_then(|c| c.get(b"Pages")).and_then(Object::as_reference) else { return pages }; let mut pending = vec![(root, 0usize)]; let mut seen = HashSet::new(); @@ -49,9 +49,11 @@ pub(crate) trait Structure { let Ok(dict) = self.get_dictionary(id) else { continue }; match dict.get_type() { Ok(b"Page") => { - // Page numbers are 1-based u32s; a tree past u32::MAX pages stops there. - let Some(number) = u32::try_from(pages.len()).ok().and_then(|n| n.checked_add(1)) else { return pages }; - pages.insert(number, id); + // Keep the effective u32 page-count bound without trusting /Count. + if u32::try_from(pages.len()).ok().and_then(|n| n.checked_add(1)).is_none() { + return pages; + } + pages.push(id); } Ok(b"Pages") => { if let Ok(kids) = dict.get(b"Kids").and_then(|o| self.dereference(o)).and_then(|(_, o)| o.as_array()) { @@ -115,7 +117,7 @@ impl Structure for lopdf::Document { fn encrypted(&self) -> bool { self.is_encrypted() || self.was_encrypted() || self.trailer.has(b"Encrypt") } - // `get_pages` is not overridden: lopdf's own page-tree walk trusts /Count and pre-allocates + // `page_ids` is not overridden: lopdf's own page-tree walk trusts /Count and pre-allocates // by it, so a hostile /Count (a fuzz finding: 4294967295) aborted the process with a 48 GiB // allocation. The trait's bounded default walk counts the real pages instead. fn get_page_fonts(&self, page: ObjectId) -> Result, &Dictionary>> { @@ -226,8 +228,62 @@ trailer << /Size 4 /Root 1 0 R >> %%EOF"; let doc = lopdf::Document::load_mem(&pdf[..]).unwrap(); - let pages = Structure::get_pages(&doc); - assert_eq!(pages.len(), 1, "the walk counts the one real page, not /Count"); + let pages = Structure::page_ids(&doc); + assert_eq!(pages, [(3, 0)], "the walk counts the one real page, not /Count"); + } + + fn nested_sparse_page_tree(repeated_and_cyclic: bool) -> lopdf::Document { + let mut doc = lopdf::Document::with_version("1.7"); + let mut catalog = Dictionary::new(); + catalog.set("Type", Object::Name(b"Catalog".to_vec())); + catalog.set("Pages", Object::Reference((5, 0))); + doc.objects.insert((17, 0), Object::Dictionary(catalog)); + + let mut pages = Dictionary::new(); + pages.set("Type", Object::Name(b"Pages".to_vec())); + pages.set("Kids", vec![Object::Reference((31, 0)), Object::Reference((88, 0))]); + pages.set("Count", Object::Integer(if repeated_and_cyclic { 5 } else { 3 })); + doc.objects.insert((5, 0), Object::Dictionary(pages)); + + let mut nested = Dictionary::new(); + nested.set("Type", Object::Name(b"Pages".to_vec())); + nested.set("Parent", Object::Reference((5, 0))); + let kids = if repeated_and_cyclic { + vec![Object::Reference((12, 0)), Object::Reference((5, 0)), Object::Reference((31, 0)), Object::Reference((42, 0))] + } else { + vec![Object::Reference((12, 0)), Object::Reference((42, 0))] + }; + nested.set("Kids", kids); + nested.set("Count", Object::Integer(if repeated_and_cyclic { 4 } else { 2 })); + doc.objects.insert((88, 0), Object::Dictionary(nested)); + + for (id, parent) in [(31, 5), (12, 88), (42, 88)] { + let mut page = Dictionary::new(); + page.set("Type", Object::Name(b"Page".to_vec())); + page.set("Parent", Object::Reference((parent, 0))); + doc.objects.insert((id, 0), Object::Dictionary(page)); + } + doc.max_id = 88; + doc.trailer.set("Root", Object::Reference((17, 0))); + doc + } + + #[test] + fn page_walk_keeps_nested_sparse_order_and_first_visit_semantics() { + let doc = nested_sparse_page_tree(true); + assert_eq!(Structure::page_ids(&doc), [(31, 0), (12, 0), (42, 0)]); + } + + #[test] + fn lazy_and_compatibility_walks_keep_nested_sparse_order() { + let mut source = nested_sparse_page_tree(false); + let mut bytes = Vec::new(); + source.save_to(&mut bytes).unwrap(); + let expected = [(31, 0), (12, 0), (42, 0)]; + let eager = lopdf::Document::load_mem(&bytes).unwrap(); + let lazy = LazyStructure::new(Arc::new(bytes), None).unwrap(); + assert_eq!(Structure::page_ids(&eager), expected); + assert_eq!(Structure::page_ids(&lazy), expected); } #[test] @@ -246,10 +302,11 @@ trailer source.save_to(&mut bytes).unwrap(); let cos = Document::open(Arc::new(bytes)).unwrap(); let compressed = pdfcraft_cos::write_full(&cos, &pdfcraft_cos::SaveOptions::default()).unwrap(); - let expected = lopdf::Document::load_mem(&compressed).unwrap().get_pages(); + let eager = lopdf::Document::load_mem(&compressed).unwrap(); + let expected = Structure::page_ids(&eager); let view = LazyStructure::new(Arc::new(compressed), None).unwrap(); - assert_eq!(view.get_pages().len(), 1); - assert_eq!(view.get_pages(), expected); + assert_eq!(view.page_ids().len(), 1); + assert_eq!(view.page_ids(), expected); assert!(!view.failed()); assert!( view.objects.iter().filter(|(_, slot)| slot.get().is_some()).count() <= 3,