From c37b1509502cfcbc70c678adcde6f6c4cdab9ec5 Mon Sep 17 00:00:00 2001 From: Naseq Date: Sun, 1 Nov 2026 11:17:08 +0430 Subject: [PATCH] PPTX: parse each tags part once, cap copied tags, and bound cache (#1164) - Parse each tags part once per import and cache in `Imp::tags_by_part` - Cap total copied tags into the model at `MAX_TAG_BYTES` (64 MiB), charging each pair a fixed size_of::<(String, String)>() in addition to text - Bound the cache: once `tags_capped` is set, return early before parsing or caching any subsequent part - Tests covering deduplicated part copies, empty-tag pair budgets, and verifying unread tag parts are not parsed or cached after the budget is reached --- crates/pptx/src/lib.rs | 1 + crates/pptx/src/read.rs | 72 +++++++++++++++++++++-- crates/pptx/tests/it/tags.rs | 108 +++++++++++++++++++++++++++++++++++ 3 files changed, 175 insertions(+), 6 deletions(-) diff --git a/crates/pptx/src/lib.rs b/crates/pptx/src/lib.rs index 1b1dc5b4..be08c680 100644 --- a/crates/pptx/src/lib.rs +++ b/crates/pptx/src/lib.rs @@ -14,6 +14,7 @@ mod maps; mod opc; mod png; pub(crate) mod read; +pub use read::MAX_TAG_BYTES; mod tables; mod write; mod xml; diff --git a/crates/pptx/src/read.rs b/crates/pptx/src/read.rs index 917aa927..288e497f 100644 --- a/crates/pptx/src/read.rs +++ b/crates/pptx/src/read.rs @@ -40,6 +40,12 @@ pub struct Imp<'a> { pub kept_sidecar_bytes: usize, /// Cached kept sidecars by normalized chart part name. pub kept_charts: HashMap, + /// Add-in tags parts, parsed once each, by normalized part name. + pub tags_by_part: HashMap>, + /// Bytes of tags copied into the model so far (see [`MAX_TAG_BYTES`]). + pub tag_bytes: usize, + /// The tag budget ran out and the warning was given. + pub tags_capped: bool, } impl Imp<'_> { @@ -196,16 +202,64 @@ fn choice_has_model3d(el: &El, rels: &Rels) -> bool { rec(el, rels, 0) } +/// Bytes of tags one import copies into the model, every tags part and reference together: a +/// small compressed tags part listed by many shapes, or many times by one, must not grow without +/// bound (#1164). The package budgets (`opc::MAX_XML_PART`, `MAX_TOTAL`, #1283) bound what is +/// parsed; this bounds what is copied from it. +pub const MAX_TAG_BYTES: usize = 64 << 20; +/// What each copied pair costs besides its text, so empty pairs count too. +const TAG_PAIR_COST: usize = std::mem::size_of::<(String, String)>(); + /// Add-in tags named by `holder`'s `p:custDataLst`: the name/value pairs of each tags part. -pub fn tags(imp: &Imp, part: &Part, holder: &El) -> Vec<(String, String)> { +/// Each part is parsed once per import and counted once per holder. Once [`MAX_TAG_BYTES`] is +/// reached, no further tag parts are parsed or cached (#1164). +pub fn tags(imp: &mut Imp, part: &Part, holder: &El) -> Vec<(String, String)> { + if imp.tags_capped { + return vec![]; + } let Some(list) = holder.child("custDataLst") else { return vec![] }; let mut out = Vec::new(); + let mut seen = HashSet::new(); for t in list.children_named("tags").take(100) { - let Some(doc) = t.attr("r:id").and_then(|id| part.rels.get(id)).and_then(|r| imp.pkg.xml(&r.target)) else { continue }; - for tag in doc.root.children_named("tag").take(10_000) { - if let Some(name) = tag.attr("name") { - out.push((name.to_string(), tag.attr("val").unwrap_or("").to_string())); + if imp.tags_capped { + return out; + } + let Some(target) = t.attr("r:id").and_then(|id| part.rels.get(id)).map(|r| opc::norm(&r.target)) else { continue }; + if !seen.insert(target.clone()) { + continue; + } + let parsed = match imp.tags_by_part.get(&target) { + Some(p) => p.clone(), + None => { + if imp.tags_capped { + return out; + } + let p: Arc<[(String, String)]> = imp + .pkg + .xml(&target) + .map(|doc| { + doc.root + .children_named("tag") + .take(10_000) + .filter_map(|tag| Some((tag.attr("name")?.to_string(), tag.attr("val").unwrap_or("").to_string()))) + .collect() + }) + .unwrap_or_default(); + imp.tags_by_part.insert(target, p.clone()); + p } + }; + for (name, val) in parsed.iter() { + let n = imp.tag_bytes.saturating_add(TAG_PAIR_COST + name.len() + val.len()); + if n > MAX_TAG_BYTES { + if !imp.tags_capped { + log::warn!("pptx: add-in tags over {MAX_TAG_BYTES} bytes; the rest are dropped"); + imp.tags_capped = true; + } + return out; + } + imp.tag_bytes = n; + out.push((name.clone(), val.clone())); } } out @@ -389,6 +443,9 @@ pub fn import_theme(bytes: &[u8]) -> Result<(deckcraft_model::theme::Theme, Vec< vml_previews: HashMap::new(), kept_sidecar_bytes: 0, kept_charts: HashMap::new(), + tags_by_part: HashMap::new(), + tag_bytes: 0, + tags_capped: false, }; let part = Part::new(&pkg, &theme_path, &doc); let theme = theme::read_theme(&mut imp, &part, &doc.root); @@ -427,6 +484,9 @@ pub fn import_checked(bytes: &[u8]) -> Result<(Presentation, Vec), PptxE vml_previews: HashMap::new(), kept_sidecar_bytes: 0, kept_charts: HashMap::new(), + tags_by_part: HashMap::new(), + tag_bytes: 0, + tags_capped: false, }; // Slide ids first so hyperlinks anywhere can point at slides. let slide_ids = register_slides(&mut imp, &docs, &slide_entries, &mut lost_slides); @@ -441,7 +501,7 @@ pub fn import_checked(bytes: &[u8]) -> Result<(Presentation, Vec), PptxE let layout_by_part = read_masters(&mut imp, &mut p, &master_paths, &docs); let file_slide_ids = read_slides(&mut imp, &mut p, &pres_part, &docs, &slide_ids, &layout_by_part); read_sections(&mut p, &pres_part, pres_el, &file_slide_ids); - p.tags = tags(&imp, &pres_part, pres_el); + p.tags = tags(&mut imp, &pres_part, pres_el); read_custom_shows(&mut p, &imp, &pres_part, pres_el); read_guides_and_show(&mut p, &imp, &pres_part); read_notes_master(&mut imp, &mut p, &pres_part); diff --git a/crates/pptx/tests/it/tags.rs b/crates/pptx/tests/it/tags.rs index e8903b44..8cc0849b 100644 --- a/crates/pptx/tests/it/tags.rs +++ b/crates/pptx/tests/it/tags.rs @@ -36,3 +36,111 @@ fn tags_survive_round_trip() { assert_eq!(first.tags, vec![t("THINKCELLSHAPEDONOTDELETE", "t!abc")]); assert!(back.slides.iter().skip(1).all(|s| s.tags.is_empty())); } + +/// #1164: the same tags part listed over and over is read once, and the tags one import copies +/// into the model are capped however often a shape lists them. +#[test] +fn a_tags_part_listed_many_times_is_copied_once_and_capped() { + let mut s = Session::new(); + deckcraft_engine::sample::open_sample(&mut s).expect("sample"); + for _ in 0..6 { + s.execute("shape.insert", &serde_json::json!({"preset": "rect", "rect": [10, 10, 50, 50]})).expect("shape"); + } + let mut p = s.doc().expect("doc").doc.as_ref().clone(); + let slide = Arc::make_mut(&mut p.slides[0]); + // A quarter of the budget with its name and pair cost: the fifth shape listing it goes over. + let pair = std::mem::size_of::<(String, String)>(); + slide.shapes[0].tags = vec![t("BIG", &"x".repeat(deckcraft_pptx::MAX_TAG_BYTES / 4 - 3 - pair))]; + let bytes = deckcraft_pptx::export(&p).expect("export"); + + // Every shape on the first slide lists that one tags part 100 times. + let mut zip = zip::ZipArchive::new(Cursor::new(bytes)).expect("zip"); + let mut xml = String::new(); + zip.by_name("ppt/slides/slide1.xml").expect("slide").read_to_string(&mut xml).expect("read"); + let start = xml.find("").expect("tags listed"); + let end = xml[start..].find("").expect("list ends") + start + "".len(); + let list = &xml[start..end]; + let inner = list.trim_start_matches("").trim_end_matches(""); + let many = format!("{}", inner.repeat(100)); + let xml = xml.replace(list, "").replace("", "").replace("", &format!("{many}")); + assert!(xml.matches(&many).count() >= 5, "enough shapes to go over the budget"); + let mut out = zip::ZipWriter::new(Cursor::new(Vec::new())); + for i in 0..zip.len() { + let mut f = zip.by_index(i).expect("entry"); + let name = f.name().to_string(); + let mut data = vec![]; + f.read_to_end(&mut data).expect("read"); + out.start_file(&name, zip::write::SimpleFileOptions::default()).expect("start"); + std::io::Write::write_all(&mut out, if name == "ppt/slides/slide1.xml" { xml.as_bytes() } else { &data }).expect("write"); + } + let bytes = out.finish().expect("finish").into_inner(); + + let back = deckcraft_pptx::import(&bytes).expect("import"); + let tagged: Vec<_> = back.slides[0].shapes.iter().filter(|x| !x.tags.is_empty()).collect(); + assert_eq!(tagged.len(), 4, "four copies fit the budget"); + assert!(tagged.iter().all(|x| x.tags.len() == 1), "each shape keeps the part's tag once, not 100 times"); +} + +/// #1164: empty tags cost something too, or a part of 10,000 empty pairs listed by every shape +/// would be copied without ever reaching the budget. +#[test] +fn empty_tags_count_against_the_budget() { + let pair = std::mem::size_of::<(String, String)>(); + let per_shape = 10_000 * pair; + let shapes = deckcraft_pptx::MAX_TAG_BYTES / per_shape + 2; + let mut s = Session::new(); + deckcraft_engine::sample::open_sample(&mut s).expect("sample"); + let mut p = s.doc().expect("doc").doc.as_ref().clone(); + let slide = Arc::make_mut(&mut p.slides[0]); + let template = slide.shapes[0].clone(); + slide.shapes.clear(); + for i in 0..shapes { + let mut sh = template.clone(); + sh.id = deckcraft_model::ShapeId(10_000 + i as u32); + sh.ph = None; + sh.tags = vec![t("", ""); 10_000]; + slide.shapes.push(sh); + } + let bytes = deckcraft_pptx::export(&p).expect("export"); + let back = deckcraft_pptx::import(&bytes).expect("import"); + let pairs: usize = back.slides[0].shapes.iter().map(|x| x.tags.len()).sum(); + assert!(pairs * pair <= deckcraft_pptx::MAX_TAG_BYTES, "{pairs} pairs copied"); + assert!(pairs < shapes * 10_000, "the budget stopped the copying"); +} + +/// #1164: once the budget is exhausted, further tags parts are not parsed or cached into +/// `tags_by_part`. +#[test] +fn new_tag_parts_are_not_parsed_or_cached_after_the_budget_is_exhausted() { + let mut s = Session::new(); + deckcraft_engine::sample::open_sample(&mut s).expect("sample"); + s.execute("shape.insert", &serde_json::json!({"preset": "rect", "rect": [10, 10, 50, 50]})).expect("shape"); + let mut p = s.doc().expect("doc").doc.as_ref().clone(); + let slide = Arc::make_mut(&mut p.slides[0]); + assert!(slide.shapes.len() >= 2); + // The first shape's tag fits the budget, and a second pair pushes it over the cap. + let pair = std::mem::size_of::<(String, String)>(); + slide.shapes[0].tags = vec![t("BIG", &"x".repeat(deckcraft_pptx::MAX_TAG_BYTES - 3 - pair)), t("OVER", "1")]; + // The second shape has its own separate tag part. + slide.shapes[1].tags = vec![t("SHAPE2_TAG", "dropped")]; + let bytes = deckcraft_pptx::export(&p).expect("export"); + + // Corrupt the second shape's tag part (`tag2.xml`) with invalid XML in the zip. + // If it were parsed, XML loading would fail. Because the budget was capped, it is never read. + let mut zip = zip::ZipArchive::new(Cursor::new(bytes)).expect("zip"); + let mut out = zip::ZipWriter::new(Cursor::new(Vec::new())); + for i in 0..zip.len() { + let mut f = zip.by_index(i).expect("entry"); + let name = f.name().to_string(); + let mut data = vec![]; + f.read_to_end(&mut data).expect("read"); + out.start_file(&name, zip::write::SimpleFileOptions::default()).expect("start"); + let payload = if name == "ppt/tags/tag2.xml" { b"