address review: bound chart decode work across rejected and repeated references, cache refused sidecar captures

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<String, deckcraft_model::chart::ChartKept>,
+    /// 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<String, usize>,
+    /// 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<String, Arc<[(String, String)]>>,
     /// 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<String>), 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,

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;

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