lopdf: a rebuild prefers a trailer whose /Root is the catalog (#2365 review)

The mirror of the cos change for render::inspect: a later cross-reference
stream whose /Root points at /Info replaced the earlier trailer, and Document
Properties lost the catalog. A candidate whose /Root is /Type /Catalog or has
/Pages now wins; another dictionary is still taken when nothing better is
there (FDF).

Test: inspect::tests::rebuilt_file_prefers_a_trailer_whose_root_is_the_catalog

crates/render/src/inspect.rs

@@ -2425,6 +2425,32 @@ trailer << /Root 1 0 R >>
         assert_eq!(info.creator.as_deref(), Some("Me"));
     }
 
+    /// #2365 review: the rebuild prefers a trailer whose /Root is the catalog. A later
+    /// cross-reference stream whose /Root points at the Info dictionary used to replace the
+    /// earlier `trailer`, so Document Properties lost the catalog it should have read.
+    #[test]
+    fn rebuilt_file_prefers_a_trailer_whose_root_is_the_catalog() {
+        let mut bytes = b"%PDF-1.5\n".to_vec();
+        for body in [
+            "<</Type/Catalog/Pages 2 0 R>>",
+            "<</Type/Pages/Kids[3 0 R]/Count 1>>",
+            "<</Type/Page/Parent 2 0 R/MediaBox[0 0 10 10]>>",
+            "<</Title(Kept)/Creator(Me)>>",
+        ] {
+            let n = bytes.iter().filter(|b| **b == b'\n').count();
+            bytes.extend_from_slice(format!("{n} 0 obj{body}endobj\n").as_bytes());
+        }
+        bytes.extend_from_slice(b"trailer\n<</Size 5/Root 1 0 R/Info 4 0 R>>\n");
+        bytes.extend_from_slice(b"5 0 obj<</Type/XRef/Root 4 0 R/Size 6/W[1 2 1]/Length 4>>stream\n\x01\x00\x09\x00\nendstream\nendobj\n");
+        // A wrong startxref, and a second file after %%EOF: every reader rebuilds the table.
+        bytes.extend_from_slice(b"startxref\n999999\n%%EOF\n%PDF-1.3\n");
+        bytes.extend_from_slice(&[b'x'; 1024]);
+
+        let info = inspect(Arc::new(bytes), None).unwrap();
+        assert_eq!(info.pages.len(), 1);
+        assert_eq!(info.title.as_deref(), Some("Kept"));
+    }
+
     #[test]
     fn lazy_structure_matches_the_compatibility_inspector() {
         // Each fixture as lopdf writes it, and with its objects in object streams.

vendor/lopdf/src/reader.rs

@@ -1627,14 +1627,30 @@ impl Reader<'_> {
         Some((number, generation))
     }
 
-    /// Newest backwards-scanned `trailer` whose `/Root` references a scanned object.
+    /// Newest backwards-scanned `trailer` whose `/Root` references a scanned object, one whose
+    /// `/Root` is the catalog first.
     fn find_latest_trailer(&self, xref: &Xref) -> Option<(usize, Dictionary)> {
         let usable = |dict: &Dictionary| {
             dict.get(b"Root")
                 .and_then(Object::as_reference)
                 .is_ok_and(|root| xref.entries.contains_key(&root.0))
         };
-        let mut latest = None;
+        // PdfCraft patch: (#2365) a candidate whose /Root is the catalog (`/Type /Catalog` or
+        // `/Pages`) beats one whose /Root is another dictionary, such as /Info, wherever it is;
+        // the other kind is still taken when nothing better is there (an FDF's /Root).
+        let is_catalog = |dict: &Dictionary| {
+            let Ok(root) = dict.get(b"Root").and_then(Object::as_reference) else {
+                return false;
+            };
+            let Some(XrefEntry::Normal { offset, .. }) = xref.entries.get(&root.0) else {
+                return false;
+            };
+            parser::indirect_object(self.buffer, *offset as usize, None, self, &mut HashSet::new(), None)
+                .ok()
+                .and_then(|(_, object)| object.as_dict().map(|d| d.has_type(b"Catalog") || d.has(b"Pages")).ok())
+                .unwrap_or(false)
+        };
+        let mut latest: Option<(bool, usize, Dictionary)> = None;
         let mut bound = self.buffer.len();
         for _ in 0..MAX_TRAILER_CANDIDATES {
             let Some(pos) = Reader::search_substring(&self.buffer[..bound], b"trailer", 0) else {
@@ -1650,8 +1666,13 @@ impl Reader<'_> {
                 continue;
             };
             if usable(&dict) {
-                latest = Some((pos, dict));
-                break;
+                let catalog = is_catalog(&dict);
+                if latest.is_none() || catalog {
+                    latest = Some((catalog, pos, dict));
+                }
+                if catalog {
+                    break;
+                }
             }
         }
         // PdfCraft patch: (#2365) a file whose only trailer is a cross-reference stream has
@@ -1665,7 +1686,7 @@ impl Reader<'_> {
                 continue;
             };
             let offset = *offset as usize;
-            if latest.as_ref().is_some_and(|(pos, _)| *pos >= offset) {
+            if latest.as_ref().is_some_and(|(catalog, pos, _)| *catalog && *pos >= offset) {
                 continue;
             }
             let Some(head) = self.buffer.get(offset..) else {
@@ -1685,10 +1706,13 @@ impl Reader<'_> {
                 continue;
             };
             if dict.has_type(b"XRef") && usable(&dict) {
-                latest = Some((offset, dict));
+                let rank = (is_catalog(&dict), offset);
+                if latest.as_ref().is_none_or(|(catalog, pos, _)| rank > (*catalog, *pos)) {
+                    latest = Some((rank.0, offset, dict));
+                }
             }
         }
-        latest
+        latest.map(|(_, pos, dict)| (pos, dict))
     }
 
     /// Some generators write `startxref` (or trailer `Prev`) values that are