lopdf: a rebuilt trailer's /Root must match its generation (#2365 review)

usable() and is_catalog() checked the object number only, so a later stream
with /Root 1 1 R outranked the trailer naming 1 0 R; get_offset then found no
such object and Document Properties lost the catalog. cos resolves by number
alone, so its rank needs no change.

Test: inspect::tests::rebuilt_file_prefers_a_trailer_whose_root_is_the_catalog
This commit is contained in:
wstock
2026-10-18 16:05:33 +00:00
parent d75d440ad9
commit a2bc0d3570
3 changed files with 18 additions and 4 deletions
+7 -1
View File
@@ -2446,9 +2446,15 @@ trailer << /Root 1 0 R >>
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();
let info = inspect(Arc::new(bytes.clone()), None).unwrap();
assert_eq!(info.pages.len(), 1);
assert_eq!(info.title.as_deref(), Some("Kept"));
// A later stream whose /Root names the catalog's number with another generation does
// not count either: loading `1 1 R` finds no such object.
let at = bytes.windows(10).position(|w| w == b"/Root 4 0 ").unwrap();
bytes[at..at + 10].copy_from_slice(b"/Root 1 1 ");
let info = inspect(Arc::new(bytes), None).unwrap();
assert_eq!((info.pages.len(), info.title.as_deref()), (1, Some("Kept")));
}
#[test]
+1 -1
View File
File diff suppressed because one or more lines are too long
+10 -2
View File
@@ -1630,10 +1630,15 @@ impl Reader<'_> {
/// 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)> {
// PdfCraft patch: (#2365) an in-file /Root must also match its generation, as
// `get_offset` requires when the root is loaded.
let usable = |dict: &Dictionary| {
dict.get(b"Root")
.and_then(Object::as_reference)
.is_ok_and(|root| xref.entries.contains_key(&root.0))
.is_ok_and(|root| match xref.entries.get(&root.0) {
Some(XrefEntry::Normal { generation, .. }) => *generation == root.1,
entry => entry.is_some(),
})
};
// 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;
@@ -1642,9 +1647,12 @@ impl Reader<'_> {
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 {
let Some(XrefEntry::Normal { offset, generation }) = xref.entries.get(&root.0) else {
return false;
};
if *generation != root.1 {
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())