aleleba/pdfcraft
1
0
Fork 0
mirror of https://github.com/storytold/pdfcraft.git synced 2026-11-09 18:01:08 -06:00
Code Issues Packages Projects Releases Wiki Activity

Repair scan: an object whose header names another generation is still found (#5580) (#5584)

Browse Source 
* A rebuilt xref finds an object whose header names another generation (#5580)

`poppler-85140-0.pdf` is 340 bytes with no cross-reference table, and the
header of its one page 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.

The generation is not really the point. `3 99999 obj` with the same page tree
is refused in exactly the same way on main -- the control for the new test
fails on that row first, with "page 1 does not exist" -- so this is a
mismatched generation in a rebuilt table, not an integer overflow. A table
rebuilt by scanning the file has no revision history to tell two generations of
an object apart, and a file that needs rebuilding references everything as
`N 0 R` whatever its headers say.

So, in the reconstruction path only:

  - a generation number that cannot be read is taken as 0, the value such a
    file's references use (`read_object_header`). A generation is at most
    65,535 (ISO 32000-2 §7.5.4), so there is nothing to lose by it;
  - every scanned object is also filed under generation 0, with `or_insert` so
    a real `N 0 obj` is never displaced by the alias;
  - the object is read back behind the same lenient header, since reading it
    parses the header again and the strict identifier check rejected it
    (`read_repaired_body`, gated on `repaired`).

Reading numbers anywhere else is unchanged, and a table read from the file's
own xref keeps the exact identifier match. The scan already files objects found
in object streams under generation 0, and cos's table parser already takes an
unreadable generation as 0, so this follows what the code does elsewhere.

The test covers generations 0, 99999 and 2^64 with no table and no `startxref`,
and asserts the page is found at its own size. Its fixture is built in the test
rather than taken from the corpus, so it holds without the pinned checkout.

* The generation-0 stand-in is one key, not a second object

Two consequences of filing a scanned object under generation 0 as well,
both from review.

`XRef::objects` walks the table's keys, so an object with a stand-in came out
once per key: a document whose page tree cannot be read is recovered by looking
for objects that are pages, and it reported the page twice. Enumeration now
yields one object per location, after the sort it already does. Both keys stay
for lookups; only this walk is deduplicated.

And `or_insert` never replaces, so a later `3 1 obj` moved the exact `3 1` key
to the later body while the `3 0` stand-in stayed on the earlier one -- `3 0 R`
and `3 1 R` would then read different objects. The scan now tracks which
generation-0 keys are stand-ins: a real `N 0 obj` takes the key and keeps it,
and a later header for the same number replaces a stand-in.

The tests check both through what comes out: page 2 must not exist for a
single-page file recovered without a page tree, and with two `3 1 obj` headers
the page comes back at the later body's 300x150 rather than the earlier
200x100.

* M2: the repair scan keeps its place on a non-header, and an object stream's first member is enumerated (#5580)

- read_object_header's lenient path works on a clone, so `42 3 0 obj` no longer costs the
  header after the stray number (CodeRabbit, minor).
- XRef::objects deduplicates by location and object number: an object stream and its first
  member share (offset, 0) and are two objects (CodeRabbit, minor).
- vendor/README.md: hayro-syntax patch (34) and its tests.

Both new tests fail with their fix reverted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Brandon Thomas <echelon@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Hamza Naveed
2026-11-02 05:02:12 -05:00
committed by GitHub
co-authored by Claude Opus 5.5 Brandon Thomas
parent d3384340a6
commit 4a0b1dd158
3 changed files with 217 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/render/src/raster/tests.rs
+93
View File
@@ -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]
vendor/README.md
Vendored
+1 -1
View File
File diff suppressed because one or more lines are too long
vendor/hayro-syntax/src/xref.rs
Vendored
+123 -2
View File
@@ -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();