From f19ecc4635a0a9e99d22dd032d8b24ec3438fdde Mon Sep 17 00:00:00 2001 From: vangel76 Date: Sat, 7 Nov 2026 18:49:31 +0100 Subject: [PATCH] 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)); -}