From 0ecdf2ff2dc28ea503cd843330a84117f08fd31a Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Thu, 22 Oct 2026 16:24:22 +0100 Subject: [PATCH] #3710: .doc floating shapes built once per spid; anchor copies charged to the work budget Finding 49. drawing::anchors built a fresh object for every PlcSpa entry, so one group of 1000 members named by 5000 entries built 5M children (1.4 s, ~1 GB) with the per-group child cap resetting each time. Word anchors each shape once, so a repeated spid is now skipped: every node of the shape tree is built at most once, bounded by the shared OfficeArt record budget. walk also cloned each floating object every time a story holding its anchor was walked; each copy is now charged by size (a group with all its members) to the document-wide MAX_WORK budget. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/docbin/src/drawing.rs | 6 ++++++ crates/docbin/src/lib.rs | 14 ++++++++++++-- crates/docbin/src/tests/drawing.rs | 24 ++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/crates/docbin/src/drawing.rs b/crates/docbin/src/drawing.rs index 461d6b372..f5c02699f 100644 --- a/crates/docbin/src/drawing.rs +++ b/crates/docbin/src/drawing.rs @@ -81,12 +81,18 @@ pub(crate) fn anchors( ) -> Vec<(u32, InlineObject)> { let Some(pair) = pair else { return Vec::new() }; let mut out = Vec::new(); + let mut seen = std::collections::HashSet::new(); for (cp, spa) in plc(table, pair, CB_SPA) { let i32_at = |o: usize| spa.get(o..o + 4).and_then(|b| b.try_into().ok()).map(i32::from_le_bytes).unwrap_or(0); let spid = i32_at(0) as u32; let [xl, yt, xr, yb] = [4, 8, 12, 16].map(i32_at); let flags = spa.get(20..22).map(|b| u16::from_le_bytes([b[0], b[1]])).unwrap_or(0); let Some(node) = shapes.get(&spid) else { continue }; + // Word anchors each shape once. A repeated spid would build another copy of the shape, + // and of every member of a group, per anchor (#3710). + if !seen.insert(spid) { + continue; + } let tw = |v: i64| (v as f32 / 20.0).clamp(-2.0 * MAX_PT, 2.0 * MAX_PT); let (mut x, mut y) = (tw(xl as i64), tw(yt as i64)); let (mut w, mut h) = (tw(xr as i64 - xl as i64).clamp(0.0, MAX_PT), tw(yb as i64 - yt as i64).clamp(0.0, MAX_PT)); diff --git a/crates/docbin/src/lib.rs b/crates/docbin/src/lib.rs index 2b1859132..4c6c0c441 100644 --- a/crates/docbin/src/lib.rs +++ b/crates/docbin/src/lib.rs @@ -50,8 +50,9 @@ const MAX_PARTS: usize = 50_000; /// WordDocument stream (see [`read`]); this caps even a maximal stream. const MAX_TOTAL_CHARS: u64 = 64 << 20; /// Bytes of repeated work per document beyond the text itself: piece Prm grpprls (Prcs, up -/// to 64 KB each) interpreted (#3692), hyperlink targets (#3708) and picture alt text (#3709) -/// copied into every run or anchor. Past it Prms are skipped and copies left empty. +/// to 64 KB each) interpreted (#3692), hyperlink targets (#3708), picture alt text (#3709) +/// and floating objects (#3710) copied into every run or anchor. Past it Prms are skipped, +/// copied strings left empty and floating objects dropped. const MAX_WORK: u64 = 64 << 20; /// `sprmCPicLocation`: the fc of a picture in the Data stream. const C_PIC_LOCATION: u16 = 0x6A03; @@ -609,7 +610,16 @@ fn walk<'a>(ctx: &WalkCtx<'a>, cp_start: u32, cp_end: u32, pics: &mut media::Pic '\u{8}' => { // Drawn object anchor: the floating shape or picture anchored at this CP. let at = ctx.drawn.partition_point(|(c, _)| *c < cp); + // Each copy (a story can be walked more than once) is charged to the work budget + // by size, a group with all its members (#3710). for (_, obj) in ctx.drawn.get(at..).unwrap_or_default().iter().take_while(|(c, _)| *c == cp) { + let parts = match obj { + InlineObject::Group { children, .. } => children.len().saturating_add(1), + _ => 1, + }; + let cost = (parts as u64).saturating_mul(std::mem::size_of::() as u64); + let Some(left) = ctx.work.get().checked_sub(cost) else { continue }; + ctx.work.set(left); pb.push('\u{FFFC}', CharProps::default()); pb.objects.push(obj.clone()); } diff --git a/crates/docbin/src/tests/drawing.rs b/crates/docbin/src/tests/drawing.rs index fc453d54c..49851883b 100644 --- a/crates/docbin/src/tests/drawing.rs +++ b/crates/docbin/src/tests/drawing.rs @@ -352,3 +352,27 @@ fn alt_text_copies_across_many_anchors_are_bounded() { assert!(total <= 64 << 20, "alt bytes {total}"); assert_eq!(alts.last().copied(), Some("")); } + +/// #3710: every PlcSpa entry naming a group built another copy of all its members. A shape is +/// anchored once; repeated spids are skipped. +#[test] +fn a_group_named_by_many_anchors_is_built_once() { + let rect = |l: i32| rec(0, 0, 0xF00F, &[l, 0, l + 10, 10].iter().flat_map(|v| v.to_le_bytes()).collect::>()); + let head = sp(&[fsp(0, 3000, 0x201), rec(1, 0, 0xF009, &[0i32, 0, 10_000, 10].iter().flat_map(|v| v.to_le_bytes()).collect::>())]); + let members: Vec> = (0..1000).map(|i| sp(&[fsp(1, 4000 + i as u32, 0xA02), rect(i * 10)])).collect(); + let group = container(0xF003, &[vec![head], members].concat()); + let art = art_content(&[], &[group]); + let n = 5000u32; + let cps: Vec = (1..=n + 1).collect(); + let spas: Vec = (0..n).flat_map(|_| spa(3000, [0, 0, 1440, 1440], spa_flags(0, 0, 3, false))).collect(); + let text = "A\u{8}\u{8}\u{8}B\r"; + let ccp = text.chars().count() as u32; + let spec = StructSpec { text, extra_pairs: vec![(40, plc(&cps, &spas)), (50, art)], ..structured_spec(text, ccp) }; + let t = std::time::Instant::now(); + let doc = read(&struct_doc(&spec)).expect("opens"); + assert!(t.elapsed().as_millis() < 1000, "opened in {:?}", t.elapsed()); + let p0 = doc.body.first().and_then(|b| b.as_para()).expect("para"); + let groups: Vec = + p0.objects.iter().filter_map(|o| if let InlineObject::Group { children, .. } = o { Some(children.len()) } else { None }).collect(); + assert_eq!(groups, [1000]); +}