From 6a238f9fd7563d8b2ce4f1f90652284fc7e76447 Mon Sep 17 00:00:00 2001 From: mtlcl410 Date: Sat, 7 Nov 2026 14:01:34 +0800 Subject: [PATCH] M5: finding a referenced object too large to copy is charged to the budgets (view.render-annotation-appearances) A referenced object rejected as over the per-annotation budget was walked up to that budget and charged nothing, so many annotations (or many references in one annotation) pointing at the same oversized nested array each walked it again, and PASS_BUDGET did not bound the work. The walk is now charged to the annotation's budget and through it to the pass, and a spent budget follows no further references. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyDBXygTf7jcqdsjcfn --- crates/annot/src/draw.rs | 45 ++++++++++++++++++++++++++++----------- crates/annot/src/tests.rs | 27 +++++++++++++++++++++++ 2 files changed, 59 insertions(+), 13 deletions(-) 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]