From 1d92efa70702c59fe32be3d1b104641ca88e1db5 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 21:49:13 -0500 Subject: [PATCH] hayro-syntax: the repair scan finds an object whose header names another generation (#900) Ports pdfcraft 4a0b1dd1 (hayro-syntax patch 34, #5580). --- vendor/README.craft-reader.md | 9 ++ vendor/hayro-syntax/SOURCE.json | 2 +- vendor/hayro-syntax/src/xref.rs | 173 +++++++++++++++++++++++++++++++- 3 files changed, 180 insertions(+), 4 deletions(-) diff --git a/vendor/README.craft-reader.md b/vendor/README.craft-reader.md index 81c5934ff..3439657b5 100644 --- a/vendor/README.craft-reader.md +++ b/vendor/README.craft-reader.md @@ -378,6 +378,15 @@ comments label them `Craft patch (flatten)`. | hayro-interpret `types.rs`, `x_object.rs` | `RasterImage::with_inks`: an image's samples in its own space's inks (`InkImage`), `/Decode` and `/Matte` applied, transfer functions left out, with its alpha at the alpha's own size | | hayro-interpret `interpret/mod.rs` | The `sh` operator drops the shading's `/Background`, which only a shading pattern paints (ISO 32000-2 §8.7.4.5.1) | +Repair scan, ported from PdfCraft [4a0b1dd1](https://github.com/storytold/pdfcraft/commit/4a0b1dd15) +(hayro-syntax patch 34, `xref.rs`, #5580): in a table rebuilt by scanning the file, an object whose +header names a generation the reference does not (`3 99999 obj`, or `3 18446744073709551616 obj` as in +poppler-85140-0.pdf) is found by its object number: the header is read leniently (an unreadable +generation is 0), each scanned object is also filed under generation 0 (a real `N 0 obj` is never +displaced, a later header replaces a stand-in), the object is read back behind the same lenient header, +and enumeration yields one object per location and number. Tests: +`xref::tests::a_rebuilt_table_finds_an_object_whose_header_names_another_generation` and +`a_rebuilt_tables_generation_0_stand_in_is_not_a_second_object`. Streams without a readable `endstream`, ported from PdfCraft [bfaa20db](https://github.com/storytold/pdfcraft/commit/bfaa20db2) and [9e29af78](https://github.com/storytold/pdfcraft/commit/9e29af784) (hayro-syntax patch 36, in diff --git a/vendor/hayro-syntax/SOURCE.json b/vendor/hayro-syntax/SOURCE.json index f2571d30b..e68a7b4a4 100644 --- a/vendor/hayro-syntax/SOURCE.json +++ b/vendor/hayro-syntax/SOURCE.json @@ -338,7 +338,7 @@ "path": "src/xref.rs", "source_sha256": "51ffeeb7d4ac035bae2cd5ba9288c180088ae06cdcefe2b29dfeda560a793aa0", "source_git_blob": "20e33a410489d42e438a3f2b87d70c7bb8d66eae", - "sha256": "f47507871327f04cc1fa30ed731dacd093144de7049342f8d11c72b6d0eeed70" + "sha256": "920adb9a303f92f3fed721242e5d0976d36ebc2a3e16ef55e19e254a0d16d564" }, { "path": "src/decode_limits_tests.rs", diff --git a/vendor/hayro-syntax/src/xref.rs b/vendor/hayro-syntax/src/xref.rs index 03573a392..5fced8a88 100644 --- a/vendor/hayro-syntax/src/xref.rs +++ b/vendor/hayro-syntax/src/xref.rs @@ -4,6 +4,7 @@ use crate::crypto::{DecryptionError, DecryptionTarget, Decryptor, get}; use crate::data::Data; use crate::metadata::Metadata; use crate::object::Name; +use crate::object::Number; use crate::object::ObjectIdentifier; use crate::object::Stream; use crate::object::dict::keys::{ @@ -68,10 +69,70 @@ pub(crate) fn fallback_controlled(data: PdfData, password: &[u8], limits: Decode } } +/// PdfCraft patch: an `N G obj` header for the repair scan, taking a generation number that +/// cannot be read as 0. +/// +/// A generation number is at most 65,535 (ISO 32000-2 §7.5.4), so `3 18446744073709551616 obj` is +/// malformed and [`ObjectIdentifier::read`] is right to refuse it everywhere else. This scan runs +/// only when a file's cross-reference table is unusable and its objects are being recovered by +/// reading the file, where refusing the header drops the object and whatever it holds (#5580). +/// Before #427 the digits wrapped, and 2^64 wrapped to exactly 0, which is the only reason such a +/// file used to open. +/// +/// Only here: reading numbers anywhere else is unchanged. +fn read_object_header<'a>(r: &mut Reader<'a>, ctx: &ReaderContext<'a>) -> Option { + let mut strict = r.clone(); + if let Some(obj_id) = strict.read::(ctx) { + *r = strict; + return Some(obj_id); + } + // On a clone as well, so a header that isn't one leaves `r` where it was for the caller. + let mut lenient = r.clone(); + let obj_number = lenient.read_without_context::()?; + lenient.skip_white_spaces_and_comments(); + // Any number at all in the generation position, including one no generation may be. + lenient.skip::(false)?; + lenient.skip_white_spaces_and_comments(); + lenient.forward_tag(b"obj")?; + *r = lenient; + Some(ObjectIdentifier::new(obj_number, 0)) +} + +/// PdfCraft patch: the body of the object at `offset` whose `N G obj` header cannot be read +/// strictly, for a table rebuilt by scanning the file (#5580). The steps are +/// `IndirectObject::read`'s, with [`read_object_header`] in place of the strict identifier. +fn read_repaired_body<'a, T>( + data: &'a [u8], + offset: usize, + id: ObjectIdentifier, + ctx: &ReaderContext<'a>, +) -> Option +where + T: ObjectLike<'a>, +{ + let mut r = Reader::new(data); + r.jump(offset); + // The number is what identifies the object; the generation is what could not be read. + if read_object_header(&mut r, ctx)?.obj_number != id.obj_number { + return None; + } + let mut ctx = ctx.clone(); + if ctx.parent_chain_contains(&id) { + warn!("cycle detected in indirect object: {id:?}"); + + return None; + } + ctx.set_obj_number(id); + ctx.parent_chain_push(id); + r.skip_white_spaces_and_comments(); + r.read_with_context::(&ctx) +} + fn fallback_xref_map<'a>(data: &'a PdfData, password: &[u8], limits: DecodeLimits, control: Option) -> (XrefMap, Option>) { fallback_xref_map_inner(data, ReaderContext::dummy_with_control(limits, control), true, password) } + fn fallback_xref_map_inner<'a>( data: &'a PdfData, mut dummy_ctx: ReaderContext<'a>, @@ -79,6 +140,10 @@ fn fallback_xref_map_inner<'a>( password: &[u8], ) -> (XrefMap, Option>) { let mut xref_map = FxHashMap::default(); + // PdfCraft patch: the generation-0 keys below that stand in for a header with another + // generation, so a later header can replace one while a real `N 0 obj` is never displaced + // (noted in review of #5580). + let mut aliases: rustc_hash::FxHashSet = rustc_hash::FxHashSet::default(); let mut trailer_dicts = vec![]; let mut root_ref = None; @@ -97,13 +162,33 @@ fn fallback_xref_map_inner<'a>( // First try to check if we have an object identifier. if r.peek_byte().is_some_and(|b: u8| b.is_ascii_digit()) { - if let Some(obj_id) = r.read::(&dummy_ctx) { + if let Some(obj_id) = read_object_header(&mut r, &dummy_ctx) { let mut cloned = r.clone(); // Check that the object following it is actually valid before inserting it. cloned.skip_white_spaces_and_comments(); if cloned.skip::>(false).is_some() && insert_entry(&mut xref_map, obj_id, EntryType::Normal(cur_pos)).is_some() { + // PdfCraft patch: also reachable by object number alone, so a reference that + // names a different generation than the header still finds it. A rebuilt + // table has no revision history to tell two generations apart, and a file + // that needs rebuilding references everything as `N 0 R` whatever its headers + // say: `poppler-85140-0.pdf` has one page whose header reads + // `3 18446744073709551616 obj` with a page tree pointing at `3 0 R`, so the + // page was lost and the file did not open at all (#5580). + let by_number = ObjectIdentifier::new(obj_id.obj_number, 0); + if obj_id.gen_number == 0 { + // A real `N 0 obj`: it owns the key from here on, and the last one wins + // as every other entry does. + aliases.remove(&by_number); + } else if !xref_map.contains_key(&by_number) || aliases.contains(&by_number) { + // Stand in for it, and let a later header for the same number replace the + // stand-in -- the exact key moves on, so an alias left behind would send + // `N 0 R` to a body the exact key no longer names (noted in review). + if insert_entry(&mut xref_map, by_number, EntryType::Normal(cur_pos)).is_some() { + aliases.insert(by_number); + } + } last_obj_num = Some(obj_id); dummy_ctx.set_obj_number(obj_id); } @@ -491,7 +576,14 @@ impl XRef { } else { elements.sort_by(|e1, e2| e1.1.cmp(&e2.1)); } - + // PdfCraft patch: one object per location. A rebuilt table can hold two keys for + // one object -- its own generation and the generation-0 stand-in (#5580) -- and + // enumeration yielded the object once for each, so a document recovered by + // `Pages::new_brute_force` reported the page twice (noted in review). Both keys + // stay for lookups; only this walk is deduplicated. The object number is part of + // the key: an object stream at (offset, 0) and its first member, also + // (offset, 0), are two objects. + elements.dedup_by(|a, b| a.1 == b.1 && a.0.obj_number == b.0.obj_number); let mut iter = elements.into_iter(); iter::from_fn(move || { @@ -607,10 +699,27 @@ impl XRef { r.jump(offset); if let Some(object) = r.read_with_context::>(&ctx) { - if object.id() == &id { + // PdfCraft patch: in a table rebuilt by scanning the file, the object number + // is what identifies an object. There is no revision history to tell two + // generations apart, the scan already files objects found in object streams + // under generation 0, and a malformed generation in a header would otherwise + // make the object unreadable through the reference that names it (#5580). A + // table read from the file's own xref keeps the exact match. + if object.id() == &id || (repaired && object.id().obj_number == id.obj_number) { return Some(object.get()); } } else { + // PdfCraft patch: the strict read of the header failed. In a table rebuilt by + // scanning the file that can be a generation number no generation may be -- + // `3 18446744073709551616 obj` -- and refusing it loses the object the + // reference names (#5580). The object number still has to be the one asked + // for; only an unreadable generation is tolerated, and only here. + if repaired + && let Some(object) = + read_repaired_body::(repr.data.get().as_ref(), offset, id, &ctx) + { + return Some(object); + } // There is a valid object at the offset, it's just not of the type the caller // expected, which is fine. if r.skip::>>(false).is_some() { @@ -1535,6 +1644,18 @@ mod tests { } } + #[test] + fn an_object_stream_and_its_first_member_are_both_enumerated() { + // Both sit at (stream offset, 0) in `objects`' ordering; one object per location must + // not cost either of them. + let pdf = crate::Pdf::new(object_stream_fixture(false)).unwrap(); + let objects: Vec<_> = pdf.xref().objects().into_iter().collect(); + assert!(objects.iter().any(|o| matches!(o, Object::Stream(_)))); + assert!(objects.iter().any( + |o| matches!(o, Object::Dict(d) if d.get::(b"Marker") == Some(123)) + )); + } + #[test] fn recursive_object_stream_locations_fail_without_recursing() { let pdf = crate::Pdf::new(object_stream_fixture(true)).unwrap(); @@ -1732,6 +1853,52 @@ mod tests { let mut reader = Reader::new(data); assert!(read_xref_table_trailer(&mut reader, &ReaderContext::dummy()).is_none()); } + + /// PdfCraft patch (#5580): a file whose table is rebuilt by scanning keeps an object whose header + /// names a generation the reference does not (`3 99999 obj`, or `3 18446744073709551616 obj` as in + /// poppler-85140-0.pdf, no generation may be), since the scan has no revision history and such a + /// file references everything as `N 0 R`. A stray number before a header (`42 3 0 obj`) costs nothing. + #[test] + fn a_rebuilt_table_finds_an_object_whose_header_names_another_generation() { + for generation in ["0", "99999", "18446744073709551616"] { + let pdf = alloc::format!( + "%PDF-1.4\n1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj\n\ + 2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n\ + 3 {generation} obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 200 100] >> endobj\n\ + trailer << /Root 1 0 R >>\n%%EOF\n" + ); + let pdf = crate::Pdf::new(pdf.into_bytes()).unwrap(); + assert_eq!(pdf.pages().len(), 1, "generation {generation}"); + assert_eq!(pdf.pages()[0].render_dimensions(), (200.0, 100.0), "generation {generation}"); + } + let pdf = "%PDF-1.4\n1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj\n\ + 2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n\ + 42 3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 200 100] >> endobj\n\ + trailer << /Root 1 0 R >>\n%%EOF\n"; + let pdf = crate::Pdf::new(pdf.as_bytes().to_vec()).unwrap(); + assert_eq!(pdf.pages()[0].render_dimensions(), (200.0, 100.0), "the header after a stray number"); + } + + /// PdfCraft patch (#5580): the generation-0 stand-in is one key, not a second object (a document + /// recovered without a page tree reported its page twice), and a later header for the same number + /// replaces it, so `3 0 R` reads the body `3 1 R` reads. + #[test] + fn a_rebuilt_tables_generation_0_stand_in_is_not_a_second_object() { + let pdf = "%PDF-1.4\n1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj\n\ + 3 99999 obj << /Type /Page /MediaBox [0 0 200 100] /Contents 4 0 R >> endobj\n\ + 4 0 obj << /Length 0 >> stream\n\nendstream endobj\n\ + trailer << /Root 1 0 R >>\n%%EOF\n"; + let pdf = crate::Pdf::new(pdf.as_bytes().to_vec()).unwrap(); + assert_eq!(pdf.pages().len(), 1, "one page, not two"); + + let twice = "%PDF-1.4\n1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj\n\ + 2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n\ + 3 1 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 200 100] >> endobj\n\ + 3 1 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 300 150] >> endobj\n\ + trailer << /Root 1 0 R >>\n%%EOF\n"; + let pdf = crate::Pdf::new(twice.as_bytes().to_vec()).unwrap(); + assert_eq!(pdf.pages()[0].render_dimensions(), (300.0, 150.0), "`3 0 R` reads the later body, as `3 1` does"); + } } #[cfg(test)]