Preferences: reconcile current main before final validation

crates/render/src/raster/tests.rs

@@ -8491,6 +8491,99 @@ mod devicen_inks;
 // patch 27).
 mod isolated_cmyk_groups;
 
+/// The generation-0 stand-in a rebuilt table files an object under must not make the object a
+/// second object: enumeration yielded it once per key, so a document recovered without a readable
+/// page tree reported the page twice (noted in review of #5580).
+///
+/// And a later header for the same object number replaces the stand-in, so `N 0 R` cannot be left
+/// pointing at a body the exact key no longer names.
+#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]
+#[cfg_attr(not(target_arch = "wasm32"), test)]
+fn a_rebuilt_tables_generation_0_stand_in_is_not_a_second_object() {
+    // No table, no `startxref`, and no readable page tree: the pages are recovered by looking for
+    // objects that are pages, which is where a doubled object shows up as a doubled page.
+    let pdf = "%PDF-1.4\n\
+               1 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 mut r = PageRenderer::new(Arc::new(pdf.to_string().into_bytes()), RenderConfig::default());
+    let first = r.render(RenderRequest { page: 0, kind: RequestKind::Pixels, tile: None, scale: 1.0, tag: 0, preview: false });
+    assert!(first.error.is_none(), "the page is recovered: {:?}", first.error);
+    let second = r.render(RenderRequest { page: 1, kind: RequestKind::Pixels, tile: None, scale: 1.0, tag: 0, preview: false });
+    assert!(second.error.is_some(), "and it is one page, not two: page 2 must not exist");
+
+    // Two headers for object 3, the second the one the exact key names: the stand-in follows it.
+    // `(B)` is drawn by the later body, so the page that comes back says which one was read.
+    let twice = "%PDF-1.4\n\
+                 1 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] /Contents 5 0 R >> endobj\n\
+                 3 1 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 300 150] /Contents 5 0 R >> endobj\n\
+                 5 0 obj << /Length 0 >> stream\n\nendstream endobj\n\
+                 trailer << /Root 1 0 R >>\n\
+                 %%EOF\n";
+    let mut r = PageRenderer::new(Arc::new(twice.to_string().into_bytes()), RenderConfig::default());
+    let page = r.render(RenderRequest { page: 0, kind: RequestKind::Pixels, tile: None, scale: 1.0, tag: 0, preview: false });
+    assert!(page.error.is_none(), "the page is found: {:?}", page.error);
+    assert_eq!((page.width, page.height), (300, 150), "`3 0 R` reads the later body, as `3 1` does");
+}
+
+/// A file whose cross-reference table has to be rebuilt keeps an object whose header names a
+/// generation the reference does not.
+///
+/// `poppler-85140-0.pdf` is 340 bytes with no table at all, and its one page's header reads
+/// `3 18446744073709551616 obj` while the page tree points at `3 0 R`. Before #427 the digits
+/// wrapped, and 2^64 wrapped to exactly 0, so the header read as `3 0 obj` and the file opened by
+/// accident; once an overflowing integer became a real the object was unreachable and the file
+/// stopped opening at all, which `cargo xtask check` reports as a regression (#5580).
+///
+/// A rebuilt table has no revision history to tell two generations of an object apart -- the scan
+/// already files objects found in object streams under generation 0 -- so the object number is
+/// what identifies one. A readable generation that simply is not the one referenced
+/// (`3 99999 obj`) was lost the same way, and is covered here too.
+#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)]
+#[cfg_attr(not(target_arch = "wasm32"), test)]
+fn a_rebuilt_table_finds_an_object_whose_header_names_another_generation() {
+    for generation in [
+        // The generation the reference names, which always worked.
+        "0",
+        // One that reads as an integer but is not the one referenced.
+        "99999",
+        // And one no generation may be: 2^64, which is what the reported file carries. A
+        // generation number is at most 65,535 (ISO 32000-2 §7.5.4).
+        "18446744073709551616",
+    ] {
+        // No table and no `startxref`, so the objects are recovered by scanning the file.
+        let pdf = format!(
+            "%PDF-1.4\n\
+             1 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 mut r = PageRenderer::new(Arc::new(pdf.into_bytes()), RenderConfig::default());
+        let page = r.render(RenderRequest { page: 0, kind: RequestKind::Pixels, tile: None, scale: 1.0, tag: 0, preview: false });
+        assert!(page.error.is_none(), "generation {generation}: the page is found: {:?}", page.error);
+        assert_eq!((page.width, page.height), (200, 100), "generation {generation}: and it is the page's own size");
+    }
+
+    // A stray number before a header (`42 3 0 obj`) is not a header, and must not cost the one
+    // after it: the lenient read leaves the scan where it was when it fails (noted in review).
+    let pdf = "%PDF-1.4\n\
+               1 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 mut r = PageRenderer::new(Arc::new(pdf.to_string().into_bytes()), RenderConfig::default());
+    let page = r.render(RenderRequest { page: 0, kind: RequestKind::Pixels, tile: None, scale: 1.0, tag: 0, preview: false });
+    assert!(page.error.is_none(), "the header after a stray number is found: {:?}", page.error);
+    assert_eq!((page.width, page.height), (200, 100));
+}
+
 /// #3109 / #3116: clipping-only text (`7 Tr`) installs a clip for subsequent painting
 /// but draws no pixels itself.
 #[test]

crates/ui-egui/src/panels/page_selection_ui.rs

@@ -22,7 +22,9 @@ pub(crate) fn parse(text: &str, count: usize) -> Result<Vec<usize>, PrintError>
     let mut pages = BTreeSet::new();
     let mut budget = 1_000_000usize;
     let number = |token: &str| {
-        let n = token.trim().parse::<usize>().map_err(|_| PrintError::NotAPage(token.into()))?;
+        let t = token.trim();
+        let n =
+            if t.bytes().all(|b| b.is_ascii_digit()) { t.parse::<usize>().ok() } else { None }.ok_or_else(|| PrintError::NotAPage(token.into()))?;
         if n == 0 || n > count { Err(PrintError::OutOfRange(n, count)) } else { Ok(n - 1) }
     };
     for part in text.split([',', ';']).map(str::trim) {
@@ -132,6 +134,13 @@ impl PageSelectionInput {
 mod tests {
     use super::*;
 
+    #[test]
+    fn plus_sign_is_not_a_page_number() {
+        for text in ["+3", "+2-4", "2-+4"] {
+            assert!(matches!(parse(text, 100), Err(PrintError::NotAPage(_))), "{text}");
+        }
+    }
+
     #[test]
     fn selection_ranges_are_sorted_unique_and_bounded() {
         let pages = parse("1,12,16-18,89,92,93,97,99", 100).unwrap();

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::{
@@ -80,6 +81,65 @@ pub(crate) fn fallback(data: PdfData, password: &[u8]) -> Option<XRef> {
     }
 }
 
+/// 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<ObjectIdentifier> {
+    let mut strict = r.clone();
+    if let Some(obj_id) = strict.read::<ObjectIdentifier>(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::<i32>()?;
+    lenient.skip_white_spaces_and_comments();
+    // Any number at all in the generation position, including one no generation may be.
+    lenient.skip::<Number>(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<T>
+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::<T>(&ctx)
+}
+
 fn fallback_xref_map<'a>(data: &'a PdfData, password: &[u8]) -> (XrefMap, Option<XRefInput<'a>>) {
     fallback_xref_map_inner(data, ReaderContext::dummy(), true, password)
 }
@@ -91,6 +151,10 @@ fn fallback_xref_map_inner<'a>(
     password: &[u8],
 ) -> (XrefMap, Option<XRefInput<'a>>) {
     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<ObjectIdentifier> = rustc_hash::FxHashSet::default();
     let mut trailer_dicts = vec![];
     let mut root_ref = None;
 
@@ -105,12 +169,32 @@ 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::<ObjectIdentifier>(&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::<Object<'_>>(false).is_some() {
                     xref_map.insert(obj_id, EntryType::Normal(cur_pos));
+                    // 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). `or_insert`, so a
+                    // real `N 0 obj` is never displaced by this alias.
+                    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).
+                        xref_map.insert(by_number, EntryType::Normal(cur_pos));
+                        aliases.insert(by_number);
+                    }
                     last_obj_num = Some(obj_id);
                     dummy_ctx.set_obj_number(obj_id);
                 }
@@ -498,6 +582,14 @@ impl XRef {
                 // Try to yield in the order the objects appeared in the
                 // PDF.
                 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();
 
@@ -653,10 +745,27 @@ impl XRef {
                 }
 
                 if let Some(object) = r.read_with_context::<IndirectObject<T>>(&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(Fetched::Object(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::<T>(repr.data.get().as_ref(), offset, id, &ctx)
+                    {
+                        return Some(Fetched::Object(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::<IndirectObject<Object<'_>>>(false).is_some() {
@@ -1734,6 +1843,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::<i32>(b"Marker") == Some(123))
+        ));
+    }
+
     #[test]
     fn recursive_object_stream_locations_fail_without_recursing() {
         let pdf = crate::Pdf::new(object_stream_fixture(true)).unwrap();