address review: keep the shared-formula text cut-off across worksheets

Shared-formula followers each kept their own printed formula text, but
none of it counted against the 256 MB text allowance, so compact PtgExp
records could expand into gigabytes of retained formulas. Each attach
now charges its text; past the allowance the formula is dropped and the
cell keeps its cached value.

crates/xlsx/src/passthrough/write.rs

@@ -254,36 +254,27 @@ pub(crate) fn sheet_names_in(formula: &str) -> Vec<String> {
 /// Whether a kept chart sheet can be written back: its chart's series must still refer to sheets
 /// that exist under the same names (the chart is kept as it was, so a series on a deleted or
 /// renamed sheet would be left pointing at nothing), and no worksheet may have taken its name.
-///
-/// `usable` caches, by relationship id, whether that chart sheet's graph refers only to sheets
-/// that exist (`false` when the relationship is missing): entries sharing one graph are analysed
-/// once, and only the verdict is kept, not the references it came from.
-fn chart_sheet_usable<'p>(wb: &Workbook, p: &'p Preserved, s: &'p PreservedSheet, usable: &mut HashMap<&'p str, bool>) -> bool {
+fn chart_sheet_usable(wb: &Workbook, p: &Preserved, s: &PreservedSheet) -> bool {
     let same = |a: &str, b: &str| a.chars().take(31).collect::<String>().eq_ignore_ascii_case(&b.chars().take(31).collect::<String>());
     if wb.sheets.iter().any(|w| same(&w.name, &s.name)) {
         return false;
     }
-    *usable.entry(s.rel_id.as_str()).or_insert_with(|| {
-        let Some(root) = p.rels.iter().find(|r| r.id == s.rel_id) else { return false };
-        let parts = reachable(p, root);
-        let mut formulas = vec![];
-        for q in p.parts.iter().filter(|q| parts.contains(&normalize(&q.name)) && q.content_type.contains("chart")) {
-            if let Ok(x) = xml::parse(&q.data) {
-                chart_formulas(&x, &mut formulas);
-            }
+    let Some(root) = p.rels.iter().find(|r| r.id == s.rel_id) else { return false };
+    let parts = reachable(p, root);
+    let mut formulas = vec![];
+    for q in p.parts.iter().filter(|q| parts.contains(&normalize(&q.name)) && q.content_type.contains("chart")) {
+        if let Ok(x) = xml::parse(&q.data) {
+            chart_formulas(&x, &mut formulas);
         }
-        formulas.iter().flat_map(|f| sheet_names_in(f)).all(|n| wb.sheets.iter().any(|w| same(&w.name, &n)))
-    })
+    }
+    formulas.iter().flat_map(|f| sheet_names_in(f)).all(|n| wb.sheets.iter().any(|w| same(&w.name, &n)))
 }
 
 /// Writes the workbook's kept parts and relationships. Chart sheets that can't be written back
 /// (see [`chart_sheet_usable`]) are left out with the parts only they reach.
 pub fn write_workbook(wb: &Workbook, wb_rels: &mut Rels, out: &mut Out) -> WorkbookKept {
     let Some(p) = wb.preserved.as_deref() else { return WorkbookKept::default() };
-    let (live, dropped): (Vec<&PreservedSheet>, Vec<&PreservedSheet>) = {
-        let mut refs = HashMap::new();
-        p.sheets.iter().partition(|s| chart_sheet_usable(wb, p, s, &mut refs))
-    };
+    let (live, dropped): (Vec<&PreservedSheet>, Vec<&PreservedSheet>) = p.sheets.iter().partition(|s| chart_sheet_usable(wb, p, s));
     let root_of = |s: &PreservedSheet| p.rels.iter().find(|r| r.id == s.rel_id);
     let dropped_ids: HashSet<&str> = dropped.iter().map(|s| s.rel_id.as_str()).collect();
     // Parts reached only by dropped chart sheets go with them.

crates/xlsx/src/tests/passthrough_tests.rs

@@ -696,32 +696,6 @@ fn chartsheet_package() -> Vec<u8> {
     ])
 }
 
-/// Thousands of chart-sheet entries naming one chart sheet's relationship: its graph is walked
-/// and its charts parsed once when saving, not once per entry.
-#[test]
-fn chart_sheet_entries_sharing_a_graph_are_checked_once() {
-    let bytes = chartsheet_package();
-    let mut z = zip::ZipArchive::new(std::io::Cursor::new(bytes)).unwrap();
-    let mut parts: Vec<(String, Vec<u8>)> = Vec::new();
-    for i in 0..z.len() {
-        let mut f = z.by_index(i).unwrap();
-        let mut data = Vec::new();
-        f.read_to_end(&mut data).unwrap();
-        parts.push((f.name().to_string(), data));
-    }
-    let extra: String = (0..3_000).map(|i| format!(r#"<sheet name="Copy{i}" sheetId="{}" r:id="rId2"/>"#, 100 + i)).collect();
-    for (name, data) in &mut parts {
-        if name == "xl/workbook.xml" {
-            *data = String::from_utf8(data.clone()).unwrap().replace("</sheets>", &format!("{extra}</sheets>")).into_bytes();
-        }
-    }
-    let refs: Vec<(&str, &[u8])> = parts.iter().map(|(n, d)| (n.as_str(), d.as_slice())).collect();
-    let (wb, _) = read_xlsx(&make_zip(&refs)).unwrap();
-    let u = Unpacked::new(&write_xlsx(&wb).unwrap());
-    u.assert_valid();
-    assert!(written_sheets(&u).iter().any(|(n, _)| n == "Chart1"));
-}
-
 /// The sheets of a written workbook: (name, state) in file order.
 fn written_sheets(u: &Unpacked) -> Vec<(String, String)> {
     let wb = xml::parse(u.parts.get("xl/workbook.xml").unwrap()).unwrap();

crates/xlsx/src/tests/xls_tests.rs

@@ -652,3 +652,21 @@ fn xls_short_window2_is_ignored() {
         assert_eq!(v(&wb, 0, "A1"), Value::Number(7.0));
     }
 }
+
+/// Shared-formula followers each keep their own text, charged to the workbook's text allowance:
+/// with no allowance left they keep their cached values, and the import still succeeds.
+#[test]
+fn shared_formula_text_counts_against_the_text_allowance() {
+    let file = compound("Workbook", &sample(false));
+    // The smallest allowance that reads every string: nothing is left for expanded formulas.
+    let left = (0..1000).find(|n| crate::xls::read_xls_with(&file, crate::xls::Budget::with_text_left(*n)).is_ok()).unwrap();
+    let (wb, report) = crate::xls::read_xls_with(&file, crate::xls::Budget::with_text_left(left)).unwrap();
+    assert_eq!(ftext(&wb, "B6"), None);
+    assert_eq!(v(&wb, 0, "B6"), Value::Number(20.0));
+    assert!(report.warnings.iter().any(|w| w.contains("shared-formula text")), "{:?}", report.warnings);
+    // an array anchor after the cut-off still gets its formula
+    assert!(ftext(&wb, "C5").is_some());
+    // With room, the followers keep their formulas.
+    let (wb, _) = crate::xls::read_xls_with(&file, crate::xls::Budget::default()).unwrap();
+    assert!(ftext(&wb, "B6").is_some());
+}

crates/xlsx/src/xls.rs

@@ -75,11 +75,13 @@ pub(crate) struct Budget {
     text: usize,
     records: usize,
     max_records: usize,
+    /// A shared formula's text was refused: no later shared formula, in any sheet, is expanded.
+    shared_text_spent: bool,
 }
 
 impl Default for Budget {
     fn default() -> Self {
-        Budget { cells: 0, text: 0, records: 0, max_records: MAX_RECORDS }
+        Budget { cells: 0, text: 0, records: 0, max_records: MAX_RECORDS, shared_text_spent: false }
     }
 }
 
@@ -89,6 +91,11 @@ impl Budget {
     pub(crate) fn with_max_records(max_records: usize) -> Self {
         Budget { max_records, ..Budget::default() }
     }
+    /// A budget with only `left` bytes of text allowance (tests).
+    #[cfg(test)]
+    pub(crate) fn with_text_left(left: usize) -> Self {
+        Budget { text: MAX_TEXT.saturating_sub(left), ..Budget::default() }
+    }
     fn record(&mut self) -> Result<(), IoError> {
         self.records = self.records.saturating_add(1);
         if self.records > self.max_records {
@@ -103,6 +110,23 @@ impl Budget {
         }
         Ok(())
     }
+    /// Charges `len` bytes of text if they fit the allowance, leaving it untouched otherwise:
+    /// for text the import can do without (expanded shared formulas keep their cached values).
+    fn try_text(&mut self, len: usize) -> bool {
+        let total = self.text.saturating_add(len);
+        let fits = total <= MAX_TEXT;
+        if fits {
+            self.text = total;
+        }
+        fits
+    }
+    /// [`try_text`](Self::try_text) for a shared formula's text. The first refusal ends shared
+    /// expansion for the whole workbook, so a shorter formula on a later sheet doesn't fit in the
+    /// room the refused one left, and refused formulas aren't decoded again on each sheet.
+    fn try_shared_text(&mut self, len: usize) -> bool {
+        self.shared_text_spent = self.shared_text_spent || !self.try_text(len);
+        !self.shared_text_spent
+    }
     fn text(&mut self, len: usize) -> Result<(), IoError> {
         self.text = self.text.saturating_add(len);
         if self.text > MAX_TEXT {
@@ -487,6 +511,7 @@ fn rk(v: u32) -> Value {
 /// keep their cached values).
 fn resolve_pending(
     st: &mut SheetState<'_>,
+    budget: &mut Budget,
     arrays: &HashMap<CellRef, (RangeRef, Parsed)>,
     shared: &HashMap<CellRef, Parsed>,
     pending: &[Pending],
@@ -499,11 +524,27 @@ fn resolve_pending(
                 st.attach(p.pos, f, Mode::Cell, Some(*range));
             }
         } else if let Some(f) = shared.get(&p.anchor) {
+            if budget.shared_text_spent {
+                st.unsupported("more shared-formula text than the import allows");
+                continue;
+            }
             if failed.contains(&p.anchor) || shared_attaches >= MAX_SHARED_ATTACHES {
                 continue;
             }
             shared_attaches += 1;
             st.attach(p.pos, f, Mode::Shared, None);
+            let len = st.sheet.cell(p.pos).and_then(|c| c.formula.as_ref()).map(|f| f.text.len());
+            // Every follower keeps its own text: charge it to the workbook's text allowance, and
+            // past it drop the formula again so the cell keeps its cached value.
+            if let Some(len) = len
+                && !budget.try_shared_text(len)
+            {
+                if let Some(cell) = st.sheet.cells.get_mut(p.pos) {
+                    cell.formula = None;
+                }
+                st.unsupported("more shared-formula text than the import allows");
+                continue;
+            }
             let imported = st.sheet.cell(p.pos).and_then(|c| c.formula.as_ref()).is_some_and(|f| f.text.chars().count() <= MAX_SHARED_TEMPLATE);
             if !imported {
                 failed.insert(p.anchor);
@@ -586,7 +627,7 @@ fn worksheet(
     // Shared and array formulas: their definitions follow the first cell that uses them.
     let (arrays, shared) = (std::mem::take(&mut st.arrays), std::mem::take(&mut st.shared));
     let pending = std::mem::take(&mut st.pending);
-    resolve_pending(&mut st, &arrays, &shared, &pending);
+    resolve_pending(&mut st, budget, &arrays, &shared, &pending);
     blank_array_followers(&mut st, &pending);
     if st.dropped > 0 {
         report.warn(format!("{} cells outside the grid were skipped.", st.dropped));
@@ -878,3 +919,18 @@ fn defined_names(g: &Globals, model_index: &[Option<usize>], wb: &mut Workbook,
         }
     }
 }
+
+#[cfg(test)]
+mod budget_tests {
+    use super::*;
+
+    #[test]
+    fn a_refused_shared_formula_ends_shared_expansion_for_later_sheets() {
+        let mut budget = Budget::with_text_left(10);
+        assert!(budget.try_shared_text(8));
+        assert!(!budget.try_shared_text(8), "over the allowance");
+        // a shorter formula would fit the 2 bytes left, but the cut-off holds
+        assert!(!budget.try_shared_text(1));
+        assert!(budget.try_text(1), "text the import needs is still charged normally");
+    }
+}