From 7f503628945f27423973c879c9123c58a3359d2d Mon Sep 17 00:00:00 2001 From: Velli20 Date: Tue, 27 Oct 2026 18:14:29 +0200 Subject: [PATCH] Identify object-stream members by header number; accept untyped page-tree nodes The loader matched compressed objects by the xref's in-stream index and stored whatever object sat at that index under the requested number. An xref whose index field is too narrow for a large object stream wraps the index, so the catalog resolved to an unrelated object. The stream header already names each member's object number, so members are now inserted under that number; objects the header does not list stay absent with a diagnostic. The unused index is dropped from CrossReferenceEntryType. Page-tree kids without /Type are now a branch when they have /Kids and a page otherwise, instead of failing the read. Fixes #573 Fixes #605 Co-Authored-By: Claude Opus 5.5 --- crates/pdf-document/src/object_loader.rs | 114 ++++++------------ crates/pdf-document/src/page_tree.rs | 17 ++- .../src/cross_reference_table.rs | 11 +- .../pdf-parser/src/cross_reference_stream.rs | 6 +- crates/pdf-parser/tests/xref_builder.rs | 4 - 5 files changed, 55 insertions(+), 97 deletions(-) diff --git a/crates/pdf-document/src/object_loader.rs b/crates/pdf-document/src/object_loader.rs index 29678210..a745dba3 100644 --- a/crates/pdf-document/src/object_loader.rs +++ b/crates/pdf-document/src/object_loader.rs @@ -12,7 +12,7 @@ //! deferred object is retried only when the exact object number it needs becomes //! available. -use std::collections::{BTreeMap, HashMap, VecDeque}; +use std::collections::{BTreeMap, BTreeSet, HashMap, VecDeque}; use crate::diagnostic::{PdfReadDiagnostic, PdfReadDiagnosticKind}; use crate::encryption_context::EncryptionContext; @@ -29,12 +29,11 @@ use pdf_parser::{ parser::PdfParser, }; -/// Maps an index within one object stream to the xref object numbers using that index. +/// The object numbers the xref places in one object stream. /// -/// Multiple object numbers can target the same index in malformed PDFs. Keeping -/// all of them preserves best-effort behavior while allowing the normal, -/// one-to-one case to move the parsed value without cloning it. -type CompressedStreamPlan = BTreeMap>; +/// Members are found by the object numbers in the stream's header, which is the +/// only authority on what the stream holds. +type CompressedStreamPlan = BTreeSet; /// Work derived from the cross-reference table before object parsing begins. struct LoadPlan { @@ -63,13 +62,12 @@ impl LoadPlan { // earlier objects, so retain the reader's established reverse-xref order. normal_objects.reverse(); - // BTreeMap keeps stream and index processing deterministic, including + // BTreeMap keeps stream processing deterministic, including // which unresolved stream is reported first. let mut compressed_streams = BTreeMap::::new(); for (&object_number, entry) in entries { let CrossReferenceEntryType::Compressed { object_stream_number, - index_within_stream, } = entry else { continue; @@ -77,15 +75,12 @@ impl LoadPlan { compressed_streams .entry(*object_stream_number) .or_default() - .entry(*index_within_stream) - .or_default() - .push(object_number); + .insert(object_number); } let compressed_count = compressed_streams .values() - .flat_map(BTreeMap::values) - .map(Vec::len) + .map(BTreeSet::len) .fold(0usize, usize::saturating_add); let object_capacity = normal_objects.len().saturating_add(compressed_count); @@ -149,7 +144,7 @@ enum ObjectLoadTask { CompressedStream { /// Indirect object number of the containing `/ObjStm`. object_stream_number: usize, - /// Xref members requested from the stream, grouped by stream index. + /// Object numbers the xref places in the stream. plan: CompressedStreamPlan, }, } @@ -440,14 +435,14 @@ impl<'input, 'loader> ObjectLoader<'input, 'loader> { /// Parses one available object stream and inserts all xref-requested members. /// - /// The header is walked once, in step with the index-ordered plan, and only - /// requested members are parsed. Reading stops after the last requested index. + /// Each member is identified by the object number the stream's header gives it. + /// The header is read only until every requested member has been found. /// /// Returns the inserted object numbers so the scheduler can wake their dependents. fn load_compressed_stream( &mut self, object_stream_number: usize, - stream_plan: CompressedStreamPlan, + mut requested: CompressedStreamPlan, ) -> Result, PdfReaderError> { let stream_object = self.objects.get(object_stream_number).ok_or( ObjectError::FailedResolveObjectReference { @@ -455,71 +450,40 @@ impl<'input, 'loader> ObjectLoader<'input, 'loader> { }, )?; let object_stream: ObjectStream = ObjectReader::new(&self.objects).read(stream_object)?; - let mut loaded = Vec::with_capacity(stream_plan.len()); - let mut requests = stream_plan.into_iter().peekable(); + let mut loaded = Vec::with_capacity(requested.len()); - for (index_within_stream, entry) in object_stream.entries().enumerate() { - if requests.peek().is_none() { + for entry in object_stream.entries() { + if requested.is_empty() { break; } let entry = entry?; - let Some((_, object_numbers)) = - requests.next_if(|(requested_index, _)| *requested_index == index_within_stream) - else { + if !requested.remove(&entry.number) { continue; - }; + } let Some(value) = object_stream.parse_member(&entry)? else { continue; }; - self.insert_compressed_member(&object_numbers, entry.number, value, &mut loaded); + self.objects.insert_compressed(entry.number, value); + loaded.push(entry.number); } + self.report_missing_members(requested); Ok(loaded) } - /// Inserts one parsed object-stream member under every xref number that requested it. - /// - /// A clone is required only when a malformed xref maps more than one object - /// number to the same object-stream index; the final target takes ownership. - fn insert_compressed_member( - &mut self, - object_numbers: &[usize], - stream_object_number: usize, - value: ObjectVariant, - loaded: &mut Vec, - ) { - let Some((&last_object_number, duplicate_object_numbers)) = object_numbers.split_last() - else { - return; - }; - - for &object_number in duplicate_object_numbers { - self.report_compressed_number_mismatch(object_number, stream_object_number); - self.objects.insert_compressed(object_number, value.clone()); - loaded.push(object_number); - } - - self.report_compressed_number_mismatch(last_object_number, stream_object_number); - self.objects.insert_compressed(last_object_number, value); - loaded.push(last_object_number); - } - - /// Records a recoverable diagnostic when xref and object-stream numbers disagree. - fn report_compressed_number_mismatch( - &mut self, - expected_object_number: usize, - actual_object_number: usize, - ) { - if actual_object_number == expected_object_number { - return; - } - self.diagnostics.push(PdfReadDiagnostic::new( - PdfReadDiagnosticKind::CompressedObject, - None, - // Objects stored in object streams always have generation zero. - Some(ObjectId::new(expected_object_number, 0)), - "xref compressed object number differed from its object stream entry", - )); + /// Records a recoverable diagnostic for each object the xref places in an object + /// stream whose header does not list it. Such objects stay absent. + fn report_missing_members(&mut self, missing: CompressedStreamPlan) { + self.diagnostics + .extend(missing.into_iter().map(|object_number| { + PdfReadDiagnostic::new( + PdfReadDiagnosticKind::CompressedObject, + None, + // Objects stored in object streams always have generation zero. + Some(ObjectId::new(object_number, 0)), + "object stream does not contain the object", + ) + })); } } @@ -528,12 +492,12 @@ mod tests { use super::*; #[test] - fn load_plan_partitions_entries_and_groups_compressed_indexes() { + fn load_plan_partitions_entries_and_groups_compressed_members() { let entries = BTreeMap::from([ (0, CrossReferenceEntryType::new_free(0, 65_535)), (1, CrossReferenceEntryType::new_normal(10, 0)), - (2, CrossReferenceEntryType::new_compressed(5, 1)), - (3, CrossReferenceEntryType::new_compressed(5, 1)), + (2, CrossReferenceEntryType::new_compressed(5)), + (3, CrossReferenceEntryType::new_compressed(5)), (4, CrossReferenceEntryType::new_normal(0, 0)), (5, CrossReferenceEntryType::new_normal(50, 0)), ]); @@ -543,10 +507,8 @@ mod tests { assert_eq!(plan.normal_objects, VecDeque::from([50, 10])); assert_eq!(plan.object_capacity, 4); assert_eq!( - plan.compressed_streams - .get(&5) - .and_then(|stream| stream.get(&1)), - Some(&vec![2, 3]) + plan.compressed_streams.get(&5), + Some(&BTreeSet::from([2, 3])) ); } diff --git a/crates/pdf-document/src/page_tree.rs b/crates/pdf-document/src/page_tree.rs index 6b1084c1..1ed5b673 100644 --- a/crates/pdf-document/src/page_tree.rs +++ b/crates/pdf-document/src/page_tree.rs @@ -45,7 +45,8 @@ impl FromPdfObject for PageTree { } } -/// A `/Kids` entry of a page-tree node, dispatched on its `/Type`. +/// A `/Kids` entry of a page-tree node, dispatched on its `/Type`, or on whether it has +/// `/Kids` when the type is missing. enum PageTreeNode { /// A leaf `/Page` object. Page(PdfPage), @@ -57,12 +58,16 @@ impl FromPdfObject for PageTreeNode { fn from_pdf_object(context: ObjectContext<'_, impl ObjectAccess + ?Sized>) -> ReadResult { let object = context.object(); let mut dictionary = context.dictionary()?; - let node_type: Arc<[u8]> = dictionary.required(b"Type")?; + let node_type: Option> = dictionary.optional(b"Type")?; + let has_kids = dictionary.contains(b"Kids"); let context = ObjectContext::new(object, dictionary.access()); - match &*node_type { - PdfPage::KEY => context.decode().map(Self::Page), - PageTree::KEY => context.decode().map(Self::Pages), - other => Err(PdfPagesError::InvalidKidsEntryType { + match node_type.as_deref() { + Some(PdfPage::KEY) => context.decode().map(Self::Page), + Some(PageTree::KEY) => context.decode().map(Self::Pages), + // A node without `/Type` is an intermediate node when it has kids and a page otherwise. + None if has_kids => context.decode().map(Self::Pages), + None => context.decode().map(Self::Page), + Some(other) => Err(PdfPagesError::InvalidKidsEntryType { found_type: String::from_utf8_lossy(other).into_owned(), } .into()), diff --git a/crates/pdf-object-reader/src/cross_reference_table.rs b/crates/pdf-object-reader/src/cross_reference_table.rs index 7ef09487..c3b7a5a5 100644 --- a/crates/pdf-object-reader/src/cross_reference_table.rs +++ b/crates/pdf-object-reader/src/cross_reference_table.rs @@ -52,10 +52,10 @@ pub enum CrossReferenceEntryType { generation_number: usize, }, /// Type 2: object stored in a compressed object stream. - Compressed { - object_stream_number: usize, - index_within_stream: usize, - }, + /// + /// The member is found by its object number in the stream's header; the xref's + /// in-stream index is not kept, because it can disagree with the header. + Compressed { object_stream_number: usize }, /// Type 0: free object (not in use). Free { next_free_object: usize, @@ -73,10 +73,9 @@ impl CrossReferenceEntryType { } /// Creates an entry for an object stored within a compressed object stream. - pub fn new_compressed(object_stream_number: usize, index_within_stream: usize) -> Self { + pub fn new_compressed(object_stream_number: usize) -> Self { CrossReferenceEntryType::Compressed { object_stream_number, - index_within_stream, } } diff --git a/crates/pdf-parser/src/cross_reference_stream.rs b/crates/pdf-parser/src/cross_reference_stream.rs index 0f0ef409..00d84526 100644 --- a/crates/pdf-parser/src/cross_reference_stream.rs +++ b/crates/pdf-parser/src/cross_reference_stream.rs @@ -141,7 +141,7 @@ impl<'data> XrefStreamEntryDecoder<'data> { Some(match entry_type { 0 => CrossReferenceEntryType::new_free(second_value, third_value), 1 => CrossReferenceEntryType::new_normal(second_value, third_value), - 2 => CrossReferenceEntryType::new_compressed(second_value, third_value), + 2 => CrossReferenceEntryType::new_compressed(second_value), _ => CrossReferenceEntryType::new_free(0, 0), }) } @@ -249,10 +249,8 @@ mod tests { match e2 { CrossReferenceEntryType::Compressed { object_stream_number, - index_within_stream, } => { assert_eq!(*object_stream_number, 5); - assert_eq!(*index_within_stream, 0); } _ => panic!("Expected compressed entry"), } @@ -439,10 +437,8 @@ mod tests { match e3 { CrossReferenceEntryType::Compressed { object_stream_number, - index_within_stream, } => { assert_eq!(*object_stream_number, 1); - assert_eq!(*index_within_stream, 0); } _ => panic!("Expected compressed entry"), } diff --git a/crates/pdf-parser/tests/xref_builder.rs b/crates/pdf-parser/tests/xref_builder.rs index 9e67eec1..fa499a42 100644 --- a/crates/pdf-parser/tests/xref_builder.rs +++ b/crates/pdf-parser/tests/xref_builder.rs @@ -926,10 +926,8 @@ fn build_xref_table_merges_hybrid_xref_stream_entries() { match pages_entry { CrossReferenceEntryType::Compressed { object_stream_number, - index_within_stream, } => { assert_eq!(*object_stream_number, 4); - assert_eq!(*index_within_stream, 0); } other => panic!("expected compressed xref entry for obj 2, got {other:?}"), } @@ -938,10 +936,8 @@ fn build_xref_table_merges_hybrid_xref_stream_entries() { match page_entry { CrossReferenceEntryType::Compressed { object_stream_number, - index_within_stream, } => { assert_eq!(*object_stream_number, 4); - assert_eq!(*index_within_stream, 1); } other => panic!("expected compressed xref entry for obj 3, got {other:?}"), }