diff --git a/crates/model/src/chart.rs b/crates/model/src/chart.rs index 9cebb5a2f..c93d12f25 100644 --- a/crates/model/src/chart.rs +++ b/crates/model/src/chart.rs @@ -392,6 +392,9 @@ pub struct Chart { /// Original chart XML part kept for round-trip of unmodelled options. #[serde(skip_serializing_if = "Option::is_none")] pub raw: Option, + /// The chart was too large to model: `raw` and `kept` are written back unchanged on save. + #[serde(skip_serializing_if = "std::ops::Not::not")] + pub verbatim: bool, /// Package parts the original chart still points at (embedded workbook, styles, drawings). /// The PPTX writer clears this before comparing modelled state. Native saves keep it too. #[serde(skip_serializing_if = "ChartKept::is_empty")] @@ -485,6 +488,7 @@ impl Default for Chart { bin_count: None, bin_size: None, raw: None, + verbatim: false, kept: ChartKept::default(), } } @@ -1311,6 +1315,7 @@ impl Chart { series: vec![], gap_width: 0.0, raw: None, + verbatim: false, subtotals: vec![], ..self.clone() }; diff --git a/crates/pptx/src/lib.rs b/crates/pptx/src/lib.rs index 3c488da2e..657435821 100644 --- a/crates/pptx/src/lib.rs +++ b/crates/pptx/src/lib.rs @@ -70,6 +70,13 @@ pub fn import_checked(bytes: &[u8]) -> Result<(Presentation, Vec), PptxE read::import_checked(bytes) } +/// [`import`] with a lower presentation-wide chart point budget, so tests reach the over-budget +/// path without a multi-million-point chart. +#[doc(hidden)] +pub fn import_with_chart_budget(bytes: &[u8], points: usize) -> Result { + read::import_with_chart_points(bytes, points).map(|(p, _)| p) +} + /// Read the theme of a `.thmx` file, or of a `.pptx` / `.potx`'s first master, with its media. pub fn import_theme(bytes: &[u8]) -> Result<(deckcraft_model::theme::Theme, Vec), PptxError> { read::import_theme(bytes) diff --git a/crates/pptx/src/read.rs b/crates/pptx/src/read.rs index 47f6229c3..63176c959 100644 --- a/crates/pptx/src/read.rs +++ b/crates/pptx/src/read.rs @@ -38,8 +38,17 @@ pub struct Imp<'a> { pub vml_previews: HashMap>, /// Aggregate byte count of retained chart sidecars across the presentation. pub kept_sidecar_bytes: usize, + /// Data slots of the charts decoded so far (see `chart::MAX_PRESENTATION_CHART_POINTS`). + pub chart_points: usize, + /// Points the presentation may hold across modelled charts. + pub chart_points_cap: usize, /// Cached kept sidecars by normalized chart part name. pub kept_charts: HashMap, + /// Chart parts whose sidecar capture was refused, with the retained-byte total it started from: + /// the same start gives the same refusal, so the copy is not repeated. + pub rejected_kept: HashMap, + /// Bytes of chart parts decoded so far, rejected ones included (see `chart::MAX_CHART_DECODE_BYTES`). + pub chart_decode_bytes: usize, /// 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`]). @@ -445,7 +454,11 @@ pub fn import_theme(bytes: &[u8]) -> Result<(deckcraft_model::theme::Theme, Vec< table_style_ids: Vec::new(), vml_previews: HashMap::new(), kept_sidecar_bytes: 0, + chart_points: 0, + chart_points_cap: chart::MAX_PRESENTATION_CHART_POINTS, kept_charts: HashMap::new(), + rejected_kept: HashMap::new(), + chart_decode_bytes: 0, tags_by_part: HashMap::new(), tag_bytes: 0, tags_capped: false, @@ -463,6 +476,11 @@ pub fn import(bytes: &[u8]) -> Result { /// [`import`], plus the parts of the slide list the file names but that couldn't be read (a /// missing or truncated slide): those slides are left out, so a caller can say so (#203). pub fn import_checked(bytes: &[u8]) -> Result<(Presentation, Vec), PptxError> { + import_with_chart_points(bytes, chart::MAX_PRESENTATION_CHART_POINTS) +} + +/// [`import_checked`] with the presentation's chart point budget set (tests lower it). +pub(crate) fn import_with_chart_points(bytes: &[u8], chart_points_cap: usize) -> Result<(Presentation, Vec), PptxError> { let mut lost_slides: Vec = vec![]; let pkg = Package::open(bytes)?; let root_rels = pkg.rels(""); @@ -487,7 +505,11 @@ pub fn import_checked(bytes: &[u8]) -> Result<(Presentation, Vec), PptxE table_style_ids: Vec::new(), vml_previews: HashMap::new(), kept_sidecar_bytes: 0, + chart_points: 0, + chart_points_cap, kept_charts: HashMap::new(), + rejected_kept: HashMap::new(), + chart_decode_bytes: 0, tags_by_part: HashMap::new(), tag_bytes: 0, tags_capped: false, diff --git a/crates/pptx/src/read/chart.rs b/crates/pptx/src/read/chart.rs index 358f88ff5..a175d4845 100644 --- a/crates/pptx/src/read/chart.rs +++ b/crates/pptx/src/read/chart.rs @@ -39,6 +39,33 @@ pub(crate) fn rich_text(t: &El) -> Option { /// Most category/value/size slots one chart may decode across all its series and caches, so /// sparse `ptCount`/`idx` declarations can't expand far beyond the XML they came from. pub(crate) const MAX_CHART_POINTS: usize = 1_000_000; +/// Data slots (values and categories) the charts of one presentation may hold in all; a chart past +/// it is read as a placeholder frame. Each frame that references a chart decodes its own copy. +pub(crate) const MAX_PRESENTATION_CHART_POINTS: usize = 4_000_000; + +/// Chart part bytes all chart frames of a presentation may decode, counting every attempt: a +/// chart that is rejected, or has no data slots, would otherwise be decoded again for each frame +/// that shows it. +pub(crate) const MAX_CHART_DECODE_BYTES: usize = 256 * 1024 * 1024; + +/// Slots a decoded chart holds, counting the chart itself as at least one. +pub(crate) fn point_count(c: &Chart) -> usize { + let series = |s: &deckcraft_model::chart::Series| { + let bars = s.error_bars.as_ref().map_or(0, |e| e.plus.len().saturating_add(e.minus.len())); + s.values.len().saturating_add(s.x_values.len()).saturating_add(s.sizes.len()).saturating_add(bars) + }; + let levels: usize = c.category_levels.iter().map(Vec::len).sum(); + 1usize.saturating_add(c.categories.len()).saturating_add(levels).saturating_add(c.series.iter().map(series).fold(0, usize::saturating_add)) +} + +/// Bytes of the strings a kept relationship owns. +fn part_bytes(p: &deckcraft_model::chart::ChartPart) -> usize { + p.bytes.len() + p.name.len() + p.content_type.len() +} + +fn rel_bytes(r: &deckcraft_model::chart::ChartRel) -> usize { + r.id.len() + r.rel_type.len() + r.target.len() + r.part.len() +} /// Most slots one cache may hold. const MAX_CACHE_POINTS: usize = 100_000; @@ -103,6 +130,48 @@ pub fn read_chart_xml(bytes: &[u8]) -> Option { Some(c) } +/// The chart a graphic frame points at. A chart that would cross the presentation's point or +/// decode limits is not modelled: it is kept as its original part with the relationships it +/// reaches (`verbatim`), so a save writes it back whole instead of dropping it and its workbook. +pub(crate) fn frame_chart(imp: &mut super::Imp, part: &super::Part, rid: &str, uri: &str) -> Option { + let rel = part.rels.get(rid)?; + let chartex = uri.ends_with("/chartex"); + let bytes = imp.pkg.xml_bytes(&rel.target)?; + if imp.chart_decode_bytes.saturating_add(bytes.len()) > MAX_CHART_DECODE_BYTES { + log::warn!("pptx: chart {} is past the presentation's chart decode limit and is not kept", rel.target); + return None; + } + imp.chart_decode_bytes += bytes.len(); + let parsed = if imp.chart_points < imp.chart_points_cap { + let c = if chartex { super::read_chartex_xml(bytes) } else { super::read_chart_xml(bytes) }?; + let n = point_count(&c); + if imp.chart_points.saturating_add(n) <= imp.chart_points_cap { + imp.chart_points = imp.chart_points.saturating_add(n); + let mut c = c; + c.kept = capture_kept(imp, &rel.target); + return Some(c); + } + Some(c) + } else { + None + }; + // Past the limit: keep the part itself (charged to the sidecar budget) and show an empty chart. + if chartex || imp.kept_sidecar_bytes.saturating_add(bytes.len()) > MAX_KEPT_TOTAL_BYTES { + log::warn!("pptx: chart {} is past the presentation's chart limits and is not kept", rel.target); + return None; + } + imp.kept_sidecar_bytes += bytes.len(); + log::warn!("pptx: chart {} is past the presentation's chart limits; shown empty and written back unchanged", rel.target); + let mut c = parsed.unwrap_or_default(); + c.series.clear(); + c.categories.clear(); + c.category_levels.clear(); + c.raw = Some(String::from_utf8_lossy(bytes).into_owned()); + c.verbatim = true; + c.kept = capture_kept(imp, &rel.target); + Some(c) +} + pub(crate) fn chart_from(root: &El) -> Option { chart_within(root, MAX_CHART_POINTS) } @@ -466,9 +535,20 @@ pub(crate) fn capture_kept(imp: &mut super::Imp, chart_part: &str) -> deckcraft_ let key = crate::opc::norm(chart_part); if let Some(cached) = imp.kept_charts.get(&key) { + // Every frame that shows this chart gets its own copy, so each copy is charged like the first. + let bytes: usize = cached.parts.iter().map(part_bytes).sum::() + cached.rels.iter().map(rel_bytes).sum::(); + if imp.kept_sidecar_bytes.saturating_add(bytes) > MAX_KEPT_TOTAL_BYTES { + log::warn!("pptx: chart sidecar retained byte budget exceeded for {chart_part}"); + return ChartKept::default(); + } + imp.kept_sidecar_bytes += bytes; return cached.clone(); } + let charged_before = imp.kept_sidecar_bytes; + if imp.rejected_kept.get(&key) == Some(&charged_before) { + return ChartKept::default(); + } 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([(key.clone(), true)]); @@ -485,6 +565,18 @@ pub(crate) fn capture_kept(imp: &mut super::Imp, chart_part: &str) -> deckcraft_ kept.rels.push(ChartRel { id: r.id, rel_type: r.ty, target: r.target_raw, part: r.target, external: false }); } } + // The first capture's relationship strings and the parts' names and content types are retained + // too (the part bytes were charged as copied). + let rel_total: usize = + kept.rels.iter().map(rel_bytes).sum::() + kept.parts.iter().map(|p| p.name.len() + p.content_type.len()).sum::(); + if imp.kept_sidecar_bytes.saturating_add(rel_total) > MAX_KEPT_TOTAL_BYTES { + log::warn!("pptx: chart sidecar retained byte budget exceeded for {chart_part}"); + // The dropped parts must not stay charged against later charts. + imp.kept_sidecar_bytes = charged_before; + imp.rejected_kept.insert(key, charged_before); + return ChartKept::default(); + } + imp.kept_sidecar_bytes += rel_total; imp.kept_charts.insert(key, kept.clone()); kept } @@ -590,6 +682,16 @@ fn text_size(e: Option<&El>) -> Option { mod tests { use super::*; + #[test] + fn point_count_charges_every_retained_vector() { + let mut c = Chart { categories: vec!["a".into()], category_levels: vec![vec!["x".into(); 2], vec!["y".into(); 3]], ..Default::default() }; + let mut s = + deckcraft_model::chart::Series { values: vec![Some(1.0); 4], x_values: vec![None; 5], sizes: vec![None; 6], ..Default::default() }; + s.error_bars = Some(deckcraft_model::chart::ErrorBars { plus: vec![None; 7], minus: vec![None; 8], ..Default::default() }); + c.series = vec![s]; + assert_eq!(point_count(&c), 1 + 1 + 5 + 4 + 5 + 6 + 15); + } + #[test] fn sparse_caches_share_one_chart_wide_budget() { // Every series declares 100000 points but writes one: without a chart-wide budget each diff --git a/crates/pptx/src/read/shapes.rs b/crates/pptx/src/read/shapes.rs index e1368b427..227b644d1 100644 --- a/crates/pptx/src/read/shapes.rs +++ b/crates/pptx/src/read/shapes.rs @@ -434,11 +434,8 @@ fn frame(imp: &mut Imp, part: &Part, ctx: &mut IdCtx, e: &El, depth: usize) -> S return s; } if let Some(c) = gd.child("chart") - && let Some(rel) = part.rels.get(c.attr("r:id").unwrap_or("")) - && 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) } + && let Some(chart) = super::chart::frame_chart(imp, part, c.attr("r:id").unwrap_or(""), uri) { - chart.kept = super::chart::capture_kept(imp, &rel.target); s.kind = ShapeKind::Chart(Box::new(chart)); return s; } diff --git a/crates/pptx/src/write/chart.rs b/crates/pptx/src/write/chart.rs index 28e62fe98..72ad6beb9 100644 --- a/crates/pptx/src/write/chart.rs +++ b/crates/pptx/src/write/chart.rs @@ -610,6 +610,18 @@ fn kept_chartex(c: &Chart) -> Option> { /// The chart part XML, and whether it is the original part (so its relationships still apply). pub fn chart_xml(c: &Chart) -> (Vec, bool) { + // A chart too large to model is written back as it was read. + if c.verbatim + && let Some(raw) = &c.raw + && let Ok(doc) = crate::xml::parse(raw.as_bytes()) + && doc.root.is("chartSpace") + { + let mut keep = doc.root; + drop_unkept_refs(&mut keep, c); + let mut w = W::new(); + w.raw(&keep.to_xml()); + return (w.finish(), true); + } // Reuse the kept part when the chart is unchanged since it was read. if let Some(raw) = &c.raw && !raw.contains(NS_CX) diff --git a/crates/pptx/tests/it/chart_embed.rs b/crates/pptx/tests/it/chart_embed.rs index 2549cdb64..cd79c94f0 100644 --- a/crates/pptx/tests/it/chart_embed.rs +++ b/crates/pptx/tests/it/chart_embed.rs @@ -328,3 +328,30 @@ fn an_unchanged_chartex_keeps_its_embedded_workbook() { assert!(saved.iter().all(|(n, _)| n != "ppt/charts/_rels/chart1.xml.rels"), "the fallback chart has no workbook"); assert!(!text_of(&saved, "ppt/charts/chart1.xml").contains("externalData")); } + +/// A chart past the presentation's point budget is not modelled, but a save writes the original +/// chart part and its workbook back instead of dropping both. +#[test] +fn a_chart_past_the_point_budget_is_saved_whole() { + let src = with_workbook(&chart_pptx()); + let p = deckcraft_pptx::import_with_chart_budget(&src, 0).expect("import"); + let chart = p + .slides + .iter() + .flat_map(|s| s.shapes.iter()) + .find_map(|s| match &s.kind { + ShapeKind::Chart(c) => Some(c.clone()), + _ => None, + }) + .expect("the chart stays a chart"); + assert!(chart.verbatim && chart.series.is_empty()); + let saved = parts(&deckcraft_pptx::export(&p).expect("export")); + let workbook = saved.iter().find(|(n, _)| n == "ppt/embeddings/kept1.xlsx").map(|(_, d)| d.as_slice()); + assert_eq!(workbook, Some(WORKBOOK)); + let xml = saved + .iter() + .find(|(n, _)| n.starts_with("ppt/charts/chart") && n.ends_with(".xml")) + .map(|(_, d)| String::from_utf8_lossy(d).into_owned()) + .expect("chart xml"); + assert!(xml.contains("externalData") && xml.contains("c:pie"), "{xml}"); +}