From be8345aa3f2d9930201e766a4c8a8f315a48f8d6 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Mon, 26 Oct 2026 21:57:29 +0100 Subject: [PATCH 1/4] perf(cos): rebuild a damaged cross-reference index without parsing every object Reconstruct parsed every object to see whether it is an object stream, and searched each object's next 1,024 bytes and the whole file byte by byte. Only objects whose dictionary holds ObjStm or a # (a name can hide letters as #xx) are now parsed for that check. Every /XRef position is found once, and find() scans for the needle's rarest byte in 32-byte blocks. The index, trailer and repair log are unchanged. Fixes #4868 Co-Authored-By: Claude Sonnet 5.5 --- crates/cos/src/document.rs | 72 ++++++++++++++++++++++++++++-- crates/cos/tests/it/main.rs | 1 + crates/cos/tests/it/reconstruct.rs | 57 +++++++++++++++++++++++ 3 files changed, 126 insertions(+), 4 deletions(-) create mode 100644 crates/cos/tests/it/reconstruct.rs diff --git a/crates/cos/src/document.rs b/crates/cos/src/document.rs index 3c7f5fbe18..4ad24a11b1 100644 --- a/crates/cos/src/document.rs +++ b/crates/cos/src/document.rs @@ -2052,10 +2052,29 @@ impl Document { // Offsets of objects that may be cross-reference streams: their dictionaries are the // trailers of files that have no `trailer` keyword (§7.5.8). let mut xref_streams = Vec::new(); - for (num, offset, generation, body) in object_headers(&data) { + // Objects whose bytes (the last object of a number wins, as in `entries`) could hold an + // `/ObjStm` type; the others are not parsed for the object-stream check below. + let mut candidates: HashSet = HashSet::new(); + let headers = object_headers(&data); + // Where `/XRef` occurs in the file, found in one pass. + let mut xref_marks = Vec::new(); + let mut mark_from = 0; + while let Some(p) = find(&data, b"/XRef", mark_from, data.len()) { + xref_marks.push(p); + mark_from = p + 1; + } + for (i, &(num, offset, generation, body)) in headers.iter().enumerate() { entries.insert(num, XrefEntry::InFile { offset, generation }); objstms.push(num); - if find(&data, b"/XRef", body, body.saturating_add(1024)).is_some() + let end = headers.get(i + 1).and_then(|h| usize::try_from(h.1).ok()).unwrap_or(data.len()); + if may_be_object_stream(&data, body, end) { + candidates.insert(num); + } else { + candidates.remove(&num); + } + // `/XRef` within the 1024 bytes after the header (the first one that is there). + let first = xref_marks.partition_point(|&p| p < body); + if xref_marks.get(first).is_some_and(|&p| p + 5 <= body.saturating_add(1024).min(data.len())) && let Ok(at) = usize::try_from(offset) { xref_streams.push(at); @@ -2068,7 +2087,8 @@ impl Document { self.cache.lock().map_err(|_| CosError::Poisoned)?.clear(); // Objects inside object streams. for num in objstms { - if let Ok(o) = self.try_get(num) + if candidates.contains(&num) + && let Ok(o) = self.try_get(num) && let Object::Stream(s) = &*o && s.dict.name(b"Type") == Some(b"ObjStm") && let Ok(stm) = self.objstm(num) @@ -2165,6 +2185,24 @@ fn shifted_subsection_end(data: &[u8], at: usize) -> Option { } } +/// Whether the object whose body is `data[from..to]` could be an object stream: its dictionary +/// holds `ObjStm`, or a `#` (a name may hide its letters as `#xx`, as in `/Obj#53tm`). A stream's +/// data is not searched: it ends the dictionary, at the `stream` keyword after `>>`. +fn may_be_object_stream(data: &[u8], from: usize, to: usize) -> bool { + let Some(body) = data.get(from..to.min(data.len())) else { return false }; + let mut dict = body; + let mut at = 0; + while let Some(p) = find(body, b"stream", at, body.len()) { + at = p + 6; + let after_dict = body[..p].trim_ascii_end().ends_with(b">>"); + if after_dict && matches!(body.get(at), Some(b'\r' | b'\n')) { + dict = &body[..p]; + break; + } + } + dict.contains(&b'#') || find(dict, b"ObjStm", 0, dict.len()).is_some() +} + /// Every `N G obj` header in `data`, in file order, as (number, offset, generation, position /// after `obj`). fn object_headers(data: &[u8]) -> Vec<(u32, u64, u16, usize)> { @@ -2241,7 +2279,33 @@ pub(crate) fn find(hay: &[u8], needle: &[u8], from: usize, to: usize) -> Option< if from >= to || needle.is_empty() || to - from < needle.len() { return None; } - hay[from..to].windows(needle.len()).position(|w| w == needle).map(|p| p + from) + let hay = &hay[from..to]; + // Look for the needle's rarest byte first, a plain byte scan, and compare the rest only there. + const COMMON: &[u8] = b" etaoinsrhldcumfpgwybvkxjqz"; + let (k, rare) = + needle.iter().copied().enumerate().max_by_key(|&(_, b)| COMMON.iter().position(|&c| c == b).unwrap_or(COMMON.len())).unwrap_or((0, 0)); + let mut i = k; + while let Some(p) = hay.get(i..).and_then(|rest| position_of(rest, rare)) { + let start = i + p - k; + if hay.get(start..start + needle.len()) == Some(needle) { + return Some(start + from); + } + i += p + 1; + } + None +} + +/// The first position of `byte` in `hay`. Whole 32-byte blocks are tested without branching, which +/// the compiler turns into vector compares; only a block that holds the byte is searched. +fn position_of(hay: &[u8], byte: u8) -> Option { + const BLOCK: usize = 32; + let (blocks, rest) = hay.as_chunks::(); + for (i, block) in blocks.iter().enumerate() { + if block.iter().fold(false, |any, &b| any | (b == byte)) { + return block.iter().position(|&b| b == byte).map(|p| i * BLOCK + p); + } + } + rest.iter().position(|&b| b == byte).map(|p| blocks.len() * BLOCK + p) } fn rfind(hay: &[u8], needle: &[u8], from: usize) -> Option { diff --git a/crates/cos/tests/it/main.rs b/crates/cos/tests/it/main.rs index 52cf05d6c2..07883e7b5a 100644 --- a/crates/cos/tests/it/main.rs +++ b/crates/cos/tests/it/main.rs @@ -4,5 +4,6 @@ mod bounded_writer; mod corpus; mod encryption; mod proptests; +mod reconstruct; mod shared_streams; mod writer; diff --git a/crates/cos/tests/it/reconstruct.rs b/crates/cos/tests/it/reconstruct.rs new file mode 100644 index 0000000000..5f61867b1b --- /dev/null +++ b/crates/cos/tests/it/reconstruct.rs @@ -0,0 +1,57 @@ +//! Rebuilding the cross-reference index of a damaged file finds the objects inside object +//! streams however the stream's type is written. + +use std::sync::Arc; + +use pdfcraft_cos::{Document, Object}; + +/// A file whose `startxref` points nowhere, with three object streams (type written plainly, as +/// `/Obj#53tm`, and after a string that contains the word `stream`), and one ordinary stream +/// whose data mentions `ObjStm` and `#`. +fn damaged() -> Vec { + let mut out = b"%PDF-1.5\n".to_vec(); + let mut add = |n: u32, body: &[u8]| { + out.extend_from_slice(format!("{n} 0 obj\n").as_bytes()); + out.extend_from_slice(body); + out.extend_from_slice(b"\nendobj\n"); + }; + let objstm = |dict_prefix: &str, members: &[(u32, &str)]| { + let mut header = String::new(); + let mut body = String::new(); + for (n, text) in members { + header.push_str(&format!("{n} {} ", body.len())); + body.push_str(text); + body.push('\n'); + } + let mut b = + format!("<< {dict_prefix} /N {} /First {} /Length {} >>\nstream\n", members.len(), header.len(), header.len() + body.len()).into_bytes(); + b.extend_from_slice(header.as_bytes()); + b.extend_from_slice(body.as_bytes()); + b.extend_from_slice(b"\nendstream"); + b + }; + add(1, b"<< /Type /Catalog /Pages 2 0 R >>"); + add(2, b"<< /Type /Pages /Kids [] /Count 0 >>"); + add(3, &objstm("/Type /ObjStm", &[(10, "<< /From 3 >>"), (11, "[3]")])); + add(4, &objstm("/Type /Obj#53tm", &[(12, "<< /From 4 >>")])); + add(5, &objstm("/Note (a stream) /Type /ObjStm", &[(13, "<< /From 5 >>")])); + let data = b"/Type /ObjStm # 14 0 << /From 6 >>"; + let mut plain = format!("<< /Length {} >>\nstream\n", data.len()).into_bytes(); + plain.extend_from_slice(data); + plain.extend_from_slice(b"\nendstream"); + add(6, &plain); + out.extend_from_slice(b"trailer\n<< /Size 20 /Root 1 0 R >>\nstartxref\n99999999\n%%EOF\n"); + out +} + +#[test] +fn objects_in_object_streams_are_found_however_the_type_is_written() { + let doc = Document::open(Arc::new(damaged())).expect("opens"); + assert!(doc.reconstructed()); + for n in [10, 11, 12, 13] { + let o = doc.try_get(n).unwrap_or_else(|e| panic!("object {n}: {e}")); + assert!(matches!(&*o, Object::Dict(_) | Object::Array(_)), "object {n} is {o:?}"); + } + // The data of an ordinary stream is not an object stream, whatever it says. + assert!(!doc.object_numbers().contains(&14)); +} From 8bdd2d37504689fbff50eb198740d384a6a2c054 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Tue, 27 Oct 2026 19:46:44 +0100 Subject: [PATCH 2/4] fix(cos): read the dictionary's syntax before the object-stream pre-check The pre-check cut the dictionary at the next object header or at a '>>' + 'stream' byte pattern, so text inside a string could hide a later /Type /ObjStm. It now reads the dictionary as the parser does (strings, hex strings, comments, nesting) and treats an object whose end is not certain as a candidate, so the reconstructed object set stays the same. Co-Authored-By: Claude Sonnet 5.5 --- crates/cos/src/document.rs | 173 ++++++++++++++++++++++++++--- crates/cos/tests/it/reconstruct.rs | 7 +- 2 files changed, 162 insertions(+), 18 deletions(-) diff --git a/crates/cos/src/document.rs b/crates/cos/src/document.rs index 4ad24a11b1..ffc4f6be72 100644 --- a/crates/cos/src/document.rs +++ b/crates/cos/src/document.rs @@ -2063,11 +2063,10 @@ impl Document { xref_marks.push(p); mark_from = p + 1; } - for (i, &(num, offset, generation, body)) in headers.iter().enumerate() { + for &(num, offset, generation, body) in &headers { entries.insert(num, XrefEntry::InFile { offset, generation }); objstms.push(num); - let end = headers.get(i + 1).and_then(|h| usize::try_from(h.1).ok()).unwrap_or(data.len()); - if may_be_object_stream(&data, body, end) { + if may_be_object_stream(&data, body) { candidates.insert(num); } else { candidates.remove(&num); @@ -2185,22 +2184,107 @@ fn shifted_subsection_end(data: &[u8], at: usize) -> Option { } } -/// Whether the object whose body is `data[from..to]` could be an object stream: its dictionary -/// holds `ObjStm`, or a `#` (a name may hide its letters as `#xx`, as in `/Obj#53tm`). A stream's -/// data is not searched: it ends the dictionary, at the `stream` keyword after `>>`. -fn may_be_object_stream(data: &[u8], from: usize, to: usize) -> bool { - let Some(body) = data.get(from..to.min(data.len())) else { return false }; - let mut dict = body; - let mut at = 0; - while let Some(p) = find(body, b"stream", at, body.len()) { - at = p + 6; - let after_dict = body[..p].trim_ascii_end().ends_with(b">>"); - if after_dict && matches!(body.get(at), Some(b'\r' | b'\n')) { - dict = &body[..p]; - break; +/// Longest dictionary [`may_be_object_stream`] reads before it gives up and says "maybe". +const DICTIONARY_SCAN_LIMIT: usize = 1 << 20; + +/// Where the dictionary that starts at `data[from..]` (after `N G obj`) ends, reading it as the +/// parser does: strings, hex strings, comments, names and nested dictionaries. `Some(None)`: the +/// object is not a dictionary, so it is never a stream. `None`: the end is not certain (no end +/// within the limit, a hex string with a bad digit, a `>>` inside an array); the caller must +/// then treat the object as a possible object stream. +fn dictionary_end(data: &[u8], from: usize) -> Option> { + let limit = data.len().min(from.saturating_add(DICTIONARY_SCAN_LIMIT)); + let mut i = from; + while i < limit && (is_whitespace(data[i]) || data[i] == b'%') { + if data[i] == b'%' { + while i < limit && !matches!(data[i], b'\n' | b'\r') { + i += 1; + } + } else { + i += 1; + } + } + if data.get(i..i + 2) != Some(b"<<") { + return if i < limit { Some(None) } else { None }; + } + i += 2; + let (mut dicts, mut arrays) = (1usize, 0usize); + while i < limit { + match data[i] { + b'(' => { + let mut depth = 1usize; + i += 1; + while i < limit && depth > 0 { + match data[i] { + b'\\' => i += 1, + b'(' => depth += 1, + b')' => depth -= 1, + _ => {} + } + i += 1; + } + if depth > 0 { + return None; + } + } + b'<' if data.get(i + 1) == Some(&b'<') => { + dicts += 1; + i += 2; + } + b'<' => { + i += 1; + while i < limit && data[i] != b'>' { + if !data[i].is_ascii_hexdigit() && !is_whitespace(data[i]) { + return None; + } + i += 1; + } + i += 1; + } + b'>' if data.get(i + 1) == Some(&b'>') => { + if arrays > 0 { + return None; + } + dicts -= 1; + i += 2; + if dicts == 0 { + return Some(Some(i)); + } + } + b'%' => { + while i < limit && !matches!(data[i], b'\n' | b'\r') { + i += 1; + } + } + b'[' => { + arrays += 1; + i += 1; + } + b']' => { + arrays = arrays.saturating_sub(1); + i += 1; + } + _ => i += 1, + } + } + None +} + +/// Whether the object whose body starts at `data[from..]` could be an object stream: its +/// dictionary holds `ObjStm`, or a `#` (a name may hide its letters as `#xx`, as in +/// `/Obj#53tm`). Only the dictionary is searched, not the data of a stream. Where the +/// dictionary ends is read by [`dictionary_end`], so text inside a string (an object header, a +/// `stream` keyword) is not taken for syntax; when that is not certain the object counts as a +/// candidate, so a doubtful object is parsed as it always was. +fn may_be_object_stream(data: &[u8], from: usize) -> bool { + match dictionary_end(data, from) { + Some(None) => false, + None => true, + Some(Some(end)) => { + let dict = &data[from..end]; + dict.contains(&b'#') || find(dict, b"ObjStm", 0, dict.len()).is_some() } } - dict.contains(&b'#') || find(dict, b"ObjStm", 0, dict.len()).is_some() } /// Every `N G obj` header in `data`, in file order, as (number, offset, generation, position @@ -3584,6 +3668,61 @@ endobj /// cross-reference data, as in #2365), is rebuilt from its object headers. The rebuild used /// to look only for the `trailer` keyword, so /Info and /ID were lost, and a full save after /// editing one Info field dropped every other. + /// Whether parsing `body` as an object gives a stream whose /Type is /ObjStm. + fn parses_as_object_stream(file: &[u8]) -> bool { + matches!(parse_indirect(file, 0, &|_| None), Ok((_, Object::Stream(s))) if s.dict.name(b"Type") == Some(b"ObjStm")) + } + + #[test] + fn the_object_stream_pre_check_never_rejects_what_parses_as_one() { + // Two inputs from review: header-shaped text, and `>>` + `stream` + newline, inside a + // string that comes before the /Type. + for dict in [ + "<< /Note (9 0 obj\n) /Type /ObjStm /N 0 /First 0 >>", + "<< /Note (>>stream\n) /Type /ObjStm /N 0 /First 0 >>", + "<< /Note (a (nested\n9 0 obj\nstream) \\) )) /Type /ObjStm >>", + "<< /Note <3e3e> /Other << /A (>>\nstream\n) >> /Type /ObjStm >>", + ] { + let file = format!("1 0 obj\n{dict}\nstream\nxx\nendstream\nendobj\n").into_bytes(); + assert!(parses_as_object_stream(&file), "test input must be an object stream: {dict}"); + assert!(may_be_object_stream(&file, 7), "{dict}"); + } + // Ordinary streams stay out; their data is never read. + let plain = b"1 0 obj\n<< /Length 9 >>\nstream\nObjStm # \nendstream\nendobj\n"; + assert!(!may_be_object_stream(plain, 7)); + assert!(!may_be_object_stream(b"1 0 obj\n[ /ObjStm ]\nendobj\n", 7)); + // Unterminated: not certain, so a candidate. + assert!(may_be_object_stream(b"1 0 obj\n<< /Note (open ", 7)); + // Random dictionaries built from tricky pieces: whatever parses as an object stream is + // a candidate. + let pieces: [&str; 22] = [ + "/Type /ObjStm", "/Type/ObjStm", "/Type /Obj#53tm", "/T#79pe /ObjStm", "(", ")", "\\", "(>>stream\n)", "(9 0 obj\n)", "<3e3e>", "<", ">", ">>", + "<<", "[", "]", "% c\n", "% >>\n", "/N 0", " ", "\n", "", + ]; + let mut seed = 0x2545_F491_4F6C_DD1Du64; + let mut next = move || { + seed ^= seed << 13; + seed ^= seed >> 7; + seed ^= seed << 17; + seed + }; + let mut hits = 0; + for _ in 0..200_000 { + let mut dict = String::from("<<"); + for _ in 0..(next() % 9) { + dict.push_str(pieces[(next() % pieces.len() as u64) as usize]); + dict.push(' '); + } + dict.push_str(if next() % 8 == 0 { "" } else { ">>" }); + let file = format!("1 0 obj\n{dict}\nstream\nxx\nendstream\nendobj\n").into_bytes(); + if parses_as_object_stream(&file) { + hits += 1; + assert!(may_be_object_stream(&file, 7), "rejected an object stream: {dict:?}"); + } + } + assert!(hits > 1000, "the generator must produce object streams ({hits})"); + } + #[test] fn reconstruction_takes_the_trailer_from_a_cross_reference_stream() { let table = build( diff --git a/crates/cos/tests/it/reconstruct.rs b/crates/cos/tests/it/reconstruct.rs index 5f61867b1b..4611c0d17e 100644 --- a/crates/cos/tests/it/reconstruct.rs +++ b/crates/cos/tests/it/reconstruct.rs @@ -40,6 +40,11 @@ fn damaged() -> Vec { plain.extend_from_slice(data); plain.extend_from_slice(b"\nendstream"); add(6, &plain); + // Strings before the type that look like an object header, or like the end of the + // dictionary and the `stream` keyword, and a hex string and a comment. + add(7, &objstm("/Note (\n99 0 obj\n) /Type /ObjStm", &[(15, "<< /From 7 >>")])); + add(8, &objstm("/Note (>>stream\n) /Type /ObjStm", &[(16, "<< /From 8 >>")])); + add(9, &objstm("/Note <3e3e> /Other << /A (a (b) \\) >>\nstream\n) >> % >>\nstream\n\n/Type /ObjStm", &[(17, "<< /From 9 >>")])); out.extend_from_slice(b"trailer\n<< /Size 20 /Root 1 0 R >>\nstartxref\n99999999\n%%EOF\n"); out } @@ -48,7 +53,7 @@ fn damaged() -> Vec { fn objects_in_object_streams_are_found_however_the_type_is_written() { let doc = Document::open(Arc::new(damaged())).expect("opens"); assert!(doc.reconstructed()); - for n in [10, 11, 12, 13] { + for n in [10, 11, 12, 13, 15, 16, 17] { let o = doc.try_get(n).unwrap_or_else(|e| panic!("object {n}: {e}")); assert!(matches!(&*o, Object::Dict(_) | Object::Array(_)), "object {n} is {o:?}"); } From 8ac51f833b891e29a40e3a8563e53a08c9320fb5 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 16:21:28 +0100 Subject: [PATCH 3/4] cos: charge the object-stream pre-scan to one budget per rebuild Overlapping unterminated dictionaries could each be read up to 1 MiB. A shared budget (the file length) is now taken off on every path; a spent budget keeps the object a candidate, as before. Co-Authored-By: Claude Sonnet 5.5 --- crates/cos/src/document/reader.rs | 163 +++++++++++++++++------------- crates/cos/src/document/tests.rs | 27 ++++- 2 files changed, 112 insertions(+), 78 deletions(-) diff --git a/crates/cos/src/document/reader.rs b/crates/cos/src/document/reader.rs index 7e8a4e3b49..6a325ec933 100644 --- a/crates/cos/src/document/reader.rs +++ b/crates/cos/src/document/reader.rs @@ -623,6 +623,9 @@ impl Document { // `/ObjStm` type; the others are not parsed for the object-stream check below. let mut candidates: HashSet = HashSet::new(); let headers = object_headers(&data); + // What the dictionary pre-check may read in all, so overlapping unterminated dictionaries + // cost no more than one more pass over the file. + let mut scan_budget = data.len(); // Where `/XRef` occurs in the file, found in one pass. let mut xref_marks = Vec::new(); let mut mark_from = 0; @@ -633,7 +636,7 @@ impl Document { for &(num, offset, generation, body) in &headers { entries.insert(num, XrefEntry::InFile { offset, generation }); objstms.push(num); - if may_be_object_stream(&data, body) { + if may_be_object_stream(&data, body, &mut scan_budget) { candidates.insert(num); } else { candidates.remove(&num); @@ -758,83 +761,86 @@ const DICTIONARY_SCAN_LIMIT: usize = 1 << 20; /// parser does: strings, hex strings, comments, names and nested dictionaries. `Some(None)`: the /// object is not a dictionary, so it is never a stream. `None`: the end is not certain (no end /// within the limit, a hex string with a bad digit, a `>>` inside an array); the caller must -/// then treat the object as a possible object stream. -fn dictionary_end(data: &[u8], from: usize) -> Option> { - let limit = data.len().min(from.saturating_add(DICTIONARY_SCAN_LIMIT)); +/// then treat the object as a possible object stream. `limit` ends the read; the second value is +/// how far the read got, for the caller's budget. +fn dictionary_end(data: &[u8], from: usize, limit: usize) -> (Option>, usize) { let mut i = from; - while i < limit && (is_whitespace(data[i]) || data[i] == b'%') { - if data[i] == b'%' { - while i < limit && !matches!(data[i], b'\n' | b'\r') { - i += 1; - } - } else { - i += 1; - } - } - if data.get(i..i + 2) != Some(b"<<") { - return if i < limit { Some(None) } else { None }; - } - i += 2; - let (mut dicts, mut arrays) = (1usize, 0usize); - while i < limit { - match data[i] { - b'(' => { - let mut depth = 1usize; - i += 1; - while i < limit && depth > 0 { - match data[i] { - b'\\' => i += 1, - b'(' => depth += 1, - b')' => depth -= 1, - _ => {} - } - i += 1; - } - if depth > 0 { - return None; - } - } - b'<' if data.get(i + 1) == Some(&b'<') => { - dicts += 1; - i += 2; - } - b'<' => { - i += 1; - while i < limit && data[i] != b'>' { - if !data[i].is_ascii_hexdigit() && !is_whitespace(data[i]) { - return None; - } - i += 1; - } - i += 1; - } - b'>' if data.get(i + 1) == Some(&b'>') => { - if arrays > 0 { - return None; - } - dicts -= 1; - i += 2; - if dicts == 0 { - return Some(Some(i)); - } - } - b'%' => { + let result = 'scan: { + while i < limit && (is_whitespace(data[i]) || data[i] == b'%') { + if data[i] == b'%' { while i < limit && !matches!(data[i], b'\n' | b'\r') { i += 1; } - } - b'[' => { - arrays += 1; + } else { i += 1; } - b']' => { - arrays = arrays.saturating_sub(1); - i += 1; - } - _ => i += 1, } - } - None + if data.get(i..i + 2) != Some(b"<<") { + break 'scan if i < limit { Some(None) } else { None }; + } + i += 2; + let (mut dicts, mut arrays) = (1usize, 0usize); + while i < limit { + match data[i] { + b'(' => { + let mut depth = 1usize; + i += 1; + while i < limit && depth > 0 { + match data[i] { + b'\\' => i += 1, + b'(' => depth += 1, + b')' => depth -= 1, + _ => {} + } + i += 1; + } + if depth > 0 { + break 'scan None; + } + } + b'<' if data.get(i + 1) == Some(&b'<') => { + dicts += 1; + i += 2; + } + b'<' => { + i += 1; + while i < limit && data[i] != b'>' { + if !data[i].is_ascii_hexdigit() && !is_whitespace(data[i]) { + break 'scan None; + } + i += 1; + } + i += 1; + } + b'>' if data.get(i + 1) == Some(&b'>') => { + if arrays > 0 { + break 'scan None; + } + dicts -= 1; + i += 2; + if dicts == 0 { + break 'scan Some(Some(i)); + } + } + b'%' => { + while i < limit && !matches!(data[i], b'\n' | b'\r') { + i += 1; + } + } + b'[' => { + arrays += 1; + i += 1; + } + b']' => { + arrays = arrays.saturating_sub(1); + i += 1; + } + _ => i += 1, + } + } + None + }; + (result, i.min(limit)) } /// Whether the object whose body starts at `data[from..]` could be an object stream: its @@ -843,8 +849,19 @@ fn dictionary_end(data: &[u8], from: usize) -> Option> { /// dictionary ends is read by [`dictionary_end`], so text inside a string (an object header, a /// `stream` keyword) is not taken for syntax; when that is not certain the object counts as a /// candidate, so a doubtful object is parsed as it always was. -pub(super) fn may_be_object_stream(data: &[u8], from: usize) -> bool { - match dictionary_end(data, from) { +/// +/// `budget` is the bytes all the calls of one rebuild may still read. A call reads at most +/// [`DICTIONARY_SCAN_LIMIT`] bytes or what is left, takes what it read off the budget on every +/// path, and says "maybe" once the budget cannot cover it. Dictionaries that do not overlap +/// add up to no more than the file, so only unterminated ones can use the budget up. +pub(super) fn may_be_object_stream(data: &[u8], from: usize, budget: &mut usize) -> bool { + let limit = data.len().min(from.saturating_add(DICTIONARY_SCAN_LIMIT.min(*budget))); + if *budget == 0 { + return true; + } + let (end, read) = dictionary_end(data, from, limit); + *budget = budget.saturating_sub(read.saturating_sub(from).max(1)); + match end { Some(None) => false, None => true, Some(Some(end)) => { diff --git a/crates/cos/src/document/tests.rs b/crates/cos/src/document/tests.rs index d2234fca41..ddd7965eda 100644 --- a/crates/cos/src/document/tests.rs +++ b/crates/cos/src/document/tests.rs @@ -1193,6 +1193,11 @@ fn parses_as_object_stream(file: &[u8]) -> bool { matches!(parse_indirect(file, 0, &|_| None), Ok((_, Object::Stream(s))) if s.dict.name(b"Type") == Some(b"ObjStm")) } +/// A scan budget that never runs out. +fn unlimited() -> usize { + usize::MAX +} + #[test] fn the_object_stream_pre_check_never_rejects_what_parses_as_one() { // Two inputs from review: header-shaped text, and `>>` + `stream` + newline, inside a @@ -1205,14 +1210,26 @@ fn the_object_stream_pre_check_never_rejects_what_parses_as_one() { ] { let file = format!("1 0 obj\n{dict}\nstream\nxx\nendstream\nendobj\n").into_bytes(); assert!(parses_as_object_stream(&file), "test input must be an object stream: {dict}"); - assert!(may_be_object_stream(&file, 7), "{dict}"); + assert!(may_be_object_stream(&file, 7, &mut unlimited()), "{dict}"); } // Ordinary streams stay out; their data is never read. let plain = b"1 0 obj\n<< /Length 9 >>\nstream\nObjStm # \nendstream\nendobj\n"; - assert!(!may_be_object_stream(plain, 7)); - assert!(!may_be_object_stream(b"1 0 obj\n[ /ObjStm ]\nendobj\n", 7)); + assert!(!may_be_object_stream(plain, 7, &mut unlimited())); + assert!(!may_be_object_stream(b"1 0 obj\n[ /ObjStm ]\nendobj\n", 7, &mut unlimited())); // Unterminated: not certain, so a candidate. - assert!(may_be_object_stream(b"1 0 obj\n<< /Note (open ", 7)); + assert!(may_be_object_stream(b"1 0 obj\n<< /Note (open ", 7, &mut unlimited())); + // The scan is charged to the shared budget on every path, and a spent budget says "maybe" + // without reading. + let open = b"1 0 obj\n<< /Note (open "; + let mut budget = 1000; + assert!(may_be_object_stream(open, 7, &mut budget)); + assert_eq!(budget, 1000 - (open.len() - 7)); + let mut budget = 3; + assert!(may_be_object_stream(plain, 7, &mut budget), "a budget too small for the dictionary is a candidate"); + assert_eq!(budget, 0); + let mut budget = 0; + assert!(may_be_object_stream(plain, 7, &mut budget)); + assert_eq!(budget, 0); // Random dictionaries built from tricky pieces: whatever parses as an object stream is // a candidate. let pieces: [&str; 22] = [ @@ -1257,7 +1274,7 @@ fn the_object_stream_pre_check_never_rejects_what_parses_as_one() { let file = format!("1 0 obj\n{dict}\nstream\nxx\nendstream\nendobj\n").into_bytes(); if parses_as_object_stream(&file) { hits += 1; - assert!(may_be_object_stream(&file, 7), "rejected an object stream: {dict:?}"); + assert!(may_be_object_stream(&file, 7, &mut unlimited()), "rejected an object stream: {dict:?}"); } } assert!(hits > 1000, "the generator must produce object streams ({hits})"); From f19ecc4635a0a9e99d22dd032d8b24ec3438fdde Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 18:49:31 +0100 Subject: [PATCH 4/4] Merge upstream/main; drop the object-stream pre-check from reconstruct Main now reports unreadable objects from the parse of every object in Document::reconstruct, so the pre-check that skipped that parse is gone. The single-pass /XRef positions and the rarest-byte find stay. Co-Authored-By: Claude Sonnet 5.5 --- crates/cos/src/document/reader.rs | 131 +---------------------------- crates/cos/src/document/tests.rs | 119 ++++++-------------------- crates/cos/tests/it/main.rs | 1 - crates/cos/tests/it/reconstruct.rs | 62 -------------- 4 files changed, 27 insertions(+), 286 deletions(-) delete mode 100644 crates/cos/tests/it/reconstruct.rs diff --git a/crates/cos/src/document/reader.rs b/crates/cos/src/document/reader.rs index 4b2da4fc8b..18f5ce848a 100644 --- a/crates/cos/src/document/reader.rs +++ b/crates/cos/src/document/reader.rs @@ -635,13 +635,7 @@ impl Document { // Offsets of objects that may be cross-reference streams: their dictionaries are the // trailers of files that have no `trailer` keyword (§7.5.8). let mut xref_streams = Vec::new(); - // Objects whose bytes (the last object of a number wins, as in `entries`) could hold an - // `/ObjStm` type; the others are not parsed for the object-stream check below. - let mut candidates: HashSet = HashSet::new(); let headers = object_headers(&data); - // What the dictionary pre-check may read in all, so overlapping unterminated dictionaries - // cost no more than one more pass over the file. - let mut scan_budget = data.len(); // Where `/XRef` occurs in the file, found in one pass. let mut xref_marks = Vec::new(); let mut mark_from = 0; @@ -652,11 +646,6 @@ impl Document { for &(num, offset, generation, body) in &headers { entries.insert(num, XrefEntry::InFile { offset, generation }); objstms.push(num); - if may_be_object_stream(&data, body, &mut scan_budget) { - candidates.insert(num); - } else { - candidates.remove(&num); - } // `/XRef` within the 1024 bytes after the header (the first one that is there). let first = xref_marks.partition_point(|&p| p < body); if xref_marks.get(first).is_some_and(|&p| p + 5 <= body.saturating_add(1024).min(data.len())) @@ -673,8 +662,7 @@ impl Document { *self.unreadable.lock().map_err(|_| CosError::Poisoned)? = Unreadable::default(); // Objects inside object streams. for num in objstms { - if candidates.contains(&num) - && let Ok(o) = self.try_get(num) + if let Ok(o) = self.try_get(num) && let Object::Stream(s) = &*o && s.dict.name(b"Type") == Some(b"ObjStm") && let Ok(stm) = self.objstm(num) @@ -773,123 +761,6 @@ fn shifted_subsection_end(data: &[u8], at: usize) -> Option { } } -/// Longest dictionary [`may_be_object_stream`] reads before it gives up and says "maybe". -const DICTIONARY_SCAN_LIMIT: usize = 1 << 20; - -/// Where the dictionary that starts at `data[from..]` (after `N G obj`) ends, reading it as the -/// parser does: strings, hex strings, comments, names and nested dictionaries. `Some(None)`: the -/// object is not a dictionary, so it is never a stream. `None`: the end is not certain (no end -/// within the limit, a hex string with a bad digit, a `>>` inside an array); the caller must -/// then treat the object as a possible object stream. `limit` ends the read; the second value is -/// how far the read got, for the caller's budget. -fn dictionary_end(data: &[u8], from: usize, limit: usize) -> (Option>, usize) { - let mut i = from; - let result = 'scan: { - while i < limit && (is_whitespace(data[i]) || data[i] == b'%') { - if data[i] == b'%' { - while i < limit && !matches!(data[i], b'\n' | b'\r') { - i += 1; - } - } else { - i += 1; - } - } - if data.get(i..i + 2) != Some(b"<<") { - break 'scan if i < limit { Some(None) } else { None }; - } - i += 2; - let (mut dicts, mut arrays) = (1usize, 0usize); - while i < limit { - match data[i] { - b'(' => { - let mut depth = 1usize; - i += 1; - while i < limit && depth > 0 { - match data[i] { - b'\\' => i += 1, - b'(' => depth += 1, - b')' => depth -= 1, - _ => {} - } - i += 1; - } - if depth > 0 { - break 'scan None; - } - } - b'<' if data.get(i + 1) == Some(&b'<') => { - dicts += 1; - i += 2; - } - b'<' => { - i += 1; - while i < limit && data[i] != b'>' { - if !data[i].is_ascii_hexdigit() && !is_whitespace(data[i]) { - break 'scan None; - } - i += 1; - } - i += 1; - } - b'>' if data.get(i + 1) == Some(&b'>') => { - if arrays > 0 { - break 'scan None; - } - dicts -= 1; - i += 2; - if dicts == 0 { - break 'scan Some(Some(i)); - } - } - b'%' => { - while i < limit && !matches!(data[i], b'\n' | b'\r') { - i += 1; - } - } - b'[' => { - arrays += 1; - i += 1; - } - b']' => { - arrays = arrays.saturating_sub(1); - i += 1; - } - _ => i += 1, - } - } - None - }; - (result, i.min(limit)) -} - -/// Whether the object whose body starts at `data[from..]` could be an object stream: its -/// dictionary holds `ObjStm`, or a `#` (a name may hide its letters as `#xx`, as in -/// `/Obj#53tm`). Only the dictionary is searched, not the data of a stream. Where the -/// dictionary ends is read by [`dictionary_end`], so text inside a string (an object header, a -/// `stream` keyword) is not taken for syntax; when that is not certain the object counts as a -/// candidate, so a doubtful object is parsed as it always was. -/// -/// `budget` is the bytes all the calls of one rebuild may still read. A call reads at most -/// [`DICTIONARY_SCAN_LIMIT`] bytes or what is left, takes what it read off the budget on every -/// path, and says "maybe" once the budget cannot cover it. Dictionaries that do not overlap -/// add up to no more than the file, so only unterminated ones can use the budget up. -pub(super) fn may_be_object_stream(data: &[u8], from: usize, budget: &mut usize) -> bool { - let limit = data.len().min(from.saturating_add(DICTIONARY_SCAN_LIMIT.min(*budget))); - if *budget == 0 { - return true; - } - let (end, read) = dictionary_end(data, from, limit); - *budget = budget.saturating_sub(read.saturating_sub(from).max(1)); - match end { - Some(None) => false, - None => true, - Some(Some(end)) => { - let dict = &data[from..end]; - dict.contains(&b'#') || find(dict, b"ObjStm", 0, dict.len()).is_some() - } - } -} - /// Every `N G obj` header in `data`, in file order, as (number, offset, generation, position /// after `obj`). fn object_headers(data: &[u8]) -> Vec<(u32, u64, u16, usize)> { diff --git a/crates/cos/src/document/tests.rs b/crates/cos/src/document/tests.rs index 59221dc0ce..f69ffae042 100644 --- a/crates/cos/src/document/tests.rs +++ b/crates/cos/src/document/tests.rs @@ -1,4 +1,3 @@ -use super::reader::may_be_object_stream; use super::*; std::thread_local! { @@ -1920,98 +1919,6 @@ fn a_prev_reference_to_an_object_holding_the_offset_reads_that_offset() { /// cross-reference data, as in #2365), is rebuilt from its object headers. The rebuild used /// to look only for the `trailer` keyword, so /Info and /ID were lost, and a full save after /// editing one Info field dropped every other. -/// Whether parsing `body` as an object gives a stream whose /Type is /ObjStm. -fn parses_as_object_stream(file: &[u8]) -> bool { - matches!(parse_indirect(file, 0, &|_| None), Ok((_, Object::Stream(s))) if s.dict.name(b"Type") == Some(b"ObjStm")) -} - -/// A scan budget that never runs out. -fn unlimited() -> usize { - usize::MAX -} - -#[test] -fn the_object_stream_pre_check_never_rejects_what_parses_as_one() { - // Two inputs from review: header-shaped text, and `>>` + `stream` + newline, inside a - // string that comes before the /Type. - for dict in [ - "<< /Note (9 0 obj\n) /Type /ObjStm /N 0 /First 0 >>", - "<< /Note (>>stream\n) /Type /ObjStm /N 0 /First 0 >>", - "<< /Note (a (nested\n9 0 obj\nstream) \\) )) /Type /ObjStm >>", - "<< /Note <3e3e> /Other << /A (>>\nstream\n) >> /Type /ObjStm >>", - ] { - let file = format!("1 0 obj\n{dict}\nstream\nxx\nendstream\nendobj\n").into_bytes(); - assert!(parses_as_object_stream(&file), "test input must be an object stream: {dict}"); - assert!(may_be_object_stream(&file, 7, &mut unlimited()), "{dict}"); - } - // Ordinary streams stay out; their data is never read. - let plain = b"1 0 obj\n<< /Length 9 >>\nstream\nObjStm # \nendstream\nendobj\n"; - assert!(!may_be_object_stream(plain, 7, &mut unlimited())); - assert!(!may_be_object_stream(b"1 0 obj\n[ /ObjStm ]\nendobj\n", 7, &mut unlimited())); - // Unterminated: not certain, so a candidate. - assert!(may_be_object_stream(b"1 0 obj\n<< /Note (open ", 7, &mut unlimited())); - // The scan is charged to the shared budget on every path, and a spent budget says "maybe" - // without reading. - let open = b"1 0 obj\n<< /Note (open "; - let mut budget = 1000; - assert!(may_be_object_stream(open, 7, &mut budget)); - assert_eq!(budget, 1000 - (open.len() - 7)); - let mut budget = 3; - assert!(may_be_object_stream(plain, 7, &mut budget), "a budget too small for the dictionary is a candidate"); - assert_eq!(budget, 0); - let mut budget = 0; - assert!(may_be_object_stream(plain, 7, &mut budget)); - assert_eq!(budget, 0); - // Random dictionaries built from tricky pieces: whatever parses as an object stream is - // a candidate. - let pieces: [&str; 22] = [ - "/Type /ObjStm", - "/Type/ObjStm", - "/Type /Obj#53tm", - "/T#79pe /ObjStm", - "(", - ")", - "\\", - "(>>stream\n)", - "(9 0 obj\n)", - "<3e3e>", - "<", - ">", - ">>", - "<<", - "[", - "]", - "% c\n", - "% >>\n", - "/N 0", - " ", - "\n", - "", - ]; - let mut seed = 0x2545_F491_4F6C_DD1Du64; - let mut next = move || { - seed ^= seed << 13; - seed ^= seed >> 7; - seed ^= seed << 17; - seed - }; - let mut hits = 0; - for _ in 0..200_000 { - let mut dict = String::from("<<"); - for _ in 0..(next() % 9) { - dict.push_str(pieces[(next() % pieces.len() as u64) as usize]); - dict.push(' '); - } - dict.push_str(if next() % 8 == 0 { "" } else { ">>" }); - let file = format!("1 0 obj\n{dict}\nstream\nxx\nendstream\nendobj\n").into_bytes(); - if parses_as_object_stream(&file) { - hits += 1; - assert!(may_be_object_stream(&file, 7, &mut unlimited()), "rejected an object stream: {dict:?}"); - } - } - assert!(hits > 1000, "the generator must produce object streams ({hits})"); -} - #[test] fn reconstruction_takes_the_trailer_from_a_cross_reference_stream() { let table = build( @@ -2282,3 +2189,29 @@ fn the_editor_cache_charges_a_dictionary_for_the_values_it_holds() { let sum = cache.values().fold(0usize, |n, o| n.saturating_add(held_bytes(o, &doc.data, 0))); assert_eq!(cache.owned(), sum, "the kept total is the cache's own"); } + +#[test] +fn find_agrees_with_a_plain_window_search() { + let naive = |hay: &[u8], needle: &[u8], from: usize, to: usize| { + if from >= to || needle.is_empty() || to - from < needle.len() { + return None; + } + hay[from..to].windows(needle.len()).position(|w| w == needle).map(|p| p + from) + }; + let mut seed = 0x9E37_79B9_7F4A_7C15u64; + let mut next = move || { + seed ^= seed << 13; + seed ^= seed >> 7; + seed ^= seed << 17; + seed + }; + let alphabet = b"/XRef t\n<>0"; + for _ in 0..20_000 { + let hay: Vec = (0..next() % 200).map(|_| alphabet[(next() % alphabet.len() as u64) as usize]).collect(); + let needles: [&[u8]; 5] = [b"/XRef", b"trailer", b"R", b"ff", b"<<"]; + let needle = needles[(next() % 5) as usize]; + let from = (next() % (hay.len() as u64 + 1)) as usize; + let to = from + (next() % (hay.len() as u64 + 1 - from as u64)) as usize; + assert_eq!(find(&hay, needle, from, to), naive(&hay, needle, from, to), "{hay:?} {needle:?} {from} {to}"); + } +} diff --git a/crates/cos/tests/it/main.rs b/crates/cos/tests/it/main.rs index d0b73b16d4..1bd7ee1fd6 100644 --- a/crates/cos/tests/it/main.rs +++ b/crates/cos/tests/it/main.rs @@ -5,6 +5,5 @@ mod corpus; mod encryption; mod generations; mod proptests; -mod reconstruct; mod shared_streams; mod writer; diff --git a/crates/cos/tests/it/reconstruct.rs b/crates/cos/tests/it/reconstruct.rs deleted file mode 100644 index 4611c0d17e..0000000000 --- a/crates/cos/tests/it/reconstruct.rs +++ /dev/null @@ -1,62 +0,0 @@ -//! Rebuilding the cross-reference index of a damaged file finds the objects inside object -//! streams however the stream's type is written. - -use std::sync::Arc; - -use pdfcraft_cos::{Document, Object}; - -/// A file whose `startxref` points nowhere, with three object streams (type written plainly, as -/// `/Obj#53tm`, and after a string that contains the word `stream`), and one ordinary stream -/// whose data mentions `ObjStm` and `#`. -fn damaged() -> Vec { - let mut out = b"%PDF-1.5\n".to_vec(); - let mut add = |n: u32, body: &[u8]| { - out.extend_from_slice(format!("{n} 0 obj\n").as_bytes()); - out.extend_from_slice(body); - out.extend_from_slice(b"\nendobj\n"); - }; - let objstm = |dict_prefix: &str, members: &[(u32, &str)]| { - let mut header = String::new(); - let mut body = String::new(); - for (n, text) in members { - header.push_str(&format!("{n} {} ", body.len())); - body.push_str(text); - body.push('\n'); - } - let mut b = - format!("<< {dict_prefix} /N {} /First {} /Length {} >>\nstream\n", members.len(), header.len(), header.len() + body.len()).into_bytes(); - b.extend_from_slice(header.as_bytes()); - b.extend_from_slice(body.as_bytes()); - b.extend_from_slice(b"\nendstream"); - b - }; - add(1, b"<< /Type /Catalog /Pages 2 0 R >>"); - add(2, b"<< /Type /Pages /Kids [] /Count 0 >>"); - add(3, &objstm("/Type /ObjStm", &[(10, "<< /From 3 >>"), (11, "[3]")])); - add(4, &objstm("/Type /Obj#53tm", &[(12, "<< /From 4 >>")])); - add(5, &objstm("/Note (a stream) /Type /ObjStm", &[(13, "<< /From 5 >>")])); - let data = b"/Type /ObjStm # 14 0 << /From 6 >>"; - let mut plain = format!("<< /Length {} >>\nstream\n", data.len()).into_bytes(); - plain.extend_from_slice(data); - plain.extend_from_slice(b"\nendstream"); - add(6, &plain); - // Strings before the type that look like an object header, or like the end of the - // dictionary and the `stream` keyword, and a hex string and a comment. - add(7, &objstm("/Note (\n99 0 obj\n) /Type /ObjStm", &[(15, "<< /From 7 >>")])); - add(8, &objstm("/Note (>>stream\n) /Type /ObjStm", &[(16, "<< /From 8 >>")])); - add(9, &objstm("/Note <3e3e> /Other << /A (a (b) \\) >>\nstream\n) >> % >>\nstream\n\n/Type /ObjStm", &[(17, "<< /From 9 >>")])); - out.extend_from_slice(b"trailer\n<< /Size 20 /Root 1 0 R >>\nstartxref\n99999999\n%%EOF\n"); - out -} - -#[test] -fn objects_in_object_streams_are_found_however_the_type_is_written() { - let doc = Document::open(Arc::new(damaged())).expect("opens"); - assert!(doc.reconstructed()); - for n in [10, 11, 12, 13, 15, 16, 17] { - let o = doc.try_get(n).unwrap_or_else(|e| panic!("object {n}: {e}")); - assert!(matches!(&*o, Object::Dict(_) | Object::Array(_)), "object {n} is {o:?}"); - } - // The data of an ordinary stream is not an object stream, whatever it says. - assert!(!doc.object_numbers().contains(&14)); -}