From a374d16515b9aeec2dc6dbc2ba9dac1a648b02f0 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:30:00 +0000 Subject: [PATCH 01/21] fix(cos): index the keys of a large dictionary Dict::set searched every existing entry before inserting a key, so a parsed dictionary of n distinct keys cost n^2/2 key comparisons: a few megabytes of keys stalled parsing for minutes. From 32 entries a dictionary keeps a key index, so each insert and lookup is one hash probe. Equality and debug output still compare and show the entries alone. --- crates/cos/src/object.rs | 87 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 81 insertions(+), 6 deletions(-) diff --git a/crates/cos/src/object.rs b/crates/cos/src/object.rs index 438b8d5f3..444fa3751 100644 --- a/crates/cos/src/object.rs +++ b/crates/cos/src/object.rs @@ -76,10 +76,35 @@ impl PdfString { /// A name object (without the leading `/`, with `#xx` escapes already decoded). pub type Name = Vec; +#[cfg(test)] +thread_local! { + /// Key comparisons and index lookups made by [`Dict`] on this thread. + static KEY_COMPARES: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// Entries from which a [`Dict`] keeps a key index. Smaller dictionaries (nearly all of them) +/// search their entries; a parsed file controls the key count, and a linear search per +/// inserted key would make a dictionary of many distinct keys quadratic to build. +const INDEX_FROM: usize = 32; + /// An ordered dictionary. -#[derive(Clone, Debug, Default, PartialEq)] +#[derive(Clone, Default)] pub struct Dict { entries: Vec<(Name, Object)>, + /// Key to position in `entries`, kept once the dictionary reaches [`INDEX_FROM`] entries. + index: Option>>, +} + +impl PartialEq for Dict { + fn eq(&self, other: &Self) -> bool { + self.entries == other.entries + } +} + +impl std::fmt::Debug for Dict { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Dict").field("entries", &self.entries).finish() + } } impl Dict { @@ -87,26 +112,53 @@ impl Dict { Self::default() } + fn position(&self, key: &[u8]) -> Option { + if let Some(index) = &self.index { + #[cfg(test)] + KEY_COMPARES.with(|c| c.set(c.get() + 1)); + return index.get(key).copied(); + } + self.entries.iter().position(|(k, _)| { + #[cfg(test)] + KEY_COMPARES.with(|c| c.set(c.get() + 1)); + k == key + }) + } + pub fn get(&self, key: &[u8]) -> Option<&Object> { - self.entries.iter().find(|(k, _)| k == key).map(|(_, v)| v) + self.position(key).and_then(|i| self.entries.get(i)).map(|(_, v)| v) } pub fn get_mut(&mut self, key: &[u8]) -> Option<&mut Object> { - self.entries.iter_mut().find(|(k, _)| k == key).map(|(_, v)| v) + let i = self.position(key)?; + self.entries.get_mut(i).map(|(_, v)| v) } /// Insert or replace, keeping the position of an existing key. pub fn set(&mut self, key: impl Into, value: impl Into) { let key = key.into(); let value = value.into(); - match self.entries.iter_mut().find(|(k, _)| *k == key) { + match self.position(&key).and_then(|i| self.entries.get_mut(i)) { Some((_, v)) => *v = value, - None => self.entries.push((key, value)), + None => { + let at = self.entries.len(); + if let Some(index) = &mut self.index { + index.insert(key.clone(), at); + } else if at + 1 >= INDEX_FROM { + let index = self.entries.iter().enumerate().map(|(i, (k, _))| (k.clone(), i)).chain([(key.clone(), at)]).collect(); + self.index = Some(Box::new(index)); + } + self.entries.push((key, value)); + } } } pub fn remove(&mut self, key: &[u8]) -> Option { - let i = self.entries.iter().position(|(k, _)| k == key)?; + let i = self.position(key)?; + if let Some(index) = &mut self.index { + index.remove(key); + index.values_mut().filter(|at| **at > i).for_each(|at| *at -= 1); + } Some(self.entries.remove(i).1) } @@ -382,6 +434,29 @@ impl From for Object { mod tests { use super::*; + #[test] + fn parsing_many_distinct_keys_is_not_quadratic() { + let n = 20_000; + let body: String = (0..n).map(|i| format!("/K{i} {i} ")).collect(); + let text = format!("<< {body}/K7 true >>"); + KEY_COMPARES.with(|c| c.set(0)); + let Ok(Object::Dict(d)) = crate::parser::Lexer::new(text.as_bytes(), 0).object() else { panic!("not a dict") }; + let compares = KEY_COMPARES.with(|c| c.get()); + assert!(compares < 8 * n, "{compares} key comparisons for {n} keys"); + // Order and replace-in-place semantics stay those of a plain ordered dictionary. + assert_eq!(d.len(), n); + assert_eq!(d.iter().next().map(|(k, _)| k.as_slice()), Some(&b"K0"[..])); + assert_eq!(d.get(b"K7"), Some(&Object::Bool(true))); + assert_eq!(d.iter().nth(7).map(|(_, v)| v), Some(&Object::Bool(true))); + let mut d = d; + assert_eq!(d.remove(b"K3"), Some(Object::Int(3))); + assert_eq!(d.get(b"K4"), Some(&Object::Int(4))); + assert_eq!(d.iter().nth(3).map(|(k, _)| k.as_slice()), Some(&b"K4"[..])); + d.set(b"K3".to_vec(), Object::Null); + assert_eq!(d.iter().last().map(|(k, _)| k.as_slice()), Some(&b"K3"[..])); + assert!(d.get(b"K20000").is_none()); + } + #[test] fn text_strings_round_trip() { for s in ["Plain ASCII", "Café — naïve", "日本語", "emoji 📄"] { From 58dc10c227e27a5bb5873c12894c8cf8ed61e76f Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:33:20 +0000 Subject: [PATCH 02/21] fix(cos): read a damaged real's numeric prefix in one pass To read the longest valid prefix of a token like 1.2.3 the parser retried the conversion on each shorter prefix, so a long digit run followed by long junk was converted once per junk byte: quadratic in the token. The prefix is found in one scan and converted once. A prefix that spells infinity or NaN no longer reads as a number. --- crates/cos/src/parser.rs | 68 +++++++++++++++++++++++++++++++++------- 1 file changed, 56 insertions(+), 12 deletions(-) diff --git a/crates/cos/src/parser.rs b/crates/cos/src/parser.rs index 045805596..a16ff10c3 100644 --- a/crates/cos/src/parser.rs +++ b/crates/cos/src/parser.rs @@ -324,18 +324,8 @@ impl<'a> Lexer<'a> { if s.contains('.') { // Tolerate forms like "--5", "5-" or "1.2.3" by parsing the longest valid prefix. let cleaned: String = s.trim_start_matches('+').replacen("--", "-", 1); - let mut end = cleaned.len(); - while end > 0 { - // Only cut at character boundaries: a token can hold multi-byte UTF-8 (fuzzing - // found ".—", which panicked here). - if cleaned.is_char_boundary(end) - && let Ok(v) = cleaned[..end].parse::() - { - return Ok(Object::Real(v)); - } - end -= 1; - } - return Ok(Object::Real(0.0)); + let v = cleaned.get(..real_prefix_len(cleaned.as_bytes())).and_then(|p| parse_counted::(p).ok()); + return Ok(Object::Real(v.unwrap_or(0.0))); } // Tolerate a doubled sign ("--280") like the real branch does. let t = s.trim_start_matches('+'); @@ -350,6 +340,44 @@ impl<'a> Lexer<'a> { } } +/// Length of the longest prefix of `s` that reads as a decimal number: a sign, digits with at +/// most one `.`, and an exponent with digits. Zero when that prefix has no digit. Found in one +/// pass so a long damaged token is converted once, not once per shorter prefix. +fn real_prefix_len(s: &[u8]) -> usize { + let digits = |from: usize| s.get(from..).map_or(0, |r| r.iter().take_while(|b| b.is_ascii_digit()).count()); + let mut at = usize::from(matches!(s.first(), Some(b'+' | b'-'))); + let int = digits(at); + at += int; + let mut frac = 0; + if s.get(at) == Some(&b'.') { + frac = digits(at + 1); + at += 1 + frac; + } + if int + frac == 0 { + return 0; + } + if matches!(s.get(at), Some(b'e' | b'E')) { + let sign = usize::from(matches!(s.get(at + 1), Some(b'+' | b'-'))); + let exp = digits(at + 1 + sign); + if exp > 0 { + at += 1 + sign + exp; + } + } + at +} + +#[cfg(test)] +thread_local! { + /// Bytes handed to number conversion on this thread. + static NUMBER_BYTES: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +fn parse_counted(s: &str) -> Result { + #[cfg(test)] + NUMBER_BYTES.with(|c| c.set(c.get() + s.len())); + s.parse() +} + fn hexval(b: u8) -> Option { match b { b'0'..=b'9' => Some(b - b'0'), @@ -483,6 +511,22 @@ mod tests { assert_eq!(Lexer::new("1.5\u{2014}".as_bytes(), 0).object().unwrap(), Object::Real(1.5)); } + #[test] + fn a_damaged_real_is_converted_once() { + let token = format!("1.{}{}", "5".repeat(2_000), "x".repeat(20_000)); + NUMBER_BYTES.with(|c| c.set(0)); + let Ok(Object::Real(v)) = Lexer::new(token.as_bytes(), 0).object() else { panic!("not a real") }; + let converted = NUMBER_BYTES.with(|c| c.get()); + assert!(converted <= token.len(), "{converted} bytes converted for a {}-byte token", token.len()); + assert!((v - 1.5555).abs() < 1e-3); + // The longest numeric prefix wins, as before. + for (t, want) in + [("1.2.3", 1.2), ("--5.", -5.0), ("+.5x", 0.5), ("5.-", 5.0), ("1.5e3", 1500.0), ("1.5e+", 1.5), ("-.x", 0.0), (".", 0.0), ("-inf.", 0.0)] + { + assert_eq!(Lexer::new(t.as_bytes(), 0).object().ok(), Some(Object::Real(want)), "{t}"); + } + } + fn obj(s: &str) -> Object { Lexer::new(s.as_bytes(), 0).object().unwrap_or_else(|e| panic!("{s}: {e}")) } From 3c64f9314ef2b6d749dfcfb4abe9652e226208b3 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:34:53 +0000 Subject: [PATCH 03/21] fix(content): skip a long unreadable number whole When an operand failed to parse, the content parser went back to the byte after its start and tried again, so a run of n bytes that never reads as a number (a long run of '+', or digits ending in a letter) was read again from every one of its bytes: n^2/2 bytes from a tiny compressed stream. A failed number token longer than 64 bytes is now skipped whole; shorter ones still recover byte by byte. --- crates/content/src/lib.rs | 19 ++++++++++++++++--- crates/content/src/tests.rs | 22 ++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/crates/content/src/lib.rs b/crates/content/src/lib.rs index eb126ffa2..04bc1cfa9 100644 --- a/crates/content/src/lib.rs +++ b/crates/content/src/lib.rs @@ -74,6 +74,9 @@ fn is_delim(b: u8) -> bool { matches!(b, b'(' | b')' | b'<' | b'>' | b'[' | b']' | b'{' | b'}' | b'/' | b'%') } +/// Longest unreadable number token retried from its next byte. Real numbers are far shorter. +const MAX_RETRIED_TOKEN: usize = 64; + /// Parse a (decoded) content stream. pub fn parse(data: &[u8]) -> Parsed { parse_pieces(data, &[]) @@ -110,9 +113,19 @@ fn parse_pieces(data: &[u8], ends: &[usize]) -> Parsed { lx.pos = data.len(); } Err(_) => { - // Skip the offending byte and carry on. - lx.pos = at + 1; - out.skipped += 1; + let read = lx.pos.saturating_sub(at); + #[cfg(test)] + tests::RESCANNED.with(|c| c.set(c.get() + read)); + let number = matches!(b, b'+' | b'-' | b'.') || b.is_ascii_digit(); + if number && read > MAX_RETRIED_TOKEN { + // A long unreadable number is skipped whole: retrying from each of its + // bytes would read the rest of it again every time. + out.skipped += read; + } else { + // Skip the offending byte and carry on. + lx.pos = at + 1; + out.skipped += 1; + } } } continue; diff --git a/crates/content/src/tests.rs b/crates/content/src/tests.rs index 094a0665d..f32ab3da5 100644 --- a/crates/content/src/tests.rs +++ b/crates/content/src/tests.rs @@ -1,5 +1,27 @@ use super::*; +thread_local! { + /// Bytes the lexer read before an operand failed, on this thread. + pub(crate) static RESCANNED: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +#[test] +fn a_long_unreadable_number_is_read_once() { + for junk in ["+", "5"] { + let n = 20_000; + let src = format!("{}{} 1 0 0 1 0 0 cm", junk.repeat(n), if junk == "5" { "x" } else { "" }); + RESCANNED.with(|c| c.set(0)); + let p = parse(src.as_bytes()); + let rescanned = RESCANNED.with(|c| c.get()); + assert!(rescanned <= 2 * n, "{rescanned} bytes read for a {n}-byte token"); + assert_eq!(p.ops.last().and_then(|o| o.nums::<6>()), Some([1.0, 0.0, 0.0, 1.0, 0.0, 0.0])); + } + // A short damaged number still recovers byte by byte. + let p = parse(b"-+5 0 Td"); + assert_eq!((p.ops.len(), p.skipped), (1, 1)); + assert_eq!(p.ops[0].nums::<2>(), Some([5.0, 0.0])); +} + #[test] fn operators_keep_operands_and_spans() { let src = b"q 1 0 0 1 72 720 cm BT /F1 12 Tf (Hello \\(world\\)) Tj [(A) -120 (B)] TJ ET Q"; From 07aea983d0b5df84b474f28d374b143254e5c41f Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:35:25 +0000 Subject: [PATCH 04/21] fix(content): splice copies only the pieces a range touches Pieces::splice copied the unedited bytes between edits by visiting every piece for every edit, so a page of many content pieces and many replaced operators cost pieces x edits steps. The copy starts at the piece holding its first byte and stops past its last one. --- crates/content/src/lib.rs | 13 ++++++++++++- crates/content/src/tests.rs | 24 ++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/crates/content/src/lib.rs b/crates/content/src/lib.rs index 04bc1cfa9..986bab495 100644 --- a/crates/content/src/lib.rs +++ b/crates/content/src/lib.rs @@ -504,9 +504,20 @@ impl Pieces { /// for byte. Returns every piece's new bytes (unchanged pieces come back equal). pub fn splice(&self, edits: impl IntoIterator, Vec)>) -> Vec> { let mut out: Vec> = (0..self.len()).map(|i| Vec::with_capacity(self.range(i).len())).collect(); + // Only the pieces from the one holding `from` up to `to`: one call per edit must not + // visit every piece. let copy = |out: &mut Vec>, from: usize, to: usize| { - for (i, piece) in out.iter_mut().enumerate() { + if from >= to { + return; + } + let first = self.piece_of(from); + for (i, piece) in out.iter_mut().enumerate().skip(first) { + #[cfg(test)] + tests::PIECE_VISITS.with(|c| c.set(c.get() + 1)); let r = self.range(i); + if r.start >= to { + break; + } let (a, b) = (from.max(r.start), to.min(r.end)); if a < b { piece.extend_from_slice(self.data.get(a..b).unwrap_or_default()); diff --git a/crates/content/src/tests.rs b/crates/content/src/tests.rs index f32ab3da5..acf494320 100644 --- a/crates/content/src/tests.rs +++ b/crates/content/src/tests.rs @@ -5,6 +5,30 @@ thread_local! { pub(crate) static RESCANNED: std::cell::Cell = const { std::cell::Cell::new(0) }; } +thread_local! { + /// Pieces [`Pieces::splice`] looked at while copying, on this thread. + pub(crate) static PIECE_VISITS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +#[test] +fn splicing_visits_each_piece_a_few_times() { + let n = 5_000; + let pieces: Vec = (0..n).map(|i| format!("({i}) Tj")).collect(); + let joined = Pieces::join(&pieces); + let parsed = joined.parse(); + let edits: Vec<_> = parsed.ops.iter().map(|op| (op.span.clone(), b"(x) Tj".to_vec())).collect(); + assert_eq!(edits.len(), n); + PIECE_VISITS.with(|c| c.set(0)); + let out = joined.splice(edits); + let visits = PIECE_VISITS.with(|c| c.get()); + assert!(visits <= 4 * n, "{visits} piece visits for {n} pieces"); + assert!(out.iter().all(|p| p == b"(x) Tj")); + // A replacement spanning two pieces stays whole in the first and leaves the rest of the second. + let two = Pieces::join(&["q (a)", "Tj Q"]); + let tj = two.parse().ops.into_iter().find(|o| o.is("Tj")).map(|o| o.span).unwrap_or_default(); + assert_eq!(two.splice([(tj, b"(b) Tj".to_vec())]), [b"q (b) Tj".to_vec(), b" Q".to_vec()]); +} + #[test] fn a_long_unreadable_number_is_read_once() { for junk in ["+", "5"] { From e2c71b60e5e1f5f8aad7da8cbabcdf1626dbe89c Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:36:19 +0000 Subject: [PATCH 05/21] fix(filters): refuse a filter chain longer than eight Every stage of a stream's filter chain could produce up to the output limit and nothing bounded the number of stages, so a long /Filter array over data that decodes to itself (FF FF under RunLengthDecode) repeated the full work once per array entry. Chains of more than eight filters are unsupported, like an image codec, and stay encoded. --- crates/filters/src/lib.rs | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/crates/filters/src/lib.rs b/crates/filters/src/lib.rs index 9a0124d85..87306166c 100644 --- a/crates/filters/src/lib.rs +++ b/crates/filters/src/lib.rs @@ -137,8 +137,8 @@ pub(crate) type Step = Result, Failure>; /// Decodes `data` through `chain` (first element applied first). /// /// Image codecs, `Crypt` and unknown filters anywhere in the chain give -/// [`FilterError::Unsupported`] (callers keep such data encoded). `max_output` bounds -/// every intermediate buffer. +/// [`FilterError::Unsupported`] (callers keep such data encoded), as does a chain longer than +/// [`MAX_CHAIN`]. `max_output` bounds every intermediate buffer. pub fn decode(chain: &[(Filter, Params)], data: &[u8], max_output: usize) -> Result, FilterError> { run(chain, data, max_output, true).map(|(v, _)| v) } @@ -150,13 +150,22 @@ pub fn decode_tolerant(chain: &[(Filter, Params)], data: &[u8], max_output: usiz run(chain, data, max_output, false) } +/// Longest filter chain decoded. Real streams use one to three filters; every stage may produce +/// up to the output limit, so the chain length multiplies the work. +pub const MAX_CHAIN: usize = 8; + fn run(chain: &[(Filter, Params)], data: &[u8], max: usize, strict: bool) -> Result<(Vec, bool), FilterError> { + if chain.len() > MAX_CHAIN { + return Err(FilterError::Unsupported(format!("a chain of {} filters", chain.len()))); + } if let Some((f, _)) = chain.iter().find(|(f, _)| !f.is_decodable()) { return Err(FilterError::Unsupported(f.name().to_owned())); } let mut cur: Cow<[u8]> = Cow::Borrowed(data); let mut partial = false; for (filter, params) in chain { + #[cfg(test)] + STAGE_BYTES.with(|c| c.set(c.get() + cur.len())); cur = Cow::Owned(match decode_one(filter, params, &cur, max) { Ok(v) => v, Err(Failure { error: FilterError::Corrupt { .. }, partial: out }) if !strict => { @@ -218,8 +227,27 @@ pub(crate) fn is_pdf_whitespace(b: u8) -> bool { matches!(b, 0 | 9 | 10 | 12 | 13 | 32) } +#[cfg(test)] +thread_local! { + /// Bytes handed to decoding stages on this thread. + static STAGE_BYTES: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + #[cfg(test)] mod tests { + #[test] + fn a_long_filter_chain_is_refused_before_decoding() { + // `FF FF` run-length decodes to itself, so every stage of the chain repeats the work. + let data = vec![0xFF; 4096]; + let chain = |n: usize| vec![(super::Filter::RunLength, super::Params::default()); n]; + super::STAGE_BYTES.with(|c| c.set(0)); + let long = super::decode(&chain(1000), &data, 1 << 20); + let read = super::STAGE_BYTES.with(|c| c.get()); + assert!(read <= 16 * data.len(), "{read} bytes decoded through the chain"); + assert!(matches!(long, Err(super::FilterError::Unsupported(_))), "{long:?}"); + assert_eq!(super::decode(&chain(super::MAX_CHAIN), &data, 1 << 20).ok(), Some(data)); + } + use super::*; #[test] From 92e713feb1ac2dac75b26bdc2d2952ed44d294ee Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:37:09 +0000 Subject: [PATCH 06/21] fix(cos): a /Filter that is not a name does not decode as unfiltered Stream::filters reads only a direct name or array of names, so an indirect /Filter (or an array holding a reference) gave an empty chain and every decode returned the encoded bytes as if they were the data: a strict decode reported success on content it never decoded, while a viewer resolving the reference shows something else. Decoding such a stream is an error, so callers keep it encoded or refuse it. --- crates/cos/src/object.rs | 45 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/crates/cos/src/object.rs b/crates/cos/src/object.rs index 444fa3751..6b6a4befe 100644 --- a/crates/cos/src/object.rs +++ b/crates/cos/src/object.rs @@ -256,6 +256,21 @@ impl Stream { .collect() } + /// [`Stream::filters`], or an error when `/Filter` is present but is not a name or an + /// array of names (an indirect reference, say): the data is not unfiltered, so it must not + /// read as decoded. + fn readable_filters(&self) -> Result, CosError> { + let readable = match self.dict.get(b"Filter") { + None | Some(Object::Null | Object::Name(_)) => true, + Some(Object::Array(a)) => a.iter().all(|f| f.as_name().is_some()), + Some(_) => false, + }; + if !readable { + return Err(CosError::Filter("/Filter is not a name or an array of names".into())); + } + Ok(self.filters()) + } + /// Decoded data, tolerating truncated or corrupt encodings the way viewers do (the data /// decoded before the damage is returned). Streams with image codecs fail with /// `CosError::Filter` (keep them encoded). Use `decoded_strict` to detect damage. @@ -266,7 +281,7 @@ impl Stream { /// Like [`Stream::decoded`], for a stream whose decoded size has a tighter bound than /// [`MAX_DECODED`]: decoding past `max` bytes is an error. pub fn decoded_within(&self, max: usize) -> Result, CosError> { - let chain = self.filters(); + let chain = self.readable_filters()?; if chain.is_empty() { return self.raw_within(max); } @@ -275,7 +290,7 @@ impl Stream { /// Decoded data, bounded by [`MAX_DECODED`]; any corruption is an error. pub fn decoded_strict(&self) -> Result, CosError> { - let chain = self.filters(); + let chain = self.readable_filters()?; if chain.is_empty() { return self.raw_within(MAX_DECODED); } @@ -285,7 +300,7 @@ impl Stream { /// Data after the first filter alone, bounded by `max`: the rest of the chain stays encoded. /// Any corruption in that first filter is an error. pub fn decoded_first_filter_strict(&self, max: usize) -> Result, CosError> { - let mut chain = self.filters(); + let mut chain = self.readable_filters()?; chain.truncate(1); if chain.is_empty() { return self.raw_within(max); @@ -434,6 +449,30 @@ impl From for Object { mod tests { use super::*; + #[test] + fn a_filter_that_is_not_a_direct_name_is_not_decoded_as_absent() { + let stream = |filter: Object| { + let mut d = Dict::new(); + d.set(b"Filter".to_vec(), filter); + Stream::from_raw(d, b"48 69>".to_vec()) + }; + let hex = Object::Name(b"ASCIIHexDecode".to_vec()); + assert_eq!(stream(hex.clone()).decoded_strict().ok(), Some(b"Hi".to_vec())); + assert_eq!(stream(Object::Array(vec![hex.clone()])).decoded().ok(), Some(b"Hi".to_vec())); + assert_eq!(stream(Object::Null).decoded_strict().ok(), Some(b"48 69>".to_vec())); + for filter in [ + Object::Ref(ObjRef::new(5, 0)), + Object::Array(vec![Object::Ref(ObjRef::new(5, 0))]), + Object::Int(3), + Object::Array(vec![hex, Object::Int(1)]), + ] { + let s = stream(filter.clone()); + assert!(s.decoded_strict().is_err(), "{filter:?}"); + assert!(s.decoded().is_err(), "{filter:?}"); + assert!(s.decoded_first_filter_strict(1 << 20).is_err(), "{filter:?}"); + } + } + #[test] fn parsing_many_distinct_keys_is_not_quadratic() { let n = 20_000; From 73d15cd8b25c38483bd6be7e9d6c0aa59bb92bf7 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:38:04 +0000 Subject: [PATCH 07/21] fix(cos): count strings at every depth against the object caches The object caches weigh what each cached object owns, but the walk stopped four levels down and counted anything deeper as nothing. Many objects holding a large string inside a few nested arrays stayed cached far past the byte budget (34 MiB against 32 MiB in the test, and without limit as objects grow). The walk now goes past the parser's nesting limit; anything deeper counts as too large to keep. --- crates/cos/src/document.rs | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/crates/cos/src/document.rs b/crates/cos/src/document.rs index c3f6c6b74..96975eb36 100644 --- a/crates/cos/src/document.rs +++ b/crates/cos/src/document.rs @@ -494,12 +494,16 @@ impl ObjectReader { } } +/// Nesting [`owned_bytes`] walks: past the parser's own nesting limit, so every parsed object +/// is measured in full. +const OWNED_BYTES_DEPTH: usize = 256; + /// Bytes `object` holds of its own: string and stream data that is not a view into `data` -/// (the input file), nested a few levels deep (deeper data is rare and still bounded by the -/// reader's object count). +/// (the input file). An object nested deeper than [`OWNED_BYTES_DEPTH`] cannot be measured +/// and counts as too large to keep. fn owned_bytes(object: &Object, data: &Arc>, depth: usize) -> usize { - if depth > 4 { - return 0; + if depth > OWNED_BYTES_DEPTH { + return usize::MAX; } match object { Object::String(s) => s.bytes.len(), @@ -2993,6 +2997,29 @@ endobj } } + #[test] + fn editor_cache_counts_deeply_nested_strings() { + let mut nested = Object::String(crate::PdfString::literal(vec![b'x'; 1 << 20])); + for _ in 0..8 { + nested = Object::Array(vec![nested]); + } + let mut source = Document::new_empty(); + let objects: Vec<_> = (0..48).map(|_| Object::Ref(source.add(nested.clone()))).collect(); + source.update_dict(source.root().unwrap(), |d| d.set(b"Extension".to_vec(), Object::Array(objects))).unwrap(); + let bytes = Arc::new(crate::write_full(&source, &crate::SaveOptions::default()).unwrap()); + let doc = Document::open(bytes).unwrap(); + assert_eq!(owned_bytes(&nested, &doc.data, 0), 1 << 20); + let root = doc.get(doc.root().unwrap()); + for reference in root.as_dict().unwrap().get(b"Extension").unwrap().as_array().unwrap() { + let Object::Ref(reference) = reference else { panic!("indirect object") }; + drop(doc.get(*reference)); + let cache = doc.cache.lock().unwrap(); + let owned = cache.values().fold(0usize, |n, o| n.saturating_add(owned_bytes(o, &doc.data, 0))); + let strings = cache.values().filter(|o| o.as_array().is_some()).count(); + assert!(strings << 20 <= Document::EDITOR_CACHE_BYTES + (1 << 20), "{strings} MiB strings cached, {owned} bytes counted"); + } + } + #[test] fn editor_cache_stays_bounded() { let mut source = Document::new_empty(); From b1224557058aaf568a17f4c5063878d6131c89a2 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:39:54 +0000 Subject: [PATCH 08/21] fix(cos): find endstream once per document, not once per stream A stream whose /Length is wrong or missing is ended at the next endstream, which the parser found by searching the rest of the file. Many such streams before one late endstream (a damaged file being reconstructed reads every one) cost the file size per stream. The document lists where endstream occurs the first time it needs it, and each stream looks its end up there. --- crates/cos/src/document.rs | 41 +++++++++++++++++++++++++++++++----- crates/cos/src/parser.rs | 43 ++++++++++++++++++++++++++++++++------ 2 files changed, 73 insertions(+), 11 deletions(-) diff --git a/crates/cos/src/document.rs b/crates/cos/src/document.rs index 96975eb36..b783a8245 100644 --- a/crates/cos/src/document.rs +++ b/crates/cos/src/document.rs @@ -10,7 +10,7 @@ use std::sync::{Arc, Mutex, Weak}; use crate::CosError; use crate::object::{Dict, ObjRef, Object}; -use crate::parser::{Lexer, is_whitespace, parse_indirect, parse_indirect_shared}; +use crate::parser::{EndstreamIndex, Lexer, is_whitespace, parse_indirect, parse_indirect_shared}; thread_local! { /// The objects being loaded on this thread, innermost last (see `Document::try_get`). @@ -405,6 +405,8 @@ impl std::fmt::Debug for ReadState { #[derive(Clone)] pub struct Document { data: Arc>, + /// Where `endstream` occurs in `data`; see [`EndstreamIndex`]. + endstreams: Arc, source_identity: Arc<()>, entries: Arc, trailer: Dict, @@ -536,6 +538,7 @@ impl Document { pub fn new_empty() -> Self { let mut doc = Document { data: Arc::new(Vec::new()), + endstreams: Arc::default(), source_identity: Arc::new(()), entries: Arc::new(XrefIndex::empty()), trailer: Dict::new(), @@ -618,6 +621,7 @@ impl Document { }; let mut doc = Document { data, + endstreams: Arc::default(), source_identity: Arc::new(()), entries: Arc::new(XrefIndex::empty()), trailer: Dict::new(), @@ -982,7 +986,7 @@ impl Document { match self.xref_entry(num) { None | Some(XrefEntry::Free { .. }) => true, Some(XrefEntry::InFile { offset, .. }) => { - let at = |off: usize| matches!(parse_indirect_shared(&self.data, off, &|_| None), Ok((id, _)) if id.num == num); + let at = |off: usize| matches!(parse_indirect_shared(&self.data, off, &|_| None, &self.endstreams), Ok((id, _)) if id.num == num); // Offsets may count from a header that does not start the file. usize::try_from(offset).is_ok_and(|off| at(off) || (self.header_offset != 0 && at(off.saturating_add(self.header_offset)))) } @@ -1180,7 +1184,7 @@ impl Document { length }; let off = offset as usize; - let at = |off: usize| match parse_indirect_shared(&self.data, off, &resolve) { + let at = |off: usize| match parse_indirect_shared(&self.data, off, &resolve, &self.endstreams) { Ok((id, o)) if id.num == num => Some(self.decrypted(id, o)), _ => None, }; @@ -1246,7 +1250,7 @@ impl Document { if p > 0 && !is_whitespace(data[p - 1]) { continue; } - if let Ok((id, o)) = parse_indirect_shared(data, p, &|_| None) + if let Ok((id, o)) = parse_indirect_shared(data, p, &|_| None, &self.endstreams) && id.num == num { found = Some((id, o)); // keep the last (newest) definition @@ -2055,7 +2059,7 @@ impl Document { if trailer.as_ref().is_some_and(|((catalog, p), _)| *catalog && *p > at) { continue; } - if let Ok((_, Object::Stream(s))) = parse_indirect_shared(&data, at, &|_| None) + if let Ok((_, Object::Stream(s))) = parse_indirect_shared(&data, at, &|_| None, &self.endstreams) && s.dict.name(b"Type") == Some(b"XRef") && let Some(r) = rank(at, &s.dict) && trailer.as_ref().is_none_or(|(best, _)| r > *best) @@ -2997,6 +3001,33 @@ endobj } } + #[test] + fn streams_without_length_share_one_endstream_search() { + let n = 2_000; + let mut file = b"%PDF-1.7\n".to_vec(); + for k in 1..=n { + file.extend_from_slice(format!("{k} 0 obj\n<< >>\nstream\nxx\n").as_bytes()); + } + file.extend_from_slice(b"endstream\nendobj\n"); + file.extend_from_slice( + format!( + "{0} 0 obj\n<< /Type /Catalog /Pages {1} 0 R >>\nendobj\n{1} 0 obj\n<< /Type /Pages /Kids [] /Count 0 >>\nendobj\n", + n + 1, + n + 2 + ) + .as_bytes(), + ); + file.extend_from_slice(format!("trailer\n<< /Root {} 0 R /Size {} >>\n%%EOF\n", n + 1, n + 3).as_bytes()); + let len = file.len(); + crate::parser::ENDSTREAM_SCAN.with(|c| c.set(0)); + let doc = Document::open(Arc::new(file)).unwrap(); + for k in [1, n / 2, n] { + assert!(matches!(doc.try_get(k).as_deref(), Ok(Object::Stream(_))), "object {k}"); + } + let scanned = crate::parser::ENDSTREAM_SCAN.with(|c| c.get()); + assert!(scanned <= 4 * len, "{scanned} bytes searched in a {len}-byte file"); + } + #[test] fn editor_cache_counts_deeply_nested_strings() { let mut nested = Object::String(crate::PdfString::literal(vec![b'x'; 1 << 20])); diff --git a/crates/cos/src/parser.rs b/crates/cos/src/parser.rs index a16ff10c3..b470cffa4 100644 --- a/crates/cos/src/parser.rs +++ b/crates/cos/src/parser.rs @@ -368,6 +368,8 @@ fn real_prefix_len(s: &[u8]) -> usize { #[cfg(test)] thread_local! { + /// Bytes searched for `endstream` on this thread. + pub(crate) static ENDSTREAM_SCAN: std::cell::Cell = const { std::cell::Cell::new(0) }; /// Bytes handed to number conversion on this thread. static NUMBER_BYTES: std::cell::Cell = const { std::cell::Cell::new(0) }; } @@ -390,17 +392,30 @@ fn hexval(b: u8) -> Option { /// Parse an indirect object at `offset`: `N G obj [stream…endstream] endobj`. /// `resolve_length` resolves an indirect `/Length`. Returns the object and its (num, gen). pub fn parse_indirect(data: &[u8], offset: usize, resolve_length: &dyn Fn(ObjRef) -> Option) -> Result<(ObjRef, Object), CosError> { - parse_indirect_in(data, None, offset, resolve_length) + parse_indirect_in(data, None, offset, resolve_length, None) } /// [`parse_indirect`] for a document's own bytes: stream data points into `data` instead of /// being copied out of it. +/// `endstreams` is the document's [`EndstreamIndex`] for `data`. pub(crate) fn parse_indirect_shared( data: &Arc>, offset: usize, resolve_length: &dyn Fn(ObjRef) -> Option, + endstreams: &EndstreamIndex, ) -> Result<(ObjRef, Object), CosError> { - parse_indirect_in(data, Some(data), offset, resolve_length) + parse_indirect_in(data, Some(data), offset, resolve_length, Some(endstreams)) +} + +/// Where `endstream` occurs in a document's bytes, found once, the first time a stream's +/// /Length is wrong or missing. Searching the rest of the file for each such stream instead +/// makes many of them ending at one late `endstream` quadratic to read. +pub(crate) type EndstreamIndex = std::sync::OnceLock>; + +fn endstream_offsets(data: &[u8]) -> Vec { + #[cfg(test)] + ENDSTREAM_SCAN.with(|c| c.set(c.get() + data.len())); + data.windows(9).enumerate().filter(|(_, w)| *w == b"endstream").map(|(i, _)| i).collect() } fn parse_indirect_in( @@ -408,6 +423,7 @@ fn parse_indirect_in( shared: Option<&Arc>>, offset: usize, resolve_length: &dyn Fn(ObjRef) -> Option, + endstreams: Option<&EndstreamIndex>, ) -> Result<(ObjRef, Object), CosError> { let mut lx = Lexer::new(data, offset); lx.skip_ws(); @@ -467,7 +483,11 @@ fn parse_indirect_in( let len = match declared { Some(l) if valid(l) => l as usize, // Wrong or missing /Length: search for "endstream" (§7.3.8.1 recovery). - _ => find_endstream(data, start).ok_or_else(|| CosError::Syntax { offset: start, detail: "stream without endstream".into() })? - start, + _ => { + find_endstream(data, start, endstreams) + .ok_or_else(|| CosError::Syntax { offset: start, detail: "stream without endstream".into() })? + - start + } }; let range = start..start + len; let raw = match shared { @@ -483,9 +503,20 @@ fn parse_indirect_in( } /// Offset where stream data ends (before the EOL preceding "endstream"). -fn find_endstream(data: &[u8], from: usize) -> Option { - let hay = data.get(from..)?; - let at = hay.windows(9).position(|w| w == b"endstream")? + from; +fn find_endstream(data: &[u8], from: usize, endstreams: Option<&EndstreamIndex>) -> Option { + let at = match endstreams { + Some(index) => { + let index = index.get_or_init(|| endstream_offsets(data)); + index.get(index.partition_point(|&p| p < from)).copied()? + } + None => { + let hay = data.get(from..)?; + let found = hay.windows(9).position(|w| w == b"endstream"); + #[cfg(test)] + ENDSTREAM_SCAN.with(|c| c.set(c.get() + found.map_or(hay.len(), |p| p + 9))); + found? + from + } + }; let mut end = at; if end > from && data[end - 1] == b'\n' { end -= 1; From 196be22b64c39bb6315a0399db4392fa404e211e Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:41:37 +0000 Subject: [PATCH 09/21] fix(cos): read a hybrid cross-reference stream once per chain The materialized cross-reference reader reread a section's /XRefStm for every revision that named it, decoding the stream and inserting its rows each time, although the first read had already merged every entry. Up to 10,000 revisions naming one large stream repeated that work 10,000 times. A stream already read is skipped, with one repair log line. --- crates/cos/src/document.rs | 42 +++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/crates/cos/src/document.rs b/crates/cos/src/document.rs index b783a8245..470f395cd 100644 --- a/crates/cos/src/document.rs +++ b/crates/cos/src/document.rs @@ -496,6 +496,12 @@ impl ObjectReader { } } +#[cfg(test)] +thread_local! { + /// Cross-reference sections read on this thread. + static SECTION_READS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + /// Nesting [`owned_bytes`] walks: past the parser's own nesting limit, so every parsed object /// is measured in full. const OWNED_BYTES_DEPTH: usize = 256; @@ -1748,6 +1754,9 @@ impl Document { let mut revisions = Vec::new(); let mut oddities = Vec::new(); let mut visited = HashSet::new(); + // Hybrid streams already read: a newer section has merged every entry of one, so reading + // it again for each older section that names it adds nothing and repeats the decode. + let (mut hybrids, mut noted) = (HashSet::new(), HashSet::new()); let mut next = Some(first); while let Some(off) = next.take() { if !visited.insert(off) || visited.len() > 10_000 { @@ -1778,7 +1787,12 @@ impl Document { revisions.push(Revision { xref_offset: off, is_stream, header_relative }); // Hybrid files (§7.5.8.4): the /XRefStm entries supplement the table and replace the // free entries it gives objects only a cross-reference stream can locate. - if let Some(x) = sect_trailer.int(b"XRefStm").filter(|x| *x >= 0) { + let hybrid = sect_trailer.int(b"XRefStm").filter(|x| *x >= 0); + let repeated = hybrid.is_some_and(|x| !hybrids.insert(x)); + if let Some(x) = hybrid.filter(|x| repeated && noted.insert(*x)) { + log.push(format!("hybrid /XRefStm at {x} named again by the section at {off}; read once")); + } + if let Some(x) = hybrid.filter(|_| !repeated) { let mut stm = BTreeMap::new(); match self.read_section(x as usize, &mut stm, &mut odd, log) { Ok(_) => { @@ -1822,6 +1836,8 @@ impl Document { odd: &mut Vec, log: &mut Vec, ) -> Result<(Dict, bool), CosError> { + #[cfg(test)] + SECTION_READS.with(|c| c.set(c.get() + 1)); let data = &self.data; let mut lx = Lexer::new(data, off); if lx.eat_keyword(b"xref") { @@ -3199,6 +3215,30 @@ endobj out } + /// Many revisions naming one hybrid /XRefStm read it once. + #[test] + fn a_hybrid_stream_named_by_many_revisions_is_read_once() { + let mut out = object_stream_file("3 0 ", "<< /Hidden (yes) >>", 0); + let stream = find(&out, b"5 0 obj", 0, out.len()).unwrap(); + let mut prev = rfind(&out, b"\nxref\n", 0).unwrap() + 1; + let revisions = 200; + for _ in 0..revisions { + let at = out.len(); + out.extend_from_slice( + format!("xref\ntrailer << /Size 6 /Root 1 0 R /XRefStm {stream} /Prev {prev} >>\nstartxref\n{at}\n%%EOF\n").as_bytes(), + ); + prev = at; + } + let doc = Document::open(Arc::new(out)).unwrap(); + SECTION_READS.with(|c| c.set(0)); + let (entries, ..) = doc.read_xref_chain_materialized(&mut Vec::new()).unwrap(); + let reads = SECTION_READS.with(|c| c.get()); + assert!(reads <= revisions + 2, "{reads} sections read for {} revisions", revisions + 1); + assert!(matches!(entries.get(&3), Some(XrefEntry::InStream { stream: 4, .. })), "{:?}", entries.get(&3)); + assert_eq!(text_of(&doc, 3, b"Hidden").as_deref(), Some("yes")); + assert_compact_matches_materialized(&doc); + } + fn text_of(doc: &Document, num: u32, key: &[u8]) -> Option { doc.get(ObjRef::new(num, 0)).as_dict().and_then(|d| d.get(key).and_then(|v| v.as_string().map(|s| s.to_text()))) } From 1127a660310853520b21a2c0f98688aaf2afb4d0 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:42:43 +0000 Subject: [PATCH 10/21] fix(crypt): resolve crypt filter methods once per handler Every string and stream decrypted searched the document's /CF filters for the method by name. A file controls both the number of filters and the number of strings, so the search multiplied the two when the filter in use was listed last. The handler resolves the string, stream and embedded-file methods when it is built, and looks named filters up in a map. --- crates/crypt/src/lib.rs | 66 +++++++++++++++++++++++++++++---------- crates/crypt/src/tests.rs | 26 +++++++++++++++ 2 files changed, 75 insertions(+), 17 deletions(-) diff --git a/crates/crypt/src/lib.rs b/crates/crypt/src/lib.rs index 82b598688..58e96e7d4 100644 --- a/crates/crypt/src/lib.rs +++ b/crates/crypt/src/lib.rs @@ -138,9 +138,51 @@ pub struct SecurityHandler { key: Vec, auth: Auth, recovered_user: Option>, + methods: Methods, +} + +/// The crypt filter methods of a handler, resolved once: a document controls both the number of +/// filters and the number of strings, so a search of `/CF` per string would multiply them. +#[derive(Clone, Debug)] +struct Methods { + string: Method, + stream: Method, + embedded_file: Method, + named: std::collections::HashMap, Method>, +} + +impl Methods { + fn new(dict: &EncryptDict) -> Self { + let mut named = std::collections::HashMap::with_capacity(dict.crypt_filters.len()); + for (name, method) in &dict.crypt_filters { + #[cfg(test)] + tests::FILTER_COMPARES.with(|c| c.set(c.get() + 1)); + // The first of two filters with one name wins, as a search would find it. + named.entry(name.clone()).or_insert(*method); + } + let mut methods = Methods { string: Method::Identity, stream: Method::Identity, embedded_file: Method::Identity, named }; + methods.string = methods.get(&dict.str_f); + methods.stream = methods.get(&dict.stm_f); + methods.embedded_file = methods.get(if dict.ef_f.is_empty() { &dict.stm_f } else { &dict.ef_f }); + methods + } + + fn get(&self, name: &[u8]) -> Method { + if name == b"Identity" || name.is_empty() { + return Method::Identity; + } + #[cfg(test)] + tests::FILTER_COMPARES.with(|c| c.set(c.get() + 1)); + self.named.get(name).copied().unwrap_or(Method::Identity) + } } impl SecurityHandler { + fn new(dict: EncryptDict, key: Vec, auth: Auth, recovered_user: Option>) -> Self { + let methods = Methods::new(&dict); + Self { dict, key, auth, recovered_user, methods } + } + /// Authenticate with `password` (tried as owner password, then user password). `None` /// tries the empty password, which opens documents that only have an owner password. pub fn open(dict: EncryptDict, id0: &[u8], password: Option<&str>) -> Result { @@ -177,7 +219,7 @@ impl SecurityHandler { } r => return Err(CryptError::Unsupported(format!("revision {r}"))), }; - Ok(Self { dict, key, auth, recovered_user }) + Ok(Self::new(dict, key, auth, recovered_user)) } /// Upgrade to owner access with the owner (permissions) password, as when a restricted @@ -235,26 +277,16 @@ impl SecurityHandler { self.dict.encrypt_metadata } - fn method_named(&self, name: &[u8]) -> Method { - if name == b"Identity" || name.is_empty() { - return Method::Identity; - } - self.dict.crypt_filters.iter().find(|(n, _)| n == name).map(|(_, m)| *m).unwrap_or(Method::Identity) - } - /// The method for strings or a kind of stream. fn method(&self, stream: Option) -> Method { if self.dict.v < 4 { return Method::Rc4; } match stream { - None => self.method_named(&self.dict.str_f), - Some(StreamKind::Normal) => self.method_named(&self.dict.stm_f), - Some(StreamKind::EmbeddedFile) => { - let eff = if self.dict.ef_f.is_empty() { &self.dict.stm_f } else { &self.dict.ef_f }; - self.method_named(eff) - } - Some(StreamKind::Named(n)) => self.method_named(n), + None => self.methods.string, + Some(StreamKind::Normal) => self.methods.stream, + Some(StreamKind::EmbeddedFile) => self.methods.embedded_file, + Some(StreamKind::Named(n)) => self.methods.get(n), } } @@ -642,7 +674,7 @@ pub fn create(params: &NewEncryption, id0: &[u8]) -> Result { (d.v, d.r, d.length_bits) = (5, 6, 256); @@ -671,7 +703,7 @@ pub fn create(params: &NewEncryption, id0: &[u8]) -> Result = const { std::cell::Cell::new(0) }; +} + +#[test] +fn crypt_filters_are_looked_up_once_per_handler() { + let plain = new(Algorithm::Aes128, "", "o", -1); + let mut d = plain.dict().clone(); + let n = 5_000; + let mut filters: Vec<_> = (0..n).map(|i| (format!("F{i}").into_bytes(), Method::Identity)).collect(); + filters.append(&mut d.crypt_filters); + d.crypt_filters = filters; + FILTER_COMPARES.with(|c| c.set(0)); + let h = SecurityHandler::open(d, ID, None).unwrap(); + let strings = 1_000; + let enc = plain.encrypt_string(7, 0, b"secret"); + for _ in 0..strings { + assert_eq!(h.decrypt_string(7, 0, &enc), b"secret"); + assert_eq!(h.decrypt_stream(7, 0, &plain.encrypt_stream(7, 0, b"s", StreamKind::Normal), StreamKind::Normal), b"s"); + } + assert_eq!(h.decrypt_stream(7, 0, b"x", StreamKind::Named(b"F3")), b"x"); + let compares = FILTER_COMPARES.with(|c| c.get()); + assert!(compares <= 2 * n + 3 * strings, "{compares} filter name comparisons"); +} + #[test] fn identity_crypt_filters_leave_data_alone() { let mut d = new(Algorithm::Aes256, "", "o", -1).dict().clone(); From 6905acad2c12e3d7051a2835657c46d5da2b3001 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:44:26 +0000 Subject: [PATCH 11/21] fix(model): bound the inherited values a page walk copies Each leaf page gets its own copy of every attribute it inherits, so a large direct /Resources on the root of a page tree with thousands of leaves was copied thousands of times: a compressed file of a few kilobytes could need tens of gigabytes. The walk weighs each inherited value once where it is declared and charges every copy to a 1 GiB budget; past it the walk is cut off, as an over-large tree already is. --- crates/model/src/lib.rs | 73 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 66 insertions(+), 7 deletions(-) diff --git a/crates/model/src/lib.rs b/crates/model/src/lib.rs index 38bf36f22..886684c6d 100644 --- a/crates/model/src/lib.rs +++ b/crates/model/src/lib.rs @@ -82,14 +82,36 @@ pub fn view_matrix_for(rotation: i64, crop: [f64; 4]) -> [f64; 6] { } /// Leaf pages in document order, with inherited attributes resolved. Cycles and absurdly deep -/// trees are cut off rather than followed. +/// trees are cut off rather than followed, as is a tree whose inherited values would take more +/// than [`MAX_INHERITED_COPIES`] bytes to copy into its leaves. pub fn pages(doc: &Document) -> Vec { PageTree::new(doc).collect() } +/// Approximate bytes of inherited values copied into leaf dictionaries by one page walk. +pub const MAX_INHERITED_COPIES: usize = 1 << 30; + +/// Approximate heap bytes of `o`: one [`Object`] per node plus string, name and key bytes. +/// Values nested deeper than the parser allows weigh as too large. +fn weight(o: &Object, depth: usize) -> usize { + if depth > 256 { + return usize::MAX; + } + let own = std::mem::size_of::(); + match o { + Object::String(s) => own.saturating_add(s.bytes.len()), + Object::Name(n) => own.saturating_add(n.len()), + Object::Array(a) => a.iter().fold(own, |n, v| n.saturating_add(weight(v, depth + 1))), + Object::Dict(d) => d.iter().fold(own, |n, (k, v)| n.saturating_add(k.len()).saturating_add(weight(v, depth + 1))), + Object::Stream(s) => s.dict.iter().fold(own, |n, (k, v)| n.saturating_add(k.len()).saturating_add(weight(v, depth + 1))), + _ => own, + } +} + // Retain the owner of each inherited value rather than deep-cloning direct Resources on -// every edge. Only the final Page dictionaries need owned copies of those values. -type Inherited = [Option>; 4]; +// every edge. Only the final Page dictionaries need owned copies of those values. Each owner +// carries the value's weight, measured once where the value is declared. +type Inherited = [Option<(std::sync::Arc, usize)>; 4]; struct Children { owner: std::sync::Arc, @@ -112,16 +134,23 @@ struct PageTree<'a> { // One cursor per ancestor, never one entry per child. This also preserves the first // depth-first occurrence's attributes when malformed trees share a child. pending: Vec, + // Weight left for copying inherited values into leaves. A large direct value on an + // ancestor of many leaves would otherwise be copied once per leaf. + budget: usize, } impl<'a> PageTree<'a> { fn new(doc: &'a Document) -> Self { + Self::with_budget(doc, MAX_INHERITED_COPIES) + } + + fn with_budget(doc: &'a Document, budget: usize) -> Self { let top = doc.root().and_then(|r| doc.get(r).as_dict().and_then(|d| d.reference(b"Pages"))); let pending = top .into_iter() .map(|r| Children { owner: std::sync::Arc::new(Object::Array(vec![Object::Ref(r)])), next: 0, inherited: Default::default() }) .collect(); - Self { doc, seen: Default::default(), pending } + Self { doc, seen: Default::default(), pending, budget } } } @@ -149,8 +178,8 @@ impl Iterator for PageTree<'_> { let Some(d) = obj.as_dict() else { continue }; let mut inherited = parent.inherited.clone(); for (key, owner) in INHERITABLE.iter().zip(&mut inherited) { - if d.contains(key) { - *owner = Some(obj.clone()); + if let Some(value) = d.get(key) { + *owner = Some((obj.clone(), weight(value, 0))); } } let is_pages = d.name(b"Type") == Some(b"Pages") || (d.contains(b"Kids") && d.name(b"Type") != Some(b"Page")); @@ -165,8 +194,15 @@ impl Iterator for PageTree<'_> { let mut dict = d.clone(); for (key, owner) in INHERITABLE.iter().zip(&inherited) { if !dict.contains(key) - && let Some(value) = owner.as_ref().and_then(|o| o.as_dict()).and_then(|d| d.get(key)) + && let Some((owner, weight)) = owner + && let Some(value) = owner.as_dict().and_then(|d| d.get(key)) { + // Past the budget the walk is cut off, like a tree too large to follow. + let Some(left) = self.budget.checked_sub(*weight) else { + self.pending.clear(); + return None; + }; + self.budget = left; dict.set(key.to_vec(), value.clone()); } } @@ -209,6 +245,29 @@ mod tests { Document::open(Arc::new(out)).unwrap() } + #[test] + fn inherited_copies_stop_at_a_budget() { + let mut d = doc(0); + let root = ObjRef { num: 2, generation: 0 }; + let leaf = d.get(ObjRef { num: 3, generation: 0 }).as_dict().unwrap().clone(); + let kids: Vec<_> = (0..200).map(|_| Object::Ref(d.add(leaf.clone()))).collect(); + let mut resources = Dict::new(); + resources.set(b"Payload".to_vec(), Object::Array(vec![Object::Int(42); 8192])); + let resources = Object::Dict(resources); + let each = weight(&resources, 0); + d.update_dict(root, |dict| { + dict.set(b"Kids".to_vec(), Object::Array(kids)); + dict.set(b"Resources".to_vec(), resources); + }) + .unwrap(); + let budget = 1 << 20; + let pages: Vec = PageTree::with_budget(&d, budget).collect(); + let copied: usize = pages.iter().filter_map(|p| p.dict.get(b"Resources")).map(|r| weight(r, 0)).sum(); + assert!(copied <= budget, "{copied} bytes of inherited values copied into {} pages", pages.len()); + assert_eq!(pages.len(), budget / each); + assert_eq!(super::pages(&d).len(), 200); + } + #[test] fn repeated_children_do_not_queue_resource_copies() { let mut d = doc(0); From a0e82da864aa65e9386678feafd2f84c01a9159b Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:48:08 +0000 Subject: [PATCH 12/21] fix(render): bound the replacement text copied into redraws Each unmarked redraw of an /ActualText span's glyphs took its own copy of the replacement text. The work budget counts comparisons, not bytes, so a long replacement around one glyph followed by many redraws at the same place copied it once per redraw: gigabytes from a small page. Redraws on one page share 4 MiB of replacement text; past it a redraw keeps its own glyphs, as it does past the work budget. --- crates/render/src/text.rs | 44 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/crates/render/src/text.rs b/crates/render/src/text.rs index 45c87bfea..11cde902f 100644 --- a/crates/render/src/text.rs +++ b/crates/render/src/text.rs @@ -420,6 +420,15 @@ const MAX_CELLS: i64 = 4096; /// their unmarked redraws (the text reads twice, as it would without `/ActualText`). const MAX_REDRAW_WORK: usize = 1 << 22; +/// Most replacement text bytes the redraw dedupe in `TextDevice::finish` copies on one page. +const MAX_REDRAW_TEXT: usize = 1 << 22; + +#[cfg(test)] +thread_local! { + /// Replacement text bytes given to redraws on this thread. + static REDRAW_TEXT_BYTES: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + fn cell(v: f32) -> i64 { // Glyph boxes are finite (`draw_glyph` and `merge` drop the rest); `as` saturates. (v / CELL).floor() as i64 @@ -564,6 +573,7 @@ impl TextDevice { redraws.sort_by_key(|(range, _)| range.start); let glyphs = std::mem::take(&mut self.glyphs); let mut redraws = redraws.into_iter().peekable(); + let mut text_left = MAX_REDRAW_TEXT; let mut i = 0; while let Some(g) = glyphs.get(i) { let Some((range, k)) = redraws.next_if(|(range, _)| range.start == i) else { @@ -572,7 +582,18 @@ impl TextDevice { continue; }; let run = glyphs.get(range.clone()).unwrap_or_default(); - let text = self.replaced.get(k).map(|r| r.text.clone()).unwrap_or_default(); + let replacement = self.replaced.get(k).map_or("", |r| r.text.as_str()); + // Each redraw takes its own copy of the replacement, and one long replacement can + // have many redraws: past the budget a redraw keeps its glyphs, as past the work budget. + let Some(left) = text_left.checked_sub(replacement.len()) else { + self.glyphs.extend_from_slice(run); + i = range.end.max(i + 1); + continue; + }; + text_left = left; + let text = replacement.to_owned(); + #[cfg(test)] + REDRAW_TEXT_BYTES.with(|c| c.set(c.get() + text.len())); if !text.chars().all(|c| c.is_control()) { match merge(run, text) { Some(glyph) => self.glyphs.push(glyph), @@ -1871,6 +1892,27 @@ mod tests { extract_page(pdf, page, &InterpreterSettings::default()).unwrap() } + #[test] + fn redraws_of_a_long_replacement_share_a_text_budget() { + let long = "0041".repeat(32 * 1024); + let redraws = 200; + let content = format!( + "BT /F1 20 Tf 1 0 0 1 100 150 Tm /Span <>> BDC (x) Tj EMC {} ET", + "1 0 0 1 100 150 Tm (x) Tj ".repeat(redraws) + ); + let pdf = marked_pages(&[&content]); + REDRAW_TEXT_BYTES.with(|c| c.set(0)); + let text = marked_text(&pdf, 0); + let given = REDRAW_TEXT_BYTES.with(|c| c.get()); + assert!(given <= MAX_REDRAW_TEXT, "{given} bytes of replacement text given to {redraws} redraws"); + assert!(text.plain_text().starts_with("AAAA")); + // A short replacement still covers its redraw. + let pdf = marked_pages(&["BT /F1 20 Tf 1 0 0 1 100 150 Tm /Span <> BDC (x) Tj EMC 1 0 0 1 100 150 Tm (x) Tj ET"]); + REDRAW_TEXT_BYTES.with(|c| c.set(0)); + assert_eq!(marked_text(&pdf, 0).plain_text(), "fi"); + assert_eq!(REDRAW_TEXT_BYTES.with(|c| c.get()), 2); + } + #[test] fn text_after_an_inline_image_survives_a_non_ascii_byte_before_a_later_ei() { // Everyday content both ways: a string with a byte above 0x7F, and a later word holding From 24701fe9591e93cf18662cc89d4f8f8d01a44fe3 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 16:00:42 +0000 Subject: [PATCH 13/21] fix(print): list the source pages once per imposition page_form listed every page of the document to take the one it printed, so imposing P pages walked the page tree and copied its leaf dictionaries P times over: P^2 page copies. impose passes the list it already holds. A page index outside it is an error, not a panic. --- crates/print/src/lib.rs | 23 ++++++++++++++++++++--- crates/print/src/tests.rs | 11 +++++++++++ crates/print/src/tests/appearances.rs | 3 ++- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/crates/print/src/lib.rs b/crates/print/src/lib.rs index 6839e5f6b..5f81d1f93 100644 --- a/crates/print/src/lib.rs +++ b/crates/print/src/lib.rs @@ -770,13 +770,16 @@ fn appearance_matrix(doc: &Document, form: &Dict, rect: [f64; 4]) -> Option, ) -> Result { - let page = pdfcraft_model::pages(doc).swap_remove(index); + let page = pages.get(index).ok_or_else(|| PrintError::Invalid(format!("there is no page {}", index.saturating_add(1))))?; let crop = page.crop(doc); let mut res = page.dict.get(b"Resources").and_then(|r| doc.resolve(r).as_dict().cloned()).unwrap_or_default(); let mut body = if content == Content::FormFieldsOnly { Vec::new() } else { decoded_contents(doc, &page.dict) }; @@ -841,10 +844,24 @@ fn page_form( Ok(doc.add(Object::Stream(Stream::flate(d, &body)))) } +#[cfg(test)] +thread_local! { + /// Pages listed by [`source_pages`] on this thread. + static PAGES_LISTED: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// The source document's pages, for printing. +fn source_pages(doc: &Document) -> Vec { + let pages = pdfcraft_model::pages(doc); + #[cfg(test)] + PAGES_LISTED.with(|c| c.set(c.get() + pages.len())); + pages +} + /// Build the print-ready PDF for `settings`. Encryption is not carried over (the file goes to a /// printer or is the user's own copy); callers must check the print permission first. pub fn impose(src: &Document, settings: &Settings) -> Result, PrintError> { - let pages = pdfcraft_model::pages(src); + let pages = source_pages(src); let sizes: Vec<(f64, f64)> = pages.iter().map(|p| p.display_size(src)).collect(); let sheets = layout(&sizes, settings)?; let mut doc = src.clone(); @@ -861,7 +878,7 @@ pub fn impose(src: &Document, settings: &Settings) -> Result, PrintError let form = match forms.get(&pl.page) { Some(f) => *f, None => { - let f = page_form(&mut doc, pl.page, settings.content, &mut promoted)?; + let f = page_form(&mut doc, &pages, pl.page, settings.content, &mut promoted)?; forms.insert(pl.page, f); f } diff --git a/crates/print/src/tests.rs b/crates/print/src/tests.rs index 99634f1aa..ce736762e 100644 --- a/crates/print/src/tests.rs +++ b/crates/print/src/tests.rs @@ -192,6 +192,17 @@ fn custom_pages_per_sheet_never_overfill_the_grid() { assert_eq!(pdfcraft_model::pages(&Document::open(Arc::new(out)).unwrap()).len(), 3); } +#[test] +fn imposing_lists_the_pages_once() { + let n = 150; + let doc = fixture(n); + PAGES_LISTED.with(|c| c.set(0)); + let out = impose(&doc, &settings((0..n).collect(), Layout::multiple(2))).unwrap(); + let listed = PAGES_LISTED.with(|c| c.get()); + assert!(listed <= 2 * n, "{listed} pages listed to print {n}"); + assert_eq!(pdfcraft_model::pages(&Document::open(Arc::new(out)).unwrap()).len(), n / 2); +} + #[test] fn booklets_pair_pages_for_folding() { let sizes = vec![(200.0, 300.0); 6]; diff --git a/crates/print/src/tests/appearances.rs b/crates/print/src/tests/appearances.rs index 8d6f56ff6..acf8ece6f 100644 --- a/crates/print/src/tests/appearances.rs +++ b/crates/print/src/tests/appearances.rs @@ -147,7 +147,8 @@ fn promotion_preserves_streams_and_explicit_subtypes() { for subtype in [None, Some(Object::name("Form")), Some(Object::name("Image")), Some(Object::Null), Some(Object::Int(7))] { let (mut doc, ap, _) = widget_fixture(subtype.clone(), Some([2.0, 0.0, 0.0, 3.0, 5.0, 7.0])); let original = doc.get(ap); - let form = page_form(&mut doc, 0, Content::Document, &mut std::collections::HashMap::new()).unwrap(); + let pages = pdfcraft_model::pages(&doc); + let form = page_form(&mut doc, &pages, 0, Content::Document, &mut std::collections::HashMap::new()).unwrap(); assert_eq!(doc.get(ap), original, "original in the print clone"); let promoted = xobject(&doc, doc.get(form).as_dict().unwrap(), b"PCPrA0"); if subtype.is_some() { From 3a11f39ef0ef58169a10d95598df832e9a5ff319 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 17:47:29 +0000 Subject: [PATCH 14/21] chore(cos): allow the boxed dictionary key index under clippy The index is boxed so that it adds one pointer to every Dict, and so to every Object, rather than an inline map. --- crates/cos/src/object.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/cos/src/object.rs b/crates/cos/src/object.rs index 6b6a4befe..ee3c49a78 100644 --- a/crates/cos/src/object.rs +++ b/crates/cos/src/object.rs @@ -92,6 +92,8 @@ const INDEX_FROM: usize = 32; pub struct Dict { entries: Vec<(Name, Object)>, /// Key to position in `entries`, kept once the dictionary reaches [`INDEX_FROM`] entries. + /// Boxed so that every `Dict`, and so every `Object`, grows by one pointer, not by a map. + #[allow(clippy::box_collection)] index: Option>>, } From 735fb8e661f39293cd7417628ce6a541feb959bc Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 17:47:29 +0000 Subject: [PATCH 15/21] docs(filters, model): record the filter chain and inherited copy limits --- crates/filters/README.md | 2 +- crates/model/README.md | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/filters/README.md b/crates/filters/README.md index df64e4b7c..fc5306e32 100644 --- a/crates/filters/README.md +++ b/crates/filters/README.md @@ -23,7 +23,7 @@ encode_flate(data) -> Vec // zlib, level 6 ``` - `chain` is applied first to last, as in a `/Filter` array. -- Image codecs (DCT, JPX, JBIG2, CCITT), `Crypt` and unknown filters anywhere in a chain give `Unsupported`: callers keep such data encoded. Decryption belongs to `crypt`. +- Image codecs (DCT, JPX, JBIG2, CCITT), `Crypt` and unknown filters anywhere in a chain give `Unsupported`: callers keep such data encoded. So does a chain longer than `MAX_CHAIN` (8) filters, before any stage runs. Decryption belongs to `crypt`. - `max_output` bounds every intermediate buffer (including the pre-predictor buffer) and gives `LimitExceeded(max_output)`. This also holds when Flate retries raw deflate after a misleading zlib header, in both strict and tolerant decoding. - `decode_tolerant` turns `Corrupt` into "what was decoded so far" plus `partial = true`, then runs the rest of the chain on it (predictors are applied to partial output too). `LimitExceeded` and `Unsupported` are still errors. diff --git a/crates/model/README.md b/crates/model/README.md index bfc6e42e3..09a060391 100644 --- a/crates/model/README.md +++ b/crates/model/README.md @@ -17,6 +17,9 @@ L3/L4 crates need: The page-tree walk keeps one cursor per ancestor and shares inherited attribute owners. Repeated child references are skipped before copying attributes, with first depth-first occurrence order preserved. Returned page dictionaries still own their effective attributes. +A walk copies at most `MAX_INHERITED_COPIES` (1 GiB, approximate heap bytes) of inherited +values into the leaves; past it the walk stops, as it does past its leaf cap, and the pages +after that point are not listed. `organize`, `annot` and `forms` still carry their own small walkers; they move here as the model grows (ADR-0004). From f63f509647ad70ec68a9b15c32b927a4e9249bbe Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 20:33:48 +0000 Subject: [PATCH 16/21] fix(cos): compare each section's table with a repeated hybrid stream A section naming an /XRefStm that a newer section had already read skipped the stream entirely, so an entry in its own table that disagreed with the stream was no longer recorded in xref_oddities. Signature validation reads those records to refuse an appended section another reader could follow. The reader keeps each hybrid stream's entries (up to MAX_XREF_OBJECTS in all) and compares every later naming section's table with them, without decoding the stream again. --- crates/cos/src/document.rs | 67 +++++++++++++++++++++++++++++++------- 1 file changed, 56 insertions(+), 11 deletions(-) diff --git a/crates/cos/src/document.rs b/crates/cos/src/document.rs index 470f395cd..892c8e04d 100644 --- a/crates/cos/src/document.rs +++ b/crates/cos/src/document.rs @@ -1754,9 +1754,12 @@ impl Document { let mut revisions = Vec::new(); let mut oddities = Vec::new(); let mut visited = HashSet::new(); - // Hybrid streams already read: a newer section has merged every entry of one, so reading - // it again for each older section that names it adds nothing and repeats the decode. - let (mut hybrids, mut noted) = (HashSet::new(), HashSet::new()); + // Hybrid streams already read, by offset: their entries, or `None` when unreadable. A newer + // section has merged every entry of one into `entries`, so an older section that names it + // again only has its own table compared with them, without decoding the stream again. + // At most MAX_XREF_OBJECTS entries are kept in all; a stream past that is read again. + let mut hybrids: HashMap>> = HashMap::new(); + let (mut hybrid_rows, mut noted) = (0usize, HashSet::new()); let mut next = Some(first); while let Some(off) = next.take() { if !visited.insert(off) || visited.len() > 10_000 { @@ -1788,15 +1791,23 @@ impl Document { // Hybrid files (§7.5.8.4): the /XRefStm entries supplement the table and replace the // free entries it gives objects only a cross-reference stream can locate. let hybrid = sect_trailer.int(b"XRefStm").filter(|x| *x >= 0); - let repeated = hybrid.is_some_and(|x| !hybrids.insert(x)); - if let Some(x) = hybrid.filter(|x| repeated && noted.insert(*x)) { - log.push(format!("hybrid /XRefStm at {x} named again by the section at {off}; read once")); - } - if let Some(x) = hybrid.filter(|_| !repeated) { + if let Some((x, read)) = hybrid.and_then(|x| hybrids.get(&x).map(|read| (x, read))) { + if noted.insert(x) { + log.push(format!("hybrid /XRefStm at {x} named again by the section at {off}; read once")); + } + if let Some(stm) = read { + for (num, old) in § { + // The table places the object somewhere other than the stream does. + if !matches!(old, XrefEntry::Free { .. }) && stm.get(num).is_some_and(|e| e != old) { + odd.push(*num); + } + } + } + } else if let Some(x) = hybrid { let mut stm = BTreeMap::new(); - match self.read_section(x as usize, &mut stm, &mut odd, log) { + let read = match self.read_section(x as usize, &mut stm, &mut odd, log) { Ok(_) => { - for (num, e) in stm { + for (&num, &e) in &stm { match sect.get(&num) { None | Some(XrefEntry::Free { .. }) => { sect.insert(num, e); @@ -1806,8 +1817,17 @@ impl Document { Some(_) => {} } } + Some(stm) } - Err(e) => log.push(format!("hybrid /XRefStm at {x} unreadable: {e}")), + Err(e) => { + log.push(format!("hybrid /XRefStm at {x} unreadable: {e}")); + None + } + }; + let rows = read.as_ref().map_or(0, BTreeMap::len); + if let Some(total) = hybrid_rows.checked_add(rows).filter(|total| *total <= MAX_XREF_OBJECTS) { + hybrid_rows = total; + hybrids.insert(x, read); } } oddities.extend(odd.into_iter().map(|num| (off, num))); @@ -3239,6 +3259,31 @@ endobj assert_compact_matches_materialized(&doc); } + /// An older section that names an /XRefStm a newer section already read still has its own + /// table compared with that stream, so an entry it sets aside is recorded for it. + #[test] + fn a_repeated_hybrid_stream_records_each_sections_conflicts() { + let mut out = object_stream_file("3 0 ", "<< /Hidden (yes) >>", 0); + let stream = find(&out, b"5 0 obj", 0, out.len()).unwrap(); + let catalog = find(&out, b"1 0 obj", 0, out.len()).unwrap(); + let base = rfind(&out, b"\nxref\n", 0).unwrap() + 1; + // This section's table places object 3 at the catalog; the stream puts it in object stream 4. + let older = out.len(); + out.extend_from_slice( + format!( + "xref\n3 1\n{catalog:010} 00000 n \ntrailer << /Size 6 /Root 1 0 R /XRefStm {stream} /Prev {base} >>\nstartxref\n{older}\n%%EOF\n" + ) + .as_bytes(), + ); + let newer = out.len(); + out.extend_from_slice( + format!("xref\ntrailer << /Size 6 /Root 1 0 R /XRefStm {stream} /Prev {older} >>\nstartxref\n{newer}\n%%EOF\n").as_bytes(), + ); + let doc = Document::open(Arc::new(out)).unwrap(); + assert_eq!(doc.xref_oddities(), [(older as u64, 3)]); + assert_eq!(text_of(&doc, 3, b"Hidden").as_deref(), Some("yes")); + } + fn text_of(doc: &Document, num: u32, key: &[u8]) -> Option { doc.get(ObjRef::new(num, 0)).as_dict().and_then(|d| d.get(key).and_then(|v| v.as_string().map(|s| s.to_text()))) } From 4e1457ed4f8b118270e737100cc0d05693d81c66 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 21:33:43 +0000 Subject: [PATCH 17/21] Marking a tag as an artifact refuses content past the page's decode budget --- crates/a11y/src/alt.rs | 20 +++++++++++++++++++- crates/a11y/src/tests.rs | 17 +++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/crates/a11y/src/alt.rs b/crates/a11y/src/alt.rs index f22a25f73..87cfce5ac 100644 --- a/crates/a11y/src/alt.rs +++ b/crates/a11y/src/alt.rs @@ -50,6 +50,8 @@ pub enum ArtifactError { AmbiguousParent, #[error("couldn't update the parent tree: {0}")] ParentTree(String), + #[error("the page has more content than can be edited")] + TooLarge, #[error(transparent)] Cos(#[from] pdfcraft_cos::CosError), } @@ -483,7 +485,23 @@ pub fn mark_tag_artifact(doc: &mut Document, obj: ObjRef) -> Result<(), Artifact let pages = pdfcraft_model::pages(doc); let page_ref = element.page.ok_or(ArtifactError::NoPage)?; let page = pages.iter().find(|page| page.obj == page_ref).ok_or(ArtifactError::NoPage)?; - let mut ops = pdfcraft_content::parse(&page_content(doc, &page.dict)).ops; + let mut content = Vec::new(); + if let Some(c) = page.dict.get(b"Contents") { + let mut budget = DecodeBudget::page(); + for data in budget.decode_contents(doc, c) { + match data { + Ok(data) => { + content.extend_from_slice(&data); + content.push(b' '); + } + // The page's /Contents is replaced with what was decoded: content past the + // budget would be dropped from the page. + Err(_) if budget.exhausted() => return Err(ArtifactError::TooLarge), + Err(_) => {} + } + } + } + let mut ops = pdfcraft_content::parse(&content).ops; let mut found = HashSet::new(); for op in &mut ops { let id = (op.op == b"BDC") diff --git a/crates/a11y/src/tests.rs b/crates/a11y/src/tests.rs index 9332d29df..399aded3c 100644 --- a/crates/a11y/src/tests.rs +++ b/crates/a11y/src/tests.rs @@ -1307,3 +1307,20 @@ fn content_past_the_decode_budget_needs_a_manual_check() { assert_eq!(status(&report, Rule::ImageOnly), Status::Manual); assert_eq!(status(&report, Rule::TaggedContent), Status::Manual); } + +/// Marking a tag as an artifact replaces the page's content with what it decoded, so content past +/// the page's decode budget is refused rather than dropped from the page. +#[test] +fn marking_a_tag_artifact_refuses_content_past_the_decode_budget() { + use pdfcraft_cos::{Dict, ObjRef, Object, Stream}; + let mut doc = figures_doc(); + let filler = doc.add(Stream::flate(Dict::new(), &vec![b' '; 1 << 20])); + let last = doc.add(Stream::from_raw(Dict::new(), b"0 0 5 5 re f".to_vec())); + let mut contents = vec![Object::Ref(ObjRef::new(4, 0))]; + contents.extend(std::iter::repeat_n(Object::Ref(filler), 256)); + contents.push(Object::Ref(last)); + let contents = Object::Array(contents); + doc.update_dict(ObjRef::new(3, 0), |p| p.set(b"Contents".to_vec(), contents.clone())).unwrap(); + assert_eq!(mark_tag_artifact(&mut doc, ObjRef::new(7, 0)), Err(ArtifactError::TooLarge)); + assert_eq!(doc.get(ObjRef::new(3, 0)).as_dict().unwrap().get(b"Contents"), Some(&contents), "the page keeps its content"); +} From 0ae289c7625d7c300220b62e346dac6deeb9ae1c Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 21:37:19 +0000 Subject: [PATCH 18/21] A failed strict decode charges the page's decode budget for what it decoded --- crates/cos/src/budget.rs | 48 ++++++++++++++++++++++++++++++++------- crates/cos/src/object.rs | 13 ++++++----- crates/filters/src/lib.rs | 19 ++++++++++++---- 3 files changed, 61 insertions(+), 19 deletions(-) diff --git a/crates/cos/src/budget.rs b/crates/cos/src/budget.rs index f88971222..b31d10391 100644 --- a/crates/cos/src/budget.rs +++ b/crates/cos/src/budget.rs @@ -12,6 +12,15 @@ pub const MAX_PAGE_DECODED: usize = 256 << 20; /// Form XObjects one page's content walk may enter, each entry counted again. pub const MAX_PAGE_FORMS: usize = 50_000; +/// A stream decode within a byte bound: the data, or the error and the bytes decoded before it. +type Decoder = fn(&Stream, usize) -> Result, (FilterError, usize)>; + +/// A tolerant decode recovers corrupt data as partial output, so its only errors are a spent +/// limit and an unsupported filter, which is refused before any decoding. +fn tolerant(stream: &Stream, max: usize) -> Result, (FilterError, usize)> { + stream.decoded_within_typed(max).map_err(|e| (e, 0)) +} + /// What one operation may still decode and enter across all the streams it reads. /// /// [`MAX_DECODED`](crate::MAX_DECODED) bounds one decode. It does not bound a page whose @@ -53,13 +62,13 @@ impl DecodeBudget { /// `stream` decoded like [`Stream::decoded`], charged to the allowance. Fails, without /// decoding, once the allowance is spent. pub fn decode(&mut self, stream: &Stream) -> Result, CosError> { - self.charge(stream, usize::MAX, Stream::decoded_within_typed) + self.charge(stream, usize::MAX, tolerant) } /// Like [`DecodeBudget::decode`], for a stream with a tighter bound of its own: decoding /// past `max` bytes is an error that charges `max`, the work it took to find out. pub fn decode_within(&mut self, stream: &Stream, max: usize) -> Result, CosError> { - self.charge(stream, max, Stream::decoded_within_typed) + self.charge(stream, max, tolerant) } /// `stream` decoded like [`Stream::decoded_strict`] (any corruption is an error), charged to @@ -68,7 +77,10 @@ impl DecodeBudget { self.charge(stream, usize::MAX, Stream::decoded_strict_within_typed) } - fn charge(&mut self, stream: &Stream, max: usize, decode: fn(&Stream, usize) -> Result, FilterError>) -> Result, CosError> { + /// Decodes `stream` within `max` and the allowance, charging what it decoded: a corrupt + /// stream pays for the bytes decoded before the error, so referencing it again and again + /// spends the allowance like any other stream. + fn charge(&mut self, stream: &Stream, max: usize, decode: Decoder) -> Result, CosError> { if self.bytes == 0 && !stream.raw.is_empty() { return Err(CosError::Filter(FilterError::LimitExceeded(self.limit).to_string())); } @@ -76,22 +88,25 @@ impl DecodeBudget { let result = decode(stream, max); let spent = match &result { Ok(data) => data.len(), - Err(FilterError::LimitExceeded(_)) => max, - Err(_) => 0, + Err((FilterError::LimitExceeded(_), _)) => max, + Err((_, spent)) => *spent, }; self.bytes = self.bytes.saturating_sub(spent); - return result.map_err(|e| CosError::Filter(e.to_string())); + return result.map_err(|(e, _)| CosError::Filter(e.to_string())); } match decode(stream, self.bytes) { Ok(data) => { self.bytes = self.bytes.saturating_sub(data.len()); Ok(data) } - Err(FilterError::LimitExceeded(_)) => { + Err((FilterError::LimitExceeded(_), _)) => { self.bytes = 0; Err(CosError::Filter(FilterError::LimitExceeded(self.limit).to_string())) } - Err(e) => Err(CosError::Filter(e.to_string())), + Err((e, spent)) => { + self.bytes = self.bytes.saturating_sub(spent); + Err(CosError::Filter(e.to_string())) + } } } @@ -169,4 +184,21 @@ mod tests { assert!(budget.first_visit(bomb)); assert!(!budget.first_visit(bomb)); } + + /// A corrupt stream fails a strict decode after decoding part of it: each reference pays + /// for that part, so references to it can't decode without limit. + #[test] + fn failed_strict_decodes_charge_what_they_decoded() { + let whole = Stream::flate(Dict::new(), &vec![b' '; 1 << 20]); + let cut = whole.raw.len() / 2; + let corrupt = Stream::from_raw(whole.dict.clone(), whole.raw.get(..cut).unwrap_or_default().to_vec()); + let partial = corrupt.decoded().map(|d| d.len()).unwrap_or(0); + assert!(partial > 0, "a cut flate stream decodes in part"); + let mut budget = DecodeBudget::new(10 << 20, 1); + assert!(budget.decode_strict(&corrupt).is_err()); + assert_eq!(budget.remaining(), (10 << 20) - partial); + let mut budget = DecodeBudget::new(10 << 20, 1); + assert!(budget.decode_within(&corrupt, 1 << 30).is_ok()); + assert_eq!(budget.remaining(), (10 << 20) - partial); + } } diff --git a/crates/cos/src/object.rs b/crates/cos/src/object.rs index d11d94711..0660d5a26 100644 --- a/crates/cos/src/object.rs +++ b/crates/cos/src/object.rs @@ -299,17 +299,18 @@ impl Stream { /// Decoded data, bounded by [`MAX_DECODED`]; any corruption is an error. pub fn decoded_strict(&self) -> Result, CosError> { - self.decoded_strict_within_typed(MAX_DECODED).map_err(|e| CosError::Filter(e.to_string())) + self.decoded_strict_within_typed(MAX_DECODED).map_err(|(e, _)| CosError::Filter(e.to_string())) } - /// [`Stream::decoded_strict`] bounded by `max`, with the filter error kept typed. - pub(crate) fn decoded_strict_within_typed(&self, max: usize) -> Result, pdfcraft_filters::FilterError> { + /// [`Stream::decoded_strict`] bounded by `max`, with the filter error kept typed and the + /// bytes decoded before it. + pub(crate) fn decoded_strict_within_typed(&self, max: usize) -> Result, (pdfcraft_filters::FilterError, usize)> { let max = max.min(MAX_DECODED); - let chain = self.readable_filters()?; + let chain = self.readable_filters().map_err(|e| (e, 0))?; if chain.is_empty() { - return self.raw_within_typed(max); + return self.raw_within_typed(max).map_err(|e| (e, 0)); } - pdfcraft_filters::decode(&chain, &self.raw, max) + pdfcraft_filters::decode_counted(&chain, &self.raw, max) } /// Data after the first filter alone, bounded by `max`: the rest of the chain stays encoded. diff --git a/crates/filters/src/lib.rs b/crates/filters/src/lib.rs index 87306166c..319796f75 100644 --- a/crates/filters/src/lib.rs +++ b/crates/filters/src/lib.rs @@ -140,6 +140,12 @@ pub(crate) type Step = Result, Failure>; /// [`FilterError::Unsupported`] (callers keep such data encoded), as does a chain longer than /// [`MAX_CHAIN`]. `max_output` bounds every intermediate buffer. pub fn decode(chain: &[(Filter, Params)], data: &[u8], max_output: usize) -> Result, FilterError> { + decode_counted(chain, data, max_output).map_err(|(e, _)| e) +} + +/// Like [`decode`], but a failure also gives the bytes decoded before it, across the stages +/// that ran: the work a caller charging an allowance spent finding out. +pub fn decode_counted(chain: &[(Filter, Params)], data: &[u8], max_output: usize) -> Result, (FilterError, usize)> { run(chain, data, max_output, true).map(|(v, _)| v) } @@ -147,22 +153,24 @@ pub fn decode(chain: &[(Filter, Params)], data: &[u8], max_output: usize) -> Res /// reports `partial = true` (the behaviour viewers expect). The remaining filters of the /// chain still run on the partial data. Limit and unsupported errors are still errors. pub fn decode_tolerant(chain: &[(Filter, Params)], data: &[u8], max_output: usize) -> Result<(Vec, bool), FilterError> { - run(chain, data, max_output, false) + run(chain, data, max_output, false).map_err(|(e, _)| e) } /// Longest filter chain decoded. Real streams use one to three filters; every stage may produce /// up to the output limit, so the chain length multiplies the work. pub const MAX_CHAIN: usize = 8; -fn run(chain: &[(Filter, Params)], data: &[u8], max: usize, strict: bool) -> Result<(Vec, bool), FilterError> { +/// The decoded data and whether it is partial, or the error and the bytes decoded before it. +fn run(chain: &[(Filter, Params)], data: &[u8], max: usize, strict: bool) -> Result<(Vec, bool), (FilterError, usize)> { if chain.len() > MAX_CHAIN { - return Err(FilterError::Unsupported(format!("a chain of {} filters", chain.len()))); + return Err((FilterError::Unsupported(format!("a chain of {} filters", chain.len())), 0)); } if let Some((f, _)) = chain.iter().find(|(f, _)| !f.is_decodable()) { - return Err(FilterError::Unsupported(f.name().to_owned())); + return Err((FilterError::Unsupported(f.name().to_owned()), 0)); } let mut cur: Cow<[u8]> = Cow::Borrowed(data); let mut partial = false; + let mut spent = 0usize; for (filter, params) in chain { #[cfg(test)] STAGE_BYTES.with(|c| c.set(c.get() + cur.len())); @@ -172,8 +180,9 @@ fn run(chain: &[(Filter, Params)], data: &[u8], max: usize, strict: bool) -> Res partial = true; out } - Err(f) => return Err(f.error), + Err(f) => return Err((f.error, spent.saturating_add(f.partial.len()))), }); + spent = spent.saturating_add(cur.len()); } Ok((cur.into_owned(), partial)) } From 11d2a576d6d27c9445c144f91b6560d74add3f17 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 21:40:10 +0000 Subject: [PATCH 19/21] Filters README lists decode_counted --- crates/filters/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/filters/README.md b/crates/filters/README.md index fc5306e32..f5eb412d7 100644 --- a/crates/filters/README.md +++ b/crates/filters/README.md @@ -17,6 +17,7 @@ pub struct Params { predictor, colors, bits_per_component, columns, early_change pub enum FilterError { Corrupt { filter, detail }, LimitExceeded(usize), Unsupported(String) } decode(chain, data, max_output) -> Result, FilterError> +decode_counted(chain, data, max_output) -> Result, (FilterError, usize /*decoded before the error*/)> decode_tolerant(chain, data, max_output) -> Result<(Vec, bool /*partial*/), FilterError> encode(filter, params, data) -> Result, FilterError> encode_flate(data) -> Vec // zlib, level 6 @@ -25,6 +26,7 @@ encode_flate(data) -> Vec // zlib, level 6 - `chain` is applied first to last, as in a `/Filter` array. - Image codecs (DCT, JPX, JBIG2, CCITT), `Crypt` and unknown filters anywhere in a chain give `Unsupported`: callers keep such data encoded. So does a chain longer than `MAX_CHAIN` (8) filters, before any stage runs. Decryption belongs to `crypt`. - `max_output` bounds every intermediate buffer (including the pre-predictor buffer) and gives `LimitExceeded(max_output)`. This also holds when Flate retries raw deflate after a misleading zlib header, in both strict and tolerant decoding. +- `decode_counted` is `decode` that also gives, on failure, the bytes the stages decoded before it, so a caller charging a budget (`pdfcraft_cos::DecodeBudget`) pays for a failed decode too. - `decode_tolerant` turns `Corrupt` into "what was decoded so far" plus `partial = true`, then runs the rest of the chain on it (predictors are applied to partial output too). `LimitExceeded` and `Unsupported` are still errors. ## Behaviour From 0c09000f7b305cbb5461f26989dc487a1e5b44d0 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 22:00:51 +0000 Subject: [PATCH 20/21] A decode charges the page's decode budget for every filter stage's output --- crates/cos/src/budget.rs | 52 +++++++++++++++++++++++++-------------- crates/cos/src/object.rs | 34 ++++++++++++++++--------- crates/filters/README.md | 5 ++-- crates/filters/src/lib.rs | 26 +++++++++++++------- 4 files changed, 77 insertions(+), 40 deletions(-) diff --git a/crates/cos/src/budget.rs b/crates/cos/src/budget.rs index b31d10391..1ddd42eaf 100644 --- a/crates/cos/src/budget.rs +++ b/crates/cos/src/budget.rs @@ -12,14 +12,9 @@ pub const MAX_PAGE_DECODED: usize = 256 << 20; /// Form XObjects one page's content walk may enter, each entry counted again. pub const MAX_PAGE_FORMS: usize = 50_000; -/// A stream decode within a byte bound: the data, or the error and the bytes decoded before it. -type Decoder = fn(&Stream, usize) -> Result, (FilterError, usize)>; - -/// A tolerant decode recovers corrupt data as partial output, so its only errors are a spent -/// limit and an unsupported filter, which is refused before any decoding. -fn tolerant(stream: &Stream, max: usize) -> Result, (FilterError, usize)> { - stream.decoded_within_typed(max).map_err(|e| (e, 0)) -} +/// A stream decode within a byte bound, with the bytes its filter stages decoded: the data or +/// the error, each with that count. +type Decoder = fn(&Stream, usize) -> Result<(Vec, usize), (FilterError, usize)>; /// What one operation may still decode and enter across all the streams it reads. /// @@ -62,24 +57,25 @@ impl DecodeBudget { /// `stream` decoded like [`Stream::decoded`], charged to the allowance. Fails, without /// decoding, once the allowance is spent. pub fn decode(&mut self, stream: &Stream) -> Result, CosError> { - self.charge(stream, usize::MAX, tolerant) + self.charge(stream, usize::MAX, Stream::decoded_within_counted) } /// Like [`DecodeBudget::decode`], for a stream with a tighter bound of its own: decoding /// past `max` bytes is an error that charges `max`, the work it took to find out. pub fn decode_within(&mut self, stream: &Stream, max: usize) -> Result, CosError> { - self.charge(stream, max, tolerant) + self.charge(stream, max, Stream::decoded_within_counted) } /// `stream` decoded like [`Stream::decoded_strict`] (any corruption is an error), charged to /// the allowance. pub fn decode_strict(&mut self, stream: &Stream) -> Result, CosError> { - self.charge(stream, usize::MAX, Stream::decoded_strict_within_typed) + self.charge(stream, usize::MAX, Stream::decoded_strict_within_counted) } - /// Decodes `stream` within `max` and the allowance, charging what it decoded: a corrupt - /// stream pays for the bytes decoded before the error, so referencing it again and again - /// spends the allowance like any other stream. + /// Decodes `stream` within `max` and the allowance, charging what every filter stage + /// decoded: a stage's output counts even when a later stage shrinks it, and a corrupt stream + /// pays for the bytes decoded before the error, so referencing a stream again and again + /// spends the allowance by the work each decode does. fn charge(&mut self, stream: &Stream, max: usize, decode: Decoder) -> Result, CosError> { if self.bytes == 0 && !stream.raw.is_empty() { return Err(CosError::Filter(FilterError::LimitExceeded(self.limit).to_string())); @@ -87,16 +83,16 @@ impl DecodeBudget { if max < self.bytes { let result = decode(stream, max); let spent = match &result { - Ok(data) => data.len(), + Ok((_, spent)) => *spent, Err((FilterError::LimitExceeded(_), _)) => max, Err((_, spent)) => *spent, }; self.bytes = self.bytes.saturating_sub(spent); - return result.map_err(|(e, _)| CosError::Filter(e.to_string())); + return result.map(|(data, _)| data).map_err(|(e, _)| CosError::Filter(e.to_string())); } match decode(stream, self.bytes) { - Ok(data) => { - self.bytes = self.bytes.saturating_sub(data.len()); + Ok((data, spent)) => { + self.bytes = self.bytes.saturating_sub(spent); Ok(data) } Err((FilterError::LimitExceeded(_), _)) => { @@ -201,4 +197,24 @@ mod tests { assert!(budget.decode_within(&corrupt, 1 << 30).is_ok()); assert_eq!(budget.remaining(), (10 << 20) - partial); } + + /// A chain whose later stage shrinks the data pays for every stage's output: a Flate stage + /// that inflates to hex text, which ASCIIHex then halves, charges both. + #[test] + fn decodes_charge_every_stage_of_a_chain() { + let hex: Vec = std::iter::repeat_n(b"ab".as_slice(), 1 << 19).flatten().copied().collect(); + let flate = Stream::flate(Dict::new(), &hex); + let mut dict = Dict::new(); + dict.set(b"Filter".to_vec(), Object::Array(vec![Object::name("FlateDecode"), Object::name("ASCIIHexDecode")])); + let chained = Stream::from_raw(dict, flate.raw.to_vec()); + assert_eq!(chained.decoded().map(|d| d.len()).ok(), Some(1 << 19)); + let charged = (1 << 20) + (1 << 19); + let mut budget = DecodeBudget::new(10 << 20, 1); + assert_eq!(budget.decode(&chained).map(|d| d.len()).ok(), Some(1 << 19)); + assert_eq!(budget.remaining(), (10 << 20) - charged); + assert!(budget.decode_within(&chained, 2 << 20).is_ok()); + assert_eq!(budget.remaining(), (10 << 20) - 2 * charged); + assert!(budget.decode_strict(&chained).is_ok()); + assert_eq!(budget.remaining(), (10 << 20) - 3 * charged); + } } diff --git a/crates/cos/src/object.rs b/crates/cos/src/object.rs index 0660d5a26..c09a8ffe3 100644 --- a/crates/cos/src/object.rs +++ b/crates/cos/src/object.rs @@ -283,32 +283,44 @@ impl Stream { /// Like [`Stream::decoded`], for a stream whose decoded size has a tighter bound than /// [`MAX_DECODED`]: decoding past `max` bytes is an error. pub fn decoded_within(&self, max: usize) -> Result, CosError> { - self.decoded_within_typed(max).map_err(|e| CosError::Filter(e.to_string())) + self.decoded_within_counted(max).map(|(v, _)| v).map_err(|(e, _)| CosError::Filter(e.to_string())) } /// [`Stream::decoded_within`] with the filter error kept typed, so a caller can tell a - /// spent limit from corrupt data. - pub(crate) fn decoded_within_typed(&self, max: usize) -> Result, pdfcraft_filters::FilterError> { + /// spent limit from corrupt data, and the bytes every filter stage decoded (the work done, + /// on success and on failure). + pub(crate) fn decoded_within_counted(&self, max: usize) -> Result<(Vec, usize), (pdfcraft_filters::FilterError, usize)> { let max = max.min(MAX_DECODED); - let chain = self.readable_filters()?; + let chain = self.readable_filters().map_err(|e| (e, 0))?; if chain.is_empty() { - return self.raw_within_typed(max); + return self + .raw_within_typed(max) + .map(|v| { + let n = v.len(); + (v, n) + }) + .map_err(|e| (e, 0)); } - pdfcraft_filters::decode_tolerant(&chain, &self.raw, max).map(|(v, _)| v) + pdfcraft_filters::decode_tolerant_counted(&chain, &self.raw, max).map(|(v, _, spent)| (v, spent)) } /// Decoded data, bounded by [`MAX_DECODED`]; any corruption is an error. pub fn decoded_strict(&self) -> Result, CosError> { - self.decoded_strict_within_typed(MAX_DECODED).map_err(|(e, _)| CosError::Filter(e.to_string())) + self.decoded_strict_within_counted(MAX_DECODED).map(|(v, _)| v).map_err(|(e, _)| CosError::Filter(e.to_string())) } - /// [`Stream::decoded_strict`] bounded by `max`, with the filter error kept typed and the - /// bytes decoded before it. - pub(crate) fn decoded_strict_within_typed(&self, max: usize) -> Result, (pdfcraft_filters::FilterError, usize)> { + /// [`Stream::decoded_strict`] bounded by `max`, counted like [`Stream::decoded_within_counted`]. + pub(crate) fn decoded_strict_within_counted(&self, max: usize) -> Result<(Vec, usize), (pdfcraft_filters::FilterError, usize)> { let max = max.min(MAX_DECODED); let chain = self.readable_filters().map_err(|e| (e, 0))?; if chain.is_empty() { - return self.raw_within_typed(max).map_err(|e| (e, 0)); + return self + .raw_within_typed(max) + .map(|v| { + let n = v.len(); + (v, n) + }) + .map_err(|e| (e, 0)); } pdfcraft_filters::decode_counted(&chain, &self.raw, max) } diff --git a/crates/filters/README.md b/crates/filters/README.md index f5eb412d7..be5151733 100644 --- a/crates/filters/README.md +++ b/crates/filters/README.md @@ -17,8 +17,9 @@ pub struct Params { predictor, colors, bits_per_component, columns, early_change pub enum FilterError { Corrupt { filter, detail }, LimitExceeded(usize), Unsupported(String) } decode(chain, data, max_output) -> Result, FilterError> -decode_counted(chain, data, max_output) -> Result, (FilterError, usize /*decoded before the error*/)> +decode_counted(chain, data, max_output) -> Result<(Vec, usize /*decoded*/), (FilterError, usize /*decoded*/)> decode_tolerant(chain, data, max_output) -> Result<(Vec, bool /*partial*/), FilterError> +decode_tolerant_counted(chain, data, max_output) -> Result<(Vec, bool /*partial*/, usize /*decoded*/), (FilterError, usize /*decoded*/)> encode(filter, params, data) -> Result, FilterError> encode_flate(data) -> Vec // zlib, level 6 ``` @@ -26,7 +27,7 @@ encode_flate(data) -> Vec // zlib, level 6 - `chain` is applied first to last, as in a `/Filter` array. - Image codecs (DCT, JPX, JBIG2, CCITT), `Crypt` and unknown filters anywhere in a chain give `Unsupported`: callers keep such data encoded. So does a chain longer than `MAX_CHAIN` (8) filters, before any stage runs. Decryption belongs to `crypt`. - `max_output` bounds every intermediate buffer (including the pre-predictor buffer) and gives `LimitExceeded(max_output)`. This also holds when Flate retries raw deflate after a misleading zlib header, in both strict and tolerant decoding. -- `decode_counted` is `decode` that also gives, on failure, the bytes the stages decoded before it, so a caller charging a budget (`pdfcraft_cos::DecodeBudget`) pays for a failed decode too. +- `decode_counted` and `decode_tolerant_counted` also give the bytes every stage that ran decoded, on success and on failure, so a caller charging a budget (`pdfcraft_cos::DecodeBudget`) pays for the work: a stage's output counts even when a later stage shrinks it, and a failed decode pays for what it decoded first. - `decode_tolerant` turns `Corrupt` into "what was decoded so far" plus `partial = true`, then runs the rest of the chain on it (predictors are applied to partial output too). `LimitExceeded` and `Unsupported` are still errors. ## Behaviour diff --git a/crates/filters/src/lib.rs b/crates/filters/src/lib.rs index 319796f75..11dc473d3 100644 --- a/crates/filters/src/lib.rs +++ b/crates/filters/src/lib.rs @@ -140,28 +140,36 @@ pub(crate) type Step = Result, Failure>; /// [`FilterError::Unsupported`] (callers keep such data encoded), as does a chain longer than /// [`MAX_CHAIN`]. `max_output` bounds every intermediate buffer. pub fn decode(chain: &[(Filter, Params)], data: &[u8], max_output: usize) -> Result, FilterError> { - decode_counted(chain, data, max_output).map_err(|(e, _)| e) + decode_counted(chain, data, max_output).map(|(v, _)| v).map_err(|(e, _)| e) } -/// Like [`decode`], but a failure also gives the bytes decoded before it, across the stages -/// that ran: the work a caller charging an allowance spent finding out. -pub fn decode_counted(chain: &[(Filter, Params)], data: &[u8], max_output: usize) -> Result, (FilterError, usize)> { - run(chain, data, max_output, true).map(|(v, _)| v) +/// Like [`decode`], also giving the bytes every stage that ran decoded, on success and on +/// failure: the work a caller charging an allowance spent. A stage's output counts even when a +/// later stage shrinks it. +pub fn decode_counted(chain: &[(Filter, Params)], data: &[u8], max_output: usize) -> Result<(Vec, usize), (FilterError, usize)> { + run(chain, data, max_output, true).map(|(v, _, spent)| (v, spent)) } /// Like [`decode`], but on corrupt data returns what was decoded before the error and /// reports `partial = true` (the behaviour viewers expect). The remaining filters of the /// chain still run on the partial data. Limit and unsupported errors are still errors. pub fn decode_tolerant(chain: &[(Filter, Params)], data: &[u8], max_output: usize) -> Result<(Vec, bool), FilterError> { - run(chain, data, max_output, false).map_err(|(e, _)| e) + decode_tolerant_counted(chain, data, max_output).map(|(v, partial, _)| (v, partial)).map_err(|(e, _)| e) +} + +/// [`decode_tolerant`], also giving the bytes every stage that ran decoded, as +/// [`decode_counted`] does. +pub fn decode_tolerant_counted(chain: &[(Filter, Params)], data: &[u8], max_output: usize) -> Result<(Vec, bool, usize), (FilterError, usize)> { + run(chain, data, max_output, false) } /// Longest filter chain decoded. Real streams use one to three filters; every stage may produce /// up to the output limit, so the chain length multiplies the work. pub const MAX_CHAIN: usize = 8; -/// The decoded data and whether it is partial, or the error and the bytes decoded before it. -fn run(chain: &[(Filter, Params)], data: &[u8], max: usize, strict: bool) -> Result<(Vec, bool), (FilterError, usize)> { +/// The decoded data, whether it is partial and the bytes all stages decoded, or the error and +/// the bytes decoded before it. +fn run(chain: &[(Filter, Params)], data: &[u8], max: usize, strict: bool) -> Result<(Vec, bool, usize), (FilterError, usize)> { if chain.len() > MAX_CHAIN { return Err((FilterError::Unsupported(format!("a chain of {} filters", chain.len())), 0)); } @@ -184,7 +192,7 @@ fn run(chain: &[(Filter, Params)], data: &[u8], max: usize, strict: bool) -> Res }); spent = spent.saturating_add(cur.len()); } - Ok((cur.into_owned(), partial)) + Ok((cur.into_owned(), partial, spent)) } fn decode_one(filter: &Filter, params: &Params, data: &[u8], max: usize) -> Step { From 03366c3c7072fa49a5c1fadf8287cc750faf5a99 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 23:33:37 +0000 Subject: [PATCH 21/21] A counted decode bounds all filter stages together and counts output before the predictor --- crates/cos/src/object.rs | 46 +++++++++++-------- crates/filters/README.md | 2 +- crates/filters/src/lib.rs | 95 +++++++++++++++++++++++++++++---------- 3 files changed, 100 insertions(+), 43 deletions(-) diff --git a/crates/cos/src/object.rs b/crates/cos/src/object.rs index c09a8ffe3..c9b2ff9a3 100644 --- a/crates/cos/src/object.rs +++ b/crates/cos/src/object.rs @@ -283,13 +283,30 @@ impl Stream { /// Like [`Stream::decoded`], for a stream whose decoded size has a tighter bound than /// [`MAX_DECODED`]: decoding past `max` bytes is an error. pub fn decoded_within(&self, max: usize) -> Result, CosError> { - self.decoded_within_counted(max).map(|(v, _)| v).map_err(|(e, _)| CosError::Filter(e.to_string())) + let decode = |chain: &[(Filter, Params)], data: &[u8], max| { + pdfcraft_filters::decode_tolerant(chain, data, max).map(|(v, _)| (v, 0)).map_err(|e| (e, 0)) + }; + self.decode_chain(max, decode).map(|(v, _)| v).map_err(|(e, _)| CosError::Filter(e.to_string())) } - /// [`Stream::decoded_within`] with the filter error kept typed, so a caller can tell a - /// spent limit from corrupt data, and the bytes every filter stage decoded (the work done, - /// on success and on failure). + /// [`Stream::decoded_within`] for a caller charging an allowance: the filter error is kept + /// typed, so a caller can tell a spent limit from corrupt data, `max` bounds all filter + /// stages together, and the result gives the bytes they decoded (the work done, on success + /// and on failure). pub(crate) fn decoded_within_counted(&self, max: usize) -> Result<(Vec, usize), (pdfcraft_filters::FilterError, usize)> { + let decode = |chain: &[(Filter, Params)], data: &[u8], max| { + pdfcraft_filters::decode_tolerant_counted(chain, data, max).map(|(v, _, spent)| (v, spent)) + }; + self.decode_chain(max, decode) + } + + /// The stream decoded through its filters by `decode`, or copied when it has none, within + /// `max` and [`MAX_DECODED`]. + fn decode_chain( + &self, + max: usize, + decode: impl FnOnce(&[(Filter, Params)], &[u8], usize) -> Result<(Vec, usize), (pdfcraft_filters::FilterError, usize)>, + ) -> Result<(Vec, usize), (pdfcraft_filters::FilterError, usize)> { let max = max.min(MAX_DECODED); let chain = self.readable_filters().map_err(|e| (e, 0))?; if chain.is_empty() { @@ -301,28 +318,19 @@ impl Stream { }) .map_err(|e| (e, 0)); } - pdfcraft_filters::decode_tolerant_counted(&chain, &self.raw, max).map(|(v, _, spent)| (v, spent)) + decode(&chain, &self.raw, max) } /// Decoded data, bounded by [`MAX_DECODED`]; any corruption is an error. pub fn decoded_strict(&self) -> Result, CosError> { - self.decoded_strict_within_counted(MAX_DECODED).map(|(v, _)| v).map_err(|(e, _)| CosError::Filter(e.to_string())) + let decode = |chain: &[(Filter, Params)], data: &[u8], max| pdfcraft_filters::decode(chain, data, max).map(|v| (v, 0)).map_err(|e| (e, 0)); + self.decode_chain(MAX_DECODED, decode).map(|(v, _)| v).map_err(|(e, _)| CosError::Filter(e.to_string())) } - /// [`Stream::decoded_strict`] bounded by `max`, counted like [`Stream::decoded_within_counted`]. + /// [`Stream::decoded_strict`] bounded by `max` and counted like + /// [`Stream::decoded_within_counted`]. pub(crate) fn decoded_strict_within_counted(&self, max: usize) -> Result<(Vec, usize), (pdfcraft_filters::FilterError, usize)> { - let max = max.min(MAX_DECODED); - let chain = self.readable_filters().map_err(|e| (e, 0))?; - if chain.is_empty() { - return self - .raw_within_typed(max) - .map(|v| { - let n = v.len(); - (v, n) - }) - .map_err(|e| (e, 0)); - } - pdfcraft_filters::decode_counted(&chain, &self.raw, max) + self.decode_chain(max, pdfcraft_filters::decode_counted) } /// Data after the first filter alone, bounded by `max`: the rest of the chain stays encoded. diff --git a/crates/filters/README.md b/crates/filters/README.md index be5151733..959180d87 100644 --- a/crates/filters/README.md +++ b/crates/filters/README.md @@ -27,7 +27,7 @@ encode_flate(data) -> Vec // zlib, level 6 - `chain` is applied first to last, as in a `/Filter` array. - Image codecs (DCT, JPX, JBIG2, CCITT), `Crypt` and unknown filters anywhere in a chain give `Unsupported`: callers keep such data encoded. So does a chain longer than `MAX_CHAIN` (8) filters, before any stage runs. Decryption belongs to `crypt`. - `max_output` bounds every intermediate buffer (including the pre-predictor buffer) and gives `LimitExceeded(max_output)`. This also holds when Flate retries raw deflate after a misleading zlib header, in both strict and tolerant decoding. -- `decode_counted` and `decode_tolerant_counted` also give the bytes every stage that ran decoded, on success and on failure, so a caller charging a budget (`pdfcraft_cos::DecodeBudget`) pays for the work: a stage's output counts even when a later stage shrinks it, and a failed decode pays for what it decoded first. +- `decode_counted` and `decode_tolerant_counted` are for a caller charging a budget (`pdfcraft_cos::DecodeBudget`). `max_output` bounds all stages together (each stage gets what the earlier ones left), and they give the bytes every stage that ran decoded, on success and on failure. A stage's output counts as its decoder produced it, before a predictor or a later stage shrinks it, and a failed decode pays for what it decoded first. - `decode_tolerant` turns `Corrupt` into "what was decoded so far" plus `partial = true`, then runs the rest of the chain on it (predictors are applied to partial output too). `LimitExceeded` and `Unsupported` are still errors. ## Behaviour diff --git a/crates/filters/src/lib.rs b/crates/filters/src/lib.rs index 11dc473d3..22a78aefd 100644 --- a/crates/filters/src/lib.rs +++ b/crates/filters/src/lib.rs @@ -140,27 +140,27 @@ pub(crate) type Step = Result, Failure>; /// [`FilterError::Unsupported`] (callers keep such data encoded), as does a chain longer than /// [`MAX_CHAIN`]. `max_output` bounds every intermediate buffer. pub fn decode(chain: &[(Filter, Params)], data: &[u8], max_output: usize) -> Result, FilterError> { - decode_counted(chain, data, max_output).map(|(v, _)| v).map_err(|(e, _)| e) + run(chain, data, max_output, true, false).map(|(v, _, _)| v).map_err(|(e, _)| e) } -/// Like [`decode`], also giving the bytes every stage that ran decoded, on success and on -/// failure: the work a caller charging an allowance spent. A stage's output counts even when a -/// later stage shrinks it. +/// Like [`decode`] for a caller charging an allowance: `max_output` bounds the bytes all stages +/// decode together, each stage getting what the earlier ones left, and the result gives those +/// bytes, on success and on failure. A stage's output counts even when a later stage or its +/// predictor shrinks it. pub fn decode_counted(chain: &[(Filter, Params)], data: &[u8], max_output: usize) -> Result<(Vec, usize), (FilterError, usize)> { - run(chain, data, max_output, true).map(|(v, _, spent)| (v, spent)) + run(chain, data, max_output, true, true).map(|(v, _, spent)| (v, spent)) } /// Like [`decode`], but on corrupt data returns what was decoded before the error and /// reports `partial = true` (the behaviour viewers expect). The remaining filters of the /// chain still run on the partial data. Limit and unsupported errors are still errors. pub fn decode_tolerant(chain: &[(Filter, Params)], data: &[u8], max_output: usize) -> Result<(Vec, bool), FilterError> { - decode_tolerant_counted(chain, data, max_output).map(|(v, partial, _)| (v, partial)).map_err(|(e, _)| e) + run(chain, data, max_output, false, false).map(|(v, partial, _)| (v, partial)).map_err(|(e, _)| e) } -/// [`decode_tolerant`], also giving the bytes every stage that ran decoded, as -/// [`decode_counted`] does. +/// [`decode_tolerant`] bounded and counted as [`decode_counted`] is. pub fn decode_tolerant_counted(chain: &[(Filter, Params)], data: &[u8], max_output: usize) -> Result<(Vec, bool, usize), (FilterError, usize)> { - run(chain, data, max_output, false) + run(chain, data, max_output, false, true) } /// Longest filter chain decoded. Real streams use one to three filters; every stage may produce @@ -168,8 +168,9 @@ pub fn decode_tolerant_counted(chain: &[(Filter, Params)], data: &[u8], max_outp pub const MAX_CHAIN: usize = 8; /// The decoded data, whether it is partial and the bytes all stages decoded, or the error and -/// the bytes decoded before it. -fn run(chain: &[(Filter, Params)], data: &[u8], max: usize, strict: bool) -> Result<(Vec, bool, usize), (FilterError, usize)> { +/// the bytes decoded before it. `max` bounds each stage, or with `cumulative` all of them +/// together. +fn run(chain: &[(Filter, Params)], data: &[u8], max: usize, strict: bool, cumulative: bool) -> Result<(Vec, bool, usize), (FilterError, usize)> { if chain.len() > MAX_CHAIN { return Err((FilterError::Unsupported(format!("a chain of {} filters", chain.len())), 0)); } @@ -182,33 +183,49 @@ fn run(chain: &[(Filter, Params)], data: &[u8], max: usize, strict: bool) -> Res for (filter, params) in chain { #[cfg(test)] STAGE_BYTES.with(|c| c.set(c.get() + cur.len())); - cur = Cow::Owned(match decode_one(filter, params, &cur, max) { + let limit = if cumulative { max.saturating_sub(spent) } else { max }; + let (step, work) = decode_one(filter, params, &cur, limit); + spent = spent.saturating_add(work); + cur = Cow::Owned(match step { Ok(v) => v, Err(Failure { error: FilterError::Corrupt { .. }, partial: out }) if !strict => { partial = true; out } - Err(f) => return Err((f.error, spent.saturating_add(f.partial.len()))), + Err(Failure { error: FilterError::LimitExceeded(_), .. }) => return Err((FilterError::LimitExceeded(max), spent)), + Err(f) => return Err((f.error, spent)), }); - spent = spent.saturating_add(cur.len()); } Ok((cur.into_owned(), partial, spent)) } -fn decode_one(filter: &Filter, params: &Params, data: &[u8], max: usize) -> Step { +/// One stage's result and the bytes its decoder produced (before any predictor). +fn decode_one(filter: &Filter, params: &Params, data: &[u8], max: usize) -> (Step, usize) { match filter { Filter::Flate => with_predictor(flate::decode(data, max), params, "FlateDecode"), Filter::Lzw => with_predictor(lzw::decode(data, params.early_change != 0, max), params, "LZWDecode"), - Filter::AsciiHex => asciihex::decode(data, max), - Filter::Ascii85 => ascii85::decode(data, max), - Filter::RunLength => runlength::decode(data, max), - other => Err(Failure { error: FilterError::Unsupported(other.name().to_owned()), partial: Vec::new() }), + Filter::AsciiHex => counted(asciihex::decode(data, max)), + Filter::Ascii85 => counted(ascii85::decode(data, max)), + Filter::RunLength => counted(runlength::decode(data, max)), + other => (Err(Failure { error: FilterError::Unsupported(other.name().to_owned()), partial: Vec::new() }), 0), } } -/// Applies the predictor to a decoder's result, including to partial output on failure. -fn with_predictor(step: Step, params: &Params, name: &'static str) -> Step { - match step { +/// A step with the bytes it produced, its output or its partial output. +fn counted(step: Step) -> (Step, usize) { + let n = match &step { + Ok(v) => v.len(), + Err(f) => f.partial.len(), + }; + (step, n) +} + +/// Applies the predictor to a decoder's result, including to partial output on failure. The +/// count is the decoder's output: a PNG predictor drops a tag byte per row, but the decoder +/// produced it. +fn with_predictor(step: Step, params: &Params, name: &'static str) -> (Step, usize) { + let (step, n) = counted(step); + let step = match step { Ok(v) => predictor::decode(params, v, name), Err(Failure { error, partial }) => { let partial = match predictor::decode(params, partial, name) { @@ -217,7 +234,8 @@ fn with_predictor(step: Step, params: &Params, name: &'static str) -> Step { }; Err(Failure { error, partial }) } - } + }; + (step, n) } /// Encodes `data` with `filter`. Supported: Flate (zlib, level 6), LZW, ASCIIHex, @@ -388,4 +406,35 @@ mod tests { assert!(partial); assert_eq!(&data[..out.len()], &out[..]); } + + /// A counted decode bounds what all stages decode together: Flate inflating 1 MiB of hex + /// that ASCIIHex halves is 1.5 MiB of work, past a 1.25 MiB allowance. A plain decode keeps + /// the limit per stage. + #[test] + fn a_counted_decode_bounds_all_stages_together() { + let hex: Vec = std::iter::repeat_n(b"ab".as_slice(), 1 << 19).flatten().copied().collect(); + let data = encode_flate(&hex); + let chain = [(Filter::Flate, Params::default()), (Filter::AsciiHex, Params::default())]; + let max = (1 << 20) + (1 << 18); + assert_eq!(decode(&chain, &data, max).map(|d| d.len()).ok(), Some(1 << 19)); + assert!(matches!(decode_counted(&chain, &data, max), Err((FilterError::LimitExceeded(m), _)) if m == max)); + assert_eq!(decode_counted(&chain, &data, 2 << 20).map(|(_, spent)| spent).ok(), Some((1 << 20) + (1 << 19))); + } + + /// A PNG predictor drops a tag byte per row, but Flate produced it: the count includes it, + /// also when the predictor fails on a bad tag. + #[test] + fn a_counted_decode_counts_output_before_the_predictor() { + let params = Params { predictor: 12, ..Params::default() }; + let rows = vec![7u8; 1 << 16]; + let data = encode(&Filter::Flate, ¶ms, &rows).unwrap(); + let chain = [(Filter::Flate, params.clone())]; + let (out, spent) = decode_counted(&chain, &data, usize::MAX).unwrap(); + assert!(out == rows); + assert_eq!(spent, 2 << 16); + let bad: Vec = std::iter::repeat_n([0u8, 7], 1 << 15).flatten().chain([9, 7]).collect(); + let (e, spent) = decode_counted(&chain, &encode_flate(&bad), usize::MAX).unwrap_err(); + assert!(matches!(e, FilterError::Corrupt { .. }), "{e}"); + assert_eq!(spent, bad.len()); + } }