From 7be391a1be0fdc09f9601e41bb55ac5fbac3df34 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sat, 24 Oct 2026 06:01:30 -0400 Subject: [PATCH] fix(pptx): harden import resource budgets, deduplication, and lookup caching (#1283) * fix(pptx): harden import resource budgets, deduplication, and lookup caching - Chart sidecars (Finding 2): Enforce an import-wide retained byte budget and cache captured sidecars by chart part to prevent memory amplification. - Content types cache (Finding 6): Cache parsed [Content_Types].xml defaults and overrides in Package.ct_cache. - Text run splitting and font lengths (Finding 8): Bound typeface attribute lengths and cap paragraph runs to prevent multiplication on text splitting. - Inner namespace decls (Finding 15): Use HashSet/HashMap indexing and cap inner declarations to 1,000. - Slide and notes deduplication (Finding 16): Deduplicate slide paths and cache parsed notes slide bodies by target. - Diagram media caching (Finding 22): Cache diagram media payloads in imp.media_by_part. - Master slide deduplication (Finding 28): Deduplicate master paths before loading. - Layout target deduplication (Finding 30): Deduplicate layout targets across master slides. - Chart value axis indexing (Finding 32): Index valAx by axId using a HashMap. - ChartEx series output budget (Finding 34): Charge series point vectors against an aggregate output budget and cap series count. - ChartEx point allocation bounds (Finding 36): Allocate point vectors only to actual point span bounded by budget rather than declared ptCount. - SmartArt recursion depth (Finding 38): Limit SmartArt recursion depth to 8. * fix(pptx): address review feedback on sidecar cache, series budget, and inner decls * fix(pptx): preserve cached sidecar bytes and non-empty category fallback --- crates/pptx/src/opc.rs | 32 +++++++++----- crates/pptx/src/read/chart.rs | 47 ++++++++++++++------ crates/pptx/src/read/chartex.rs | 76 ++++++++++++++++++++++++--------- crates/pptx/src/read/dml.rs | 53 +++++++++++++++-------- crates/pptx/src/read/mod.rs | 60 +++++++++++++++++++------- crates/pptx/src/read/shapes.rs | 10 ++++- crates/pptx/src/write/chart.rs | 5 ++- crates/pptx/src/write/shapes.rs | 6 ++- 8 files changed, 207 insertions(+), 82 deletions(-) diff --git a/crates/pptx/src/opc.rs b/crates/pptx/src/opc.rs index f39b88bf..8e9e8357 100644 --- a/crates/pptx/src/opc.rs +++ b/crates/pptx/src/opc.rs @@ -39,6 +39,7 @@ pub const RT_CORE: &str = "http://schemas.openxmlformats.org/package/2006/relati /// The files of a package, keyed by normalised part name (no leading slash, lowercase). pub struct Package { parts: HashMap)>, + ct_cache: std::sync::OnceLock<(HashMap, HashMap)>, } impl Package { @@ -94,7 +95,7 @@ impl Package { total = total.saturating_add(data.len() as u64); parts.insert(name.to_ascii_lowercase(), (name, data)); } - Ok(Package { parts }) + Ok(Package { parts, ct_cache: std::sync::OnceLock::new() }) } pub fn get(&self, name: &str) -> Option<&[u8]> { self.parts.get(&norm(name)).map(|(_, d)| d.as_slice()) @@ -131,18 +132,29 @@ impl Package { } /// Content types: (extension defaults, part overrides). pub fn content_type(&self, part: &str) -> Option { - let doc = self.xml("[Content_Types].xml")?; - let p = format!("/{}", norm(part)); - for o in doc.root.children_named("Override") { - if o.attr("PartName").is_some_and(|n| n.to_ascii_lowercase() == p) { - return o.attr("ContentType").map(String::from); + let (overrides, defaults) = self.ct_cache.get_or_init(|| { + let mut ov = HashMap::new(); + let mut def = HashMap::new(); + if let Some(doc) = self.xml("[Content_Types].xml") { + for o in doc.root.children_named("Override") { + if let (Some(n), Some(ct)) = (o.attr("PartName"), o.attr("ContentType")) { + ov.insert(n.to_ascii_lowercase(), ct.to_string()); + } + } + for d in doc.root.children_named("Default") { + if let (Some(e), Some(ct)) = (d.attr("Extension"), d.attr("ContentType")) { + def.insert(e.to_ascii_lowercase(), ct.to_string()); + } + } } + (ov, def) + }); + let p = format!("/{}", norm(part)); + if let Some(ct) = overrides.get(&p) { + return Some(ct.clone()); } let ext = part.rsplit('.').next().unwrap_or("").to_ascii_lowercase(); - doc.root - .children_named("Default") - .find(|d| d.attr("Extension").is_some_and(|e| e.eq_ignore_ascii_case(&ext))) - .and_then(|d| d.attr("ContentType").map(String::from)) + defaults.get(&ext).cloned() } pub fn original_name(&self, name: &str) -> Option<&str> { self.parts.get(&norm(name)).map(|(n, _)| n.as_str()) diff --git a/crates/pptx/src/read/chart.rs b/crates/pptx/src/read/chart.rs index 73cc416c..6c8417cd 100644 --- a/crates/pptx/src/read/chart.rs +++ b/crates/pptx/src/read/chart.rs @@ -126,23 +126,21 @@ fn chart_within(root: &El, mut budget: usize) -> Option { let mut kinds: Vec = vec![]; // The value axis the first plot group uses is the primary one; a group on another value axis // (a combo chart's line on the right) is on the secondary axis. - let val_ids: Vec<(&str, bool)> = + let val_map: std::collections::HashMap<&str, bool> = pa.children_named("valAx").filter_map(|a| Some((a.child_val("axId")?, matches!(a.child_val("axPos"), Some("l" | "r"))))).collect(); // A group's value axis: its vertical one when it has two (scatter), else its one. let group_val_axis = |g: &El| { let ids: Vec<(&str, bool)> = - g.children_named("axId").filter_map(|a| a.attr("val")).filter_map(|id| val_ids.iter().find(|(v, _)| *v == id).copied()).collect(); + g.children_named("axId").filter_map(|a| a.attr("val")).filter_map(|id| val_map.get(id).copied().map(|v| (id, v))).collect(); ids.iter().find(|(_, vertical)| *vertical).or(ids.first()).map(|(id, _)| id.to_string()) }; // The primary value axis is the one on the left (bottom for bar charts); the secondary one // sits on the right or top, or crosses the category axis at its maximum. let secondary_placed = |a: &El| a.child_val("crosses") == Some("max") || matches!(a.child_val("axPos"), Some("r" | "t")); + let val_axes_by_id: std::collections::HashMap<&str, &El> = pa.children_named("valAx").filter_map(|a| Some((a.child_val("axId")?, a))).collect(); let used: Vec = pa.elements().filter(|g| g.local().ends_with("Chart")).filter_map(group_val_axis).collect(); - let primary_val: Option = used - .iter() - .find(|id| pa.children_named("valAx").find(|a| a.child_val("axId") == Some(id.as_str())).is_some_and(|a| !secondary_placed(a))) - .or(used.first()) - .cloned(); + let primary_val: Option = + used.iter().find(|id| val_axes_by_id.get(id.as_str()).is_some_and(|a| !secondary_placed(a))).or(used.first()).cloned(); for g in pa.elements() { let name = g.local(); if !name.ends_with("Chart") { @@ -410,15 +408,21 @@ fn text_color(e: Option<&El>) -> Option { const MAX_KEPT_PARTS: usize = 256; const MAX_KEPT_DEPTH: usize = 8; +pub const MAX_KEPT_TOTAL_BYTES: usize = 32 * 1024 * 1024; /// Relationships and parts a chart points at, so a later save can write them back. -pub(crate) fn capture_kept(pkg: &crate::opc::Package, chart_part: &str) -> deckcraft_model::chart::ChartKept { +pub(crate) fn capture_kept(imp: &mut super::Imp, chart_part: &str) -> deckcraft_model::chart::ChartKept { use deckcraft_model::chart::{ChartKept, ChartRel}; + let key = crate::opc::norm(chart_part); + if let Some(cached) = imp.kept_charts.get(&key) { + return cached.clone(); + } + let mut kept = ChartKept::default(); // The chart document itself is rewritten; don't store a second copy of it. - let mut seen = std::collections::HashMap::from([(crate::opc::norm(chart_part), true)]); - for r in pkg.rels(chart_part).items { + let mut seen = std::collections::HashMap::from([(key.clone(), true)]); + for r in imp.pkg.rels(chart_part).items { if !safe_rel(&r.id, &r.ty, &r.target_raw) { log::warn!("pptx: skipping chart relationship {}", r.id); continue; @@ -427,10 +431,11 @@ pub(crate) fn capture_kept(pkg: &crate::opc::Package, chart_part: &str) -> deckc kept.rels.push(ChartRel { id: r.id, rel_type: r.ty, target: r.target_raw, part: String::new(), external: true }); continue; } - if copy_part(pkg, &r.target, &mut kept.parts, &mut seen, 0) { + if copy_part(imp.pkg, &r.target, &mut kept.parts, &mut seen, &mut imp.kept_sidecar_bytes, 0) { kept.rels.push(ChartRel { id: r.id, rel_type: r.ty, target: r.target_raw, part: r.target, external: false }); } } + imp.kept_charts.insert(key, kept.clone()); kept } @@ -444,6 +449,7 @@ fn copy_part( part: &str, out: &mut Vec, seen: &mut std::collections::HashMap, + retained_bytes: &mut usize, depth: usize, ) -> bool { use deckcraft_model::chart::ChartPart; @@ -473,7 +479,14 @@ fn copy_part( return false; } let content_type = pkg.content_type(part).unwrap_or_default(); - out.push(ChartPart { name, content_type, bytes: bytes.to_vec(), native_entry: None }); + let stored_bytes = if *retained_bytes + bytes.len() <= MAX_KEPT_TOTAL_BYTES { + *retained_bytes += bytes.len(); + bytes.to_vec() + } else { + log::warn!("pptx: chart sidecar retained byte budget exceeded for {part}"); + Vec::new() + }; + out.push(ChartPart { name, content_type, bytes: stored_bytes, native_entry: None }); seen.insert(key, true); let rels_name = crate::opc::rels_path(part); if crate::opc::sidecar_part(&rels_name) @@ -482,15 +495,21 @@ fn copy_part( { seen.insert(crate::opc::norm(&rels_name), true); let name = pkg.original_name(&rels_name).unwrap_or(&rels_name).trim_start_matches('/').to_string(); + let stored_rels = if *retained_bytes + rels_bytes.len() <= MAX_KEPT_TOTAL_BYTES { + *retained_bytes += rels_bytes.len(); + rels_bytes.to_vec() + } else { + Vec::new() + }; out.push(ChartPart { name, content_type: "application/vnd.openxmlformats-package.relationships+xml".into(), - bytes: rels_bytes.to_vec(), + bytes: stored_rels, native_entry: None, }); let targets: Vec = pkg.rels(part).items.into_iter().filter(|r| !r.external).map(|r| r.target).collect(); for target in targets { - copy_part(pkg, &target, out, seen, depth + 1); + copy_part(pkg, &target, out, seen, retained_bytes, depth + 1); } } true diff --git a/crates/pptx/src/read/chartex.rs b/crates/pptx/src/read/chartex.rs index d8f3e9ce..407d9863 100644 --- a/crates/pptx/src/read/chartex.rs +++ b/crates/pptx/src/read/chartex.rs @@ -9,6 +9,7 @@ use std::collections::HashMap; /// Points per level and levels per dimension we keep (guards against hostile parts). const MAX_POINTS: usize = 100_000; +const MAX_CHARTEX_POINTS: usize = 1_000_000; /// One `cx:data`: its categories (the innermost level) and named numeric dimensions. #[derive(Default)] @@ -20,38 +21,52 @@ struct Data { } /// A level's points, by `idx`. -fn points(lvl: &El) -> Vec { - let n = lvl.u32("ptCount").unwrap_or(0).min(MAX_POINTS as u32) as usize; - let mut out = vec![String::new(); n]; - for pt in lvl.children_named("pt").take(MAX_POINTS) { +fn points(lvl: &El, budget: &mut usize) -> Vec { + if *budget == 0 { + return Vec::new(); + } + let cap = MAX_POINTS.min(*budget); + let mut pts = Vec::new(); + let mut max_idx = 0; + for pt in lvl.children_named("pt").take(cap) { let i = pt.u32("idx").unwrap_or(0) as usize; - if i >= MAX_POINTS { + if i >= cap { continue; } - if i >= out.len() { - out.resize(i + 1, String::new()); - } - if let Some(s) = out.get_mut(i) { - *s = pt.text(); + max_idx = max_idx.max(i + 1); + pts.push((i, pt.text())); + } + if pts.is_empty() { + return Vec::new(); + } + let n = max_idx.min(cap); + let mut out = vec![String::new(); n]; + for (i, s) in pts { + if let Some(target) = out.get_mut(i) { + *target = s; } } + *budget = budget.saturating_sub(out.len()); out } -fn data(d: &El) -> Data { +fn data(d: &El, budget: &mut usize) -> Data { let mut out = Data::default(); - for dim in d.elements() { + for dim in d.elements().take(64) { + if *budget == 0 { + break; + } // The first level is the innermost: a treemap's leaves, a waterfall's categories. let Some(lvl) = dim.child("lvl") else { continue }; let ty = dim.attr("type").unwrap_or(""); if dim.is("strDim") || (dim.is("numDim") && ty == "cat") { if ty == "cat" || out.categories.is_empty() { - out.categories = points(lvl); + out.categories = points(lvl, budget); // Further levels are the hierarchy's parents, innermost first. - out.levels = dim.children_named("lvl").skip(1).take(deckcraft_model::chart::MAX_LEVELS).map(points).collect(); + out.levels = dim.children_named("lvl").skip(1).take(deckcraft_model::chart::MAX_LEVELS).map(|l| points(l, budget)).collect(); } } else if dim.is("numDim") { - let vals = points(lvl).into_iter().map(|s| s.trim().parse::().ok().filter(|v| v.is_finite())).collect(); + let vals = points(lvl, budget).into_iter().map(|s| s.trim().parse::().ok().filter(|v| v.is_finite())).collect(); out.numbers.insert(ty.to_string(), (vals, lvl.attr("formatCode").map(str::to_string))); } } @@ -85,15 +100,22 @@ pub(crate) fn chartex_from(root: &El) -> Option { if !root.is("chartSpace") { return None; } - let datas: HashMap = - root.child("chartData")?.children_named("data").map(|d| (d.attr("id").unwrap_or("0").to_string(), data(d))).collect(); + let mut decode_budget = MAX_CHARTEX_POINTS; + let datas: HashMap = root + .child("chartData")? + .children_named("data") + .take(64) + .map(|d| (d.attr("id").unwrap_or("0").to_string(), data(d, &mut decode_budget))) + .collect(); let ch = root.child("chart")?; let region = ch.child("plotArea")?.child("plotAreaRegion")?; let mut c = Chart { categories: vec![], series: vec![], title: None, legend: None, gridlines: false, ..Default::default() }; c.title = ch.child("title").map(|t| rich_text(t).filter(|s| !s.is_empty()).unwrap_or_else(|| "Chart Title".into())); c.legend = ch.child("legend").map(|l| l.attr("pos").unwrap_or("t").to_string()); let mut kind = None; - for s in region.children_named("series") { + let mut output_budget = MAX_CHARTEX_POINTS; + let mut categories_set = false; + for s in region.children_named("series").take(1000) { let layout = s.attr("layoutId").unwrap_or(""); if layout == "paretoLine" { // The cumulative line PowerPoint adds to a Pareto histogram. @@ -109,11 +131,23 @@ pub(crate) fn chartex_from(root: &El) -> Option { _ => k, }); let d = datas.get(s.child("dataId").and_then(|i| i.attr("val")).unwrap_or("0"))?; - let (values, format) = - ["val", "size", "colorVal"].iter().find_map(|t| d.numbers.get(*t)).or_else(|| d.numbers.values().next()).cloned().unwrap_or_default(); - if c.categories.is_empty() { + let (values_src, format) = ["val", "size", "colorVal"] + .iter() + .find_map(|t| d.numbers.get(*t)) + .or_else(|| d.numbers.values().next()) + .map(|(v, f)| (v.as_slice(), f.clone())) + .unwrap_or((&[], None)); + let take_len = values_src.len().min(output_budget); + let values = values_src[..take_len].to_vec(); + output_budget = output_budget.saturating_sub(values.len()); + if !categories_set && (!d.categories.is_empty() || !d.levels.is_empty()) { + categories_set = true; c.categories = d.categories.clone(); c.category_levels = d.levels.clone(); + output_budget = output_budget.saturating_sub(c.categories.len()); + for lvl in &c.category_levels { + output_budget = output_budget.saturating_sub(lvl.len()); + } } if let Some(st) = s.child("layoutPr").and_then(|l| l.child("statistics")) { c.inclusive_quartiles = st.attr("quartileMethod") == Some("inclusive"); diff --git a/crates/pptx/src/read/dml.rs b/crates/pptx/src/read/dml.rs index c88bd1a9..ab19ede4 100644 --- a/crates/pptx/src/read/dml.rs +++ b/crates/pptx/src/read/dml.rs @@ -493,6 +493,11 @@ pub fn hyperlink(imp: &mut Imp, part: &Part, h: &El) -> Option { }) } +fn typeface(e: &El) -> Option { + let t = e.attr("typeface").filter(|t| !t.is_empty())?; + Some(t.chars().take(256).collect()) +} + pub fn rpr(imp: &mut Imp, part: &Part, r: &El) -> RunProps { let mut p = RunProps { lang: r.attr("lang").map(String::from), @@ -530,10 +535,10 @@ pub fn rpr(imp: &mut Imp, part: &Part, r: &El) -> RunProps { } "highlight" => p.highlight = color(e), "uFill" => p.underline_color = e.child("solidFill").and_then(color), - "latin" => p.font = e.attr("typeface").filter(|t| !t.is_empty()).map(String::from), - "ea" => p.font_ea = e.attr("typeface").filter(|t| !t.is_empty()).map(String::from), - "cs" => p.font_cs = e.attr("typeface").filter(|t| !t.is_empty()).map(String::from), - "sym" => p.font_sym = e.attr("typeface").filter(|t| !t.is_empty()).map(String::from), + "latin" => p.font = typeface(e), + "ea" => p.font_ea = typeface(e), + "cs" => p.font_cs = typeface(e), + "sym" => p.font_sym = typeface(e), "hlinkClick" => p.link = hyperlink(imp, part, e), "hlinkMouseOver" => p.hover = hyperlink(imp, part, e), _ => {} @@ -564,6 +569,8 @@ pub fn text_body(imp: &mut Imp, part: &Part, tx: &El) -> TextBody { TextBody { body, list_style, paragraphs } } +const MAX_PARAGRAPH_RUNS: usize = 10_000; + pub fn paragraph(imp: &mut Imp, part: &Part, p: &El) -> Paragraph { let mut para = Paragraph::default(); for e in p.elements() { @@ -581,33 +588,45 @@ pub fn paragraph(imp: &mut Imp, part: &Part, p: &El) -> Paragraph { let text: String = e.child("t").map(|t| t.text()).unwrap_or_default(); // A vertical tab or line feed inside text is a line break, as it lays out and as export writes it. for (i, piece) in text.split(['\u{b}', '\n']).enumerate() { + if para.runs.len() >= MAX_PARAGRAPH_RUNS { + break; + } if i > 0 { para.runs.push(Run { text: String::new(), props: props.clone(), kind: RunKind::Break }); } if !piece.is_empty() { + if para.runs.len() >= MAX_PARAGRAPH_RUNS { + break; + } para.runs.push(Run { text: piece.to_string(), props: props.clone(), kind: RunKind::Text }); } } } "br" => { - let props = e.child("rPr").map(|r| rpr(imp, part, r)).unwrap_or_default(); - para.runs.push(Run { text: String::new(), props, kind: RunKind::Break }); + if para.runs.len() < MAX_PARAGRAPH_RUNS { + let props = e.child("rPr").map(|r| rpr(imp, part, r)).unwrap_or_default(); + para.runs.push(Run { text: String::new(), props, kind: RunKind::Break }); + } } "fld" => { - let props = e.child("rPr").map(|r| rpr(imp, part, r)).unwrap_or_default(); - let text = e.child("t").map(|t| t.text()).unwrap_or_default(); - para.runs.push(Run { text, props, kind: RunKind::Field { field: e.attr("type").unwrap_or("").to_string() } }); + if para.runs.len() < MAX_PARAGRAPH_RUNS { + let props = e.child("rPr").map(|r| rpr(imp, part, r)).unwrap_or_default(); + let text = e.child("t").map(|t| t.text()).unwrap_or_default(); + para.runs.push(Run { text, props, kind: RunKind::Field { field: e.attr("type").unwrap_or("").to_string() } }); + } } "m" => { - // Office Math (a14:m) — keep the OMML and its plain text. - let mut text = String::new(); - let mut ts = vec![]; - e.find_all("t", &mut ts); - for t in ts { - text.push_str(&t.text()); + if para.runs.len() < MAX_PARAGRAPH_RUNS { + // Office Math (a14:m) — keep the OMML and its plain text. + let mut text = String::new(); + let mut ts = vec![]; + e.find_all("t", &mut ts); + for t in ts { + text.push_str(&t.text()); + } + let omml = e.elements().map(|c| part.keep_inner(c)).collect::(); + para.runs.push(Run { text, props: RunProps::default(), kind: RunKind::Math { omml } }); } - let omml = e.elements().map(|c| part.keep_inner(c)).collect::(); - para.runs.push(Run { text, props: RunProps::default(), kind: RunKind::Math { omml } }); } "endParaRPr" => para.end_props = rpr(imp, part, e), _ => {} diff --git a/crates/pptx/src/read/mod.rs b/crates/pptx/src/read/mod.rs index 644ffa8f..bb717d45 100644 --- a/crates/pptx/src/read/mod.rs +++ b/crates/pptx/src/read/mod.rs @@ -36,6 +36,10 @@ pub struct Imp<'a> { pub table_style_ids: Vec, /// Per legacy VML drawing part: shape id → preview picture part (see `shapes::vml_preview`). pub vml_previews: HashMap>, + /// Aggregate byte count of retained chart sidecars across the presentation. + pub kept_sidecar_bytes: usize, + /// Cached kept sidecars by normalized chart part name. + pub kept_charts: HashMap, } impl Imp<'_> { @@ -97,29 +101,32 @@ impl Part { /// Prefixes declared below the root but not on it, skipping any bound to different namespaces. fn inner_decls(root: &El, ns: &[(String, String)]) -> Vec<(String, String)> { - fn rec(e: &El, out: &mut Vec<(String, String)>, clash: &mut HashSet, depth: usize) { + fn rec(e: &El, out: &mut Vec<(String, String)>, out_map: &mut HashMap, clash: &mut HashSet, depth: usize) { if depth > crate::xml::MAX_DEPTH { return; } for (k, v) in e.attrs.iter().filter(|(k, _)| k.starts_with("xmlns:")) { - match out.iter().find(|(o, _)| o == k).map(|(_, u)| u != v) { - Some(true) => { + if let Some(existing) = out_map.get(k) { + if existing != v { clash.insert(k.clone()); } - Some(false) => {} - None => out.push((k.clone(), v.clone())), + } else if out.len() < 1000 { + out_map.insert(k.clone(), v.clone()); + out.push((k.clone(), v.clone())); } } for c in e.elements() { - rec(c, out, clash, depth + 1); + rec(c, out, out_map, clash, depth + 1); } } let mut out = vec![]; + let mut out_map = HashMap::new(); let mut clash = HashSet::new(); + let root_ns: HashSet<&str> = ns.iter().map(|(r, _)| r.as_str()).collect(); for c in root.elements() { - rec(c, &mut out, &mut clash, 1); + rec(c, &mut out, &mut out_map, &mut clash, 1); } - out.retain(|(k, _)| !clash.contains(k) && !ns.iter().any(|(r, _)| r == k)); + out.retain(|(k, _)| !clash.contains(k) && !root_ns.contains(k.as_str())); out } @@ -380,6 +387,8 @@ pub fn import_theme(bytes: &[u8]) -> Result<(deckcraft_model::theme::Theme, Vec< custom_show_names: HashMap::new(), table_style_ids: Vec::new(), vml_previews: HashMap::new(), + kept_sidecar_bytes: 0, + kept_charts: HashMap::new(), }; let part = Part::new(&pkg, &theme_path, &doc); let theme = theme::read_theme(&mut imp, &part, &doc.root); @@ -409,9 +418,9 @@ pub fn import_checked(bytes: &[u8]) -> Result<(Presentation, Vec), PptxE .child("sldMasterIdLst") .map(|l| l.children_named("sldMasterId").filter_map(|m| pres_part.rels.target(m.attr("r:id").unwrap_or(""))).map(String::from).collect()) .unwrap_or_default(); - let master_paths: Vec = + let mut master_paths: Vec = if master_paths.is_empty() { pres_part.rels.all_of("slideMaster").map(|r| r.target.clone()).collect() } else { master_paths }; - let slide_entries: Vec<(u32, String)> = pres_el + let mut slide_entries: Vec<(u32, String)> = pres_el .child("sldIdLst") .map(|l| { l.children_named("sldId") @@ -423,10 +432,21 @@ pub fn import_checked(bytes: &[u8]) -> Result<(Presentation, Vec), PptxE let mut docs: HashMap = HashMap::new(); let mut max_id = 0u32; + let mut seen_masters = HashSet::new(); + master_paths.retain(|m| seen_masters.insert(opc::norm(m))); + let mut all_parts: Vec = master_paths.clone(); + let mut seen_layouts = HashSet::new(); for m in &master_paths { - all_parts.extend(pkg.rels(m).all_of("slideLayout").map(|r| r.target.clone())); + for r in pkg.rels(m).all_of("slideLayout") { + let key = opc::norm(&r.target); + if seen_layouts.insert(key) { + all_parts.push(r.target.clone()); + } + } } + let mut seen_slides = HashSet::new(); + slide_entries.retain(|(_, p)| seen_slides.insert(opc::norm(p))); all_parts.extend(slide_entries.iter().map(|(_, p)| p.clone())); for p in &all_parts { let key = opc::norm(p); @@ -448,6 +468,8 @@ pub fn import_checked(bytes: &[u8]) -> Result<(Presentation, Vec), PptxE custom_show_names: HashMap::new(), table_style_ids: Vec::new(), vml_previews: HashMap::new(), + kept_sidecar_bytes: 0, + kept_charts: HashMap::new(), }; // Slide ids first so hyperlinks anywhere can point at slides. let mut slide_ids: Vec<(u32, String, SlideId)> = vec![]; @@ -516,17 +538,23 @@ pub fn import_checked(bytes: &[u8]) -> Result<(Presentation, Vec), PptxE // Slides. let mut file_slide_ids: HashMap = HashMap::new(); + let mut notes_by_target: HashMap = HashMap::new(); for (file_id, path, id) in &slide_ids { let Some(doc) = docs.get(&opc::norm(path)) else { continue }; let part = Part::new(&pkg, path, doc); let layout = part.rels.first_of("slideLayout").and_then(|r| layout_by_part.get(&opc::norm(&r.target)).copied()).unwrap_or(fallback_layout); let mut slide = read_slide(&mut imp, &part, doc, *id, layout); // Notes. - if let Some(n) = part.rels.first_of("notesSlide") - && let Some(nd) = load(&pkg, &n.target) - { - let np = Part::new(&pkg, &n.target, &nd); - slide.notes = read_notes(&mut imp, &np, &nd.root); + if let Some(n) = part.rels.first_of("notesSlide") { + let n_key = opc::norm(&n.target); + if let Some(cached_notes) = notes_by_target.get(&n_key) { + slide.notes = cached_notes.clone(); + } else if let Some(nd) = load(&pkg, &n.target) { + let np = Part::new(&pkg, &n.target, &nd); + let notes = read_notes(&mut imp, &np, &nd.root); + notes_by_target.insert(n_key, notes.clone()); + slide.notes = notes; + } } // Comments. A modern part and the legacy part can describe the same threads (PowerPoint // writes both): the modern one wins, its flattened legacy copy is skipped. diff --git a/crates/pptx/src/read/shapes.rs b/crates/pptx/src/read/shapes.rs index 50e6e250..0d6c8f8c 100644 --- a/crates/pptx/src/read/shapes.rs +++ b/crates/pptx/src/read/shapes.rs @@ -430,7 +430,7 @@ fn frame(imp: &mut Imp, part: &Part, ctx: &mut IdCtx, e: &El, depth: usize) -> S && let Some(bytes) = imp.pkg.xml_bytes(&rel.target) && let Some(mut chart) = if uri.ends_with("/chartex") { super::read_chartex_xml(bytes) } else { super::read_chart_xml(bytes) } { - chart.kept = super::chart::capture_kept(imp.pkg, &rel.target); + chart.kept = super::chart::capture_kept(imp, &rel.target); s.kind = ShapeKind::Chart(Box::new(chart)); return s; } @@ -516,6 +516,10 @@ fn vml_index(pkg: &crate::opc::Package, vml: &str) -> std::collections::HashMap< /// relationship id, which a save renumbers. Only diagrams whose drawing is missing or empty get /// here, and those are drawn from their data. fn diagram_data(imp: &mut Imp, target: &str) -> Option { + let norm_target = crate::opc::norm(target); + if let Some(&id) = imp.media_by_part.get(&norm_target) { + return Some(id); + } fn strip(e: &mut El, depth: usize) { if depth > crate::xml::MAX_DEPTH { return; @@ -537,11 +541,15 @@ fn diagram_data(imp: &mut Imp, target: &str) -> Option data: std::sync::Arc::new(xml.into_bytes()), link: None, }); + imp.media_by_part.insert(norm_target, id); Some(id) } /// SmartArt: read the pre-drawn shapes of its drawing part as a group. fn smartart(imp: &mut Imp, part: &Part, gd: &El, frame: &Shape, depth: usize) -> Option { + if depth > 8 { + return None; + } let rel_ids = gd.child("relIds")?; let dm = part.rels.get(rel_ids.attr("r:dm").unwrap_or(""))?; let data = imp.pkg.xml(&dm.target)?; diff --git a/crates/pptx/src/write/chart.rs b/crates/pptx/src/write/chart.rs index 5dcbe33b..1ff896bb 100644 --- a/crates/pptx/src/write/chart.rs +++ b/crates/pptx/src/write/chart.rs @@ -750,7 +750,10 @@ pub fn write_kept(pkg: &mut crate::opc::PackageOut, chart_name: &str, c: &Chart, } } pkg.rels(chart_name, &rels); - let parts_by_norm: std::collections::HashMap<_, _> = c.kept.parts.iter().map(|p| (norm(&p.name), p)).collect(); + let mut parts_by_norm = std::collections::HashMap::new(); + for p in &c.kept.parts { + parts_by_norm.entry(norm(&p.name)).or_insert(p); + } let mut seen_payloads = std::collections::HashSet::new(); // The kept parts' own relationships, pointed at the new names. for p in c.kept.parts.iter().filter(|p| kept_payload(&p.name)) { diff --git a/crates/pptx/src/write/shapes.rs b/crates/pptx/src/write/shapes.rs index 38c71235..5ef269f3 100644 --- a/crates/pptx/src/write/shapes.rs +++ b/crates/pptx/src/write/shapes.rs @@ -853,8 +853,10 @@ fn opaque( if let Ok(doc) = crate::xml::parse(xml.as_bytes()) && matches!(doc.root.local(), "graphicFrame" | "sp" | "grpSp" | "cxnSp") { - let part_map: std::collections::HashMap<&str, (&str, deckcraft_model::MediaId)> = - parts.iter().map(|(id, kind, m)| (id.as_str(), (kind.as_str(), *m))).collect(); + let mut part_map: std::collections::HashMap<&str, (&str, deckcraft_model::MediaId)> = std::collections::HashMap::new(); + for (id, kind, m) in parts { + part_map.entry(id.as_str()).or_insert((kind.as_str(), *m)); + } if let Some(ids) = crate::read::rel_refs(&doc.root) .iter() .map(|r| {