From 3c84d0e12ca2c4b4473a1f418d27bf8c32df05fd Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 02:10:04 +0000 Subject: [PATCH] address review: bound chart decode work across rejected and repeated references, cache refused sidecar captures --- crates/pptx/src/read.rs | 9 +++++++++ crates/pptx/src/read/chart.rs | 9 +++++++++ crates/pptx/src/read/shapes.rs | 5 +++++ 3 files changed, 23 insertions(+) diff --git a/crates/pptx/src/read.rs b/crates/pptx/src/read.rs index ad8453538..a7ed9df1c 100644 --- a/crates/pptx/src/read.rs +++ b/crates/pptx/src/read.rs @@ -42,6 +42,11 @@ pub struct Imp<'a> { pub chart_points: 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`]). @@ -446,6 +451,8 @@ pub fn import_theme(bytes: &[u8]) -> Result<(deckcraft_model::theme::Theme, Vec< kept_sidecar_bytes: 0, chart_points: 0, kept_charts: HashMap::new(), + rejected_kept: HashMap::new(), + chart_decode_bytes: 0, tags_by_part: HashMap::new(), tag_bytes: 0, tags_capped: false, @@ -488,6 +495,8 @@ pub fn import_checked(bytes: &[u8]) -> Result<(Presentation, Vec), PptxE kept_sidecar_bytes: 0, chart_points: 0, 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 687323621..5c9cd78d7 100644 --- a/crates/pptx/src/read/chart.rs +++ b/crates/pptx/src/read/chart.rs @@ -43,6 +43,11 @@ pub(crate) const MAX_CHART_POINTS: usize = 1_000_000; /// 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| { @@ -499,6 +504,9 @@ pub(crate) fn capture_kept(imp: &mut super::Imp, chart_part: &str) -> deckcraft_ } 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)]); @@ -523,6 +531,7 @@ pub(crate) fn capture_kept(imp: &mut super::Imp, chart_part: &str) -> deckcraft_ 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; diff --git a/crates/pptx/src/read/shapes.rs b/crates/pptx/src/read/shapes.rs index 5193b7070..94bce0843 100644 --- a/crates/pptx/src/read/shapes.rs +++ b/crates/pptx/src/read/shapes.rs @@ -431,6 +431,11 @@ fn frame(imp: &mut Imp, part: &Part, ctx: &mut IdCtx, e: &El, depth: usize) -> S && imp.chart_points < super::chart::MAX_PRESENTATION_CHART_POINTS && let Some(rel) = part.rels.get(c.attr("r:id").unwrap_or("")) && let Some(bytes) = imp.pkg.xml_bytes(&rel.target) + && imp.chart_decode_bytes.saturating_add(bytes.len()) <= super::chart::MAX_CHART_DECODE_BYTES + && { + imp.chart_decode_bytes += bytes.len(); + true + } && let Some(mut chart) = if uri.ends_with("/chartex") { super::read_chartex_xml(bytes) } else { super::read_chart_xml(bytes) } // A chart that would cross the presentation limit is read as the placeholder frame instead. && imp.chart_points.saturating_add(super::chart::point_count(&chart)) <= super::chart::MAX_PRESENTATION_CHART_POINTS