diff --git a/crates/annot/src/draw.rs b/crates/annot/src/draw.rs index 85eab4a487..cda0e6992b 100644 --- a/crates/annot/src/draw.rs +++ b/crates/annot/src/draw.rs @@ -79,13 +79,15 @@ const LINK_KEYS: [&[u8]; 5] = [b"P", b"Popup", b"IRT", b"Parent", b"AP"]; /// arrays, `/Border` may end in a dash array. const INLINE_DEPTH: usize = 4; -/// Values [`with_direct_values`] copies out of referenced arrays and dictionaries for one -/// annotation, so a file can't have a large object copied for every entry that points at it. -const INLINE_BUDGET: usize = 1 << 16; +/// Values [`with_direct_values`] copies out of referenced arrays and dictionaries, or walks +/// before finding one too large to copy, for one annotation, so a file can't have a large object +/// copied or scanned for every entry that points at it. +pub(crate) const INLINE_BUDGET: usize = 1 << 16; -/// Values copied for all the annotations one display pass draws +/// Values copied or walked for all the annotations one display pass draws /// ([`with_missing_appearances_cached`]), so a large object that many annotations point at is -/// not copied for each of them without bound. Past it, references are left as they are. +/// not copied, or scanned and found too large, for each of them without bound. Past it, +/// references are left as they are. pub(crate) const PASS_BUDGET: usize = 1 << 22; /// `d` with the references among the values [`appearance::build`] reads followed. Any value may be @@ -117,13 +119,18 @@ pub(crate) fn with_direct_values_within<'d>(doc: &Document, d: &'d Dict, pass: & } /// The number of values in `o`, counting into its arrays and dictionaries (not through -/// references, which are charged when followed), or `None` past `limit`. Walks at most `limit` -/// values and holds no more than `limit` waiting. -fn weight(o: &Object, limit: usize) -> Option { +/// references, which are charged when followed), or, past `limit`, `Err` with the values it +/// visited or queued before finding that out (at least one), so a rejected walk is charged too. +/// Walks at most `limit` values and holds no more than `limit` waiting. +fn weight(o: &Object, limit: usize) -> Result { let mut count = 0usize; let mut stack = vec![o]; while let Some(o) = stack.pop() { - count = count.checked_add(1).filter(|c| *c <= limit)?; + let spent = count.saturating_add(stack.len()).max(1); + count = match count.checked_add(1).filter(|c| *c <= limit) { + Some(c) => c, + None => return Err(spent), + }; let inner = match o { Object::Array(items) => items.len(), Object::Dict(entries) => entries.len(), @@ -131,7 +138,7 @@ fn weight(o: &Object, limit: usize) -> Option { }; // Every value waiting is counted later, so more than `limit` in all is over it already. if count.saturating_add(stack.len()).saturating_add(inner) > limit { - return None; + return Err(spent.saturating_add(1)); } match o { Object::Array(items) => stack.extend(items.iter()), @@ -139,7 +146,7 @@ fn weight(o: &Object, limit: usize) -> Option { _ => {} } } - Some(count) + Ok(count) } /// `o` with references followed `depth` levels into arrays and dictionaries, or `None` when there @@ -148,12 +155,24 @@ fn direct_value(doc: &Document, o: &Object, depth: usize, budget: &mut usize) -> let held; let (base, followed) = match o { Object::Ref(_) => { + // A spent budget follows nothing more, not even to find out what is there. + if *budget == 0 { + return None; + } held = doc.resolve(o); if matches!(&*held, Object::Stream(_) | Object::Ref(_)) { return None; } - // Everything copied counts, inner arrays and dictionaries too. - *budget = budget.checked_sub(weight(&held, *budget)?)?; + // Everything copied counts, inner arrays and dictionaries too, and so does the walk + // that finds an object too large: many references to one would otherwise each walk + // it again for free. + match weight(&held, *budget) { + Ok(n) => *budget = budget.checked_sub(n)?, + Err(spent) => { + *budget = budget.saturating_sub(spent); + return None; + } + } (&*held, true) } other => (other, false), diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index 132a69bb02..2bc8121612 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -1593,6 +1593,33 @@ fn following_references_is_bounded_per_value_and_per_pass() { assert_eq!(copies, crate::draw::PASS_BUDGET / 60_001, "copies stop when the pass budget is spent"); } +/// Finding an object too large to copy costs the walk that found it out: an array of two arrays +/// of 40,000 numbers that many annotations point at is walked about 40,000 values deep before it +/// is rejected, and that is charged to the annotation and to the pass, so after a bounded number +/// of such walks the pass has no room left to walk it again. A reference to it repeated in one annotation +/// is not walked again in full, and a small referenced colour after it is still read. +#[test] +fn a_rejected_large_object_is_charged_to_the_budgets() { + let mut doc = fixture(); + let inner = Object::Array(vec![Object::Int(0); 40_000]); + let nested = doc.add(Object::Array(vec![inner.clone(), inner])); + let red = doc.add(Object::Array(vec![Object::Int(1), Object::Int(0), Object::Int(0)])); + let mut d = Dict::new(); + d.set(b"Subtype".to_vec(), Object::name("Square")); + d.set(b"Junk".to_vec(), Object::Array(vec![Object::Ref(nested); 1_000])); + d.set(b"C".to_vec(), Object::Ref(red)); + let mut pass = crate::draw::PASS_BUDGET; + let direct = crate::draw::with_direct_values_within(&doc, &d, &mut pass); + assert_eq!(direct.get(b"C"), Some(&Object::Array(vec![Object::Int(1), Object::Int(0), Object::Int(0)]))); + let charged = crate::draw::PASS_BUDGET - pass; + assert!((40_000..=crate::draw::INLINE_BUDGET).contains(&charged), "the rejected walk is charged once: {charged}"); + d.set(b"Junk".to_vec(), Object::Ref(nested)); + for _ in 0..crate::draw::PASS_BUDGET / 40_000 { + crate::draw::with_direct_values_within(&doc, &d, &mut pass); + } + assert!(pass < 40_000, "after {} annotations the pass has no room for another full walk: {pass}", crate::draw::PASS_BUDGET / 40_000 + 1); +} + /// `/RD` is optional (ISO 32000-2 Table 177): a callout without it has its text box at `/Rect` /// and is drawn, box, text and leader line. A callout line and `/RD` given by reference are read too. #[test]