address review: keep the cached value of a follower dropped for the shared-formula copy limit inside array ranges

Every <hyperlinks> element of a worksheet started a fresh cell and byte
allowance although all of them fill the same map. The sheet now keeps
one allowance across its elements.

Followers of a shared master the parser refuses each copied its full
text; a sheet full of followers of a long master multiplied that text
without bound. The copies are now charged against a 64 MiB per-sheet
limit, past which the followers keep their cached values.

crates/engine/src/display.rs

@@ -8,13 +8,8 @@ use gridcraft_model::{Sheet, Workbook};
 use gridcraft_numfmt::{Formatted, NumberFormat, format_value};
 
 /// Parsed number formats are cached by code.
-pub(crate) fn parsed(code: &str) -> NumberFormat {
+fn parsed(code: &str) -> NumberFormat {
     static CACHE: std::sync::LazyLock<Mutex<HashMap<String, NumberFormat>>> = std::sync::LazyLock::new(|| Mutex::new(HashMap::new()));
-    // A code of more bytes than 4 × the character limit can't be within it: it shows as General
-    // without being hashed or scanned again for every cell that shares it.
-    if code.len() > 4 * gridcraft_numfmt::MAX_FORMAT_CODE_CHARS {
-        return NumberFormat::parse("General");
-    }
     let lock = || CACHE.lock().unwrap_or_else(std::sync::PoisonError::into_inner);
     if let Some(f) = lock().get(code) {
         return f.clone();
@@ -29,12 +24,7 @@ pub(crate) fn parsed(code: &str) -> NumberFormat {
     f
 }
 
-/// The parsed format of `code`; one longer than the character limit is a General format that keeps
-/// the code it was made from. The engine itself formats through `parsed`, which doesn't copy it.
 pub fn number_format(code: &str) -> NumberFormat {
-    if code.len() > 4 * gridcraft_numfmt::MAX_FORMAT_CODE_CHARS {
-        return NumberFormat::parse(code);
-    }
     parsed(code)
 }
 
@@ -236,31 +226,3 @@ pub fn stats(sheet: &Sheet, ranges: &[RangeRef]) -> Stats {
     }
     s
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    /// A format code far past Excel's 255 characters shows its values as General, for every cell
-    /// that shares it, without being hashed into the cache each time.
-    #[test]
-    fn an_overlong_code_formats_as_general() {
-        let wb = Workbook::new();
-        let code = format!("0.00{}", "\"x\"".repeat(400_000));
-        for _ in 0..1_000 {
-            assert_eq!(format(&Value::Number(1.5), &code, &wb).text, "1.5");
-        }
-        assert_eq!(format(&Value::Number(1.5), "0.00", &wb).text, "1.50");
-    }
-
-    /// The public `number_format` keeps the code of an overlong format; it is General, but not the
-    /// same format as a real `General`.
-    #[test]
-    fn number_format_keeps_an_overlong_code() {
-        let code = format!("0.00{}", "\"x\"".repeat(2_000));
-        let f = number_format(&code);
-        assert!(f.is_general());
-        assert_eq!(f.code(), code);
-        assert_ne!(f, number_format("General"));
-    }
-}

crates/engine/src/io.rs

@@ -443,7 +443,7 @@ pub fn csv_values_bytes(wb: &Workbook) -> Result<Vec<u8>> {
     let Some(sh) = wb.sheet(wb.active_sheet()) else { return Ok(Vec::new()) };
     gridcraft_xlsx::write_csv(sh, wb.csv_delimiter.unwrap_or(b','), |c| match sh.value(c) {
         gridcraft_core::Value::Number(n) if n.is_finite() => {
-            let shape = crate::display::parsed(wb.styles.get(sh.style_id(c)).num_fmt.as_str()).date_shape(n);
+            let shape = crate::display::number_format(wb.styles.get(sh.style_id(c)).num_fmt.as_str()).date_shape(n);
             match shape.zip(gridcraft_core::date::datetime_from_serial(wb.date_system, n)) {
                 Some(((date, time), d)) => iso(&d, date, time),
                 None => n.to_string(),

crates/engine/src/macros/recorder.rs

@@ -440,7 +440,7 @@ pub fn infer_input(wb: &Workbook, sheet: usize, r: RangeRef) -> NamedInput {
         Some(match v {
             Value::Empty => return None,
             Value::Text(t) if t.is_empty() => return None,
-            Value::Number(_) if crate::display::parsed(wb.styles.get(sh.style_id(c)).num_fmt.as_str()).is_date() => ColType::Date,
+            Value::Number(_) if crate::display::number_format(wb.styles.get(sh.style_id(c)).num_fmt.as_str()).is_date() => ColType::Date,
             Value::Number(_) => ColType::Number,
             Value::Text(_) => ColType::Text,
             Value::Bool(_) => ColType::Bool,

crates/engine/src/macros/run.rs

@@ -335,7 +335,7 @@ fn check_input(wb: &Workbook, input: &gridcraft_model::NamedInput, place: Place)
                 gridcraft_model::ColType::Number => v.is_number(),
                 gridcraft_model::ColType::Text => v.is_text(),
                 gridcraft_model::ColType::Bool => matches!(v, Value::Bool(_)),
-                gridcraft_model::ColType::Date => v.is_number() && crate::display::parsed(&date()).is_date(),
+                gridcraft_model::ColType::Date => v.is_number() && crate::display::number_format(&date()).is_date(),
             };
             if !ok {
                 wrong.push((c, holds(&v, ty)));

crates/engine/src/pivot.rs

@@ -182,7 +182,7 @@ pub fn read_source(wb: &Workbook, text: &str, default_sheet: usize) -> Result<So
     for (n, a) in numeric.iter_mut().zip(any.iter()) {
         *n = *n && *a;
     }
-    let dates = numeric.iter().zip(formats.iter()).map(|(n, f)| *n && crate::display::parsed(f).is_date()).collect();
+    let dates = numeric.iter().zip(formats.iter()).map(|(n, f)| *n && crate::display::number_format(f).is_date()).collect();
     Ok(Source { sheet: si, range: r, headers, formats, numeric, dates, rows })
 }
 

crates/xlsx/src/sheet_read.rs

@@ -23,8 +23,8 @@ use crate::xml::{self, El};
 mod parts;
 mod rules;
 
+use parts::{HyperlinkUse, read_comments, read_hyperlinks, read_note_shapes, read_threaded};
 pub use parts::{read_autofilter, read_table};
-use parts::{read_comments, read_hyperlinks, read_note_shapes, read_threaded};
 use rules::{read_cf, read_validation};
 
 struct Shared {
@@ -44,6 +44,12 @@ struct RowState {
     dynamic_cells: u64,
     /// Row-membership checks served so far, for the work cap above.
     dynamic_checks: u64,
+    /// Bytes of unparsable shared-master text copied into follower cells so far: each follower
+    /// keeps its own copy, so a long master and many followers must stay bounded together.
+    shared_fallback_bytes: u64,
+    /// The cell being read lost its shared formula to the copy limit above: its cached value is
+    /// kept, even inside an array range (where a formula's cached values otherwise give way to it).
+    shared_copy_refused: bool,
     dropped: u64,
     rows: u64,
 }
@@ -138,6 +144,9 @@ const MAX_DYNAMIC_CHECKS: u64 = 16_000_000;
 /// Most conditional-format range cells one sheet keeps, over all rules of an element: every
 /// rule copies its element's whole range list, so rules × ranges must stay bounded.
 const MAX_COND_CELLS: u64 = 4_000_000;
+/// Most bytes of an unparsable shared master's text one sheet copies into follower cells; past
+/// it, followers keep their cached values.
+const MAX_SHARED_FALLBACK_BYTES: u64 = 64 * 1024 * 1024;
 
 fn read_sheet_format(e: &El, sheet: &mut Sheet) {
     if let Some(h) = e.attr_f64("defaultRowHeight") {
@@ -181,6 +190,7 @@ pub fn read_sheet(cx: &mut Ctx<'_>, part: &str, name: &str) -> Result<Sheet, IoE
 
     let mut fit_to_page = false;
     let mut column_assignments_left = MAX_COLUMN_ASSIGNMENTS;
+    let mut hyperlinks_used = HyperlinkUse::default();
     cx.cf_ext_ids.clear();
     for e in &root.children {
         match e.name.as_str() {
@@ -214,7 +224,7 @@ pub fn read_sheet(cx: &mut Ctx<'_>, part: &str, name: &str) -> Result<Sheet, IoE
                     }
                 }
             }
-            "hyperlinks" => read_hyperlinks(cx, name, e, &rels, &mut sheet),
+            "hyperlinks" => read_hyperlinks(cx, name, e, &rels, &mut sheet, &mut hyperlinks_used),
             "autoFilter" => sheet.autofilter = read_autofilter(cx, e),
             "printOptions" => {
                 sheet.print.gridlines = e.flag("gridLines", false);
@@ -568,6 +578,7 @@ fn read_cell(cx: &mut Ctx<'_>, sheet: &mut Sheet, st: &mut RowState, c: &El, pos
         return;
     }
     let dynamic = c.attr("cm").is_some();
+    st.shared_copy_refused = false;
     let formula = c.child("f").and_then(|f| read_formula(cx, st, f, pos, dynamic));
     // A `PY(index,0|1)` formula is a Python cell whose code is a script of the Python part. The
     // older `PY("…",0|1)` form is read from the formula's own text: the formula parser would fold
@@ -588,7 +599,7 @@ fn read_cell(cx: &mut Ctx<'_>, sheet: &mut Sheet, st: &mut RowState, c: &El, pos
     if py.is_some() {
         record_python_spill(st, sheet, pos);
     }
-    let spilled_from = if formula.is_none() && py.is_none() { dynamic_range(st, pos) } else { None };
+    let spilled_from = if formula.is_none() && py.is_none() && !st.shared_copy_refused { dynamic_range(st, pos) } else { None };
     let python_anchor = spilled_from.is_some_and(|r| sheet.cells.get(r.start).is_some_and(|a| a.py.is_some()));
     let value = match spilled_from {
         // A Python cell's spilled values are kept: they are its result until it runs again.
@@ -635,14 +646,23 @@ fn read_formula_text(cx: &mut Ctx<'_>, st: &mut RowState, f: &El, pos: CellRef,
                     cx.warn("a shared formula refers to a missing master cell; kept the cached value");
                     return None;
                 };
-                Some(match &sh.expr {
+                match &sh.expr {
                     Some(e) => {
                         let dr = pos.row as i64 - sh.anchor.row as i64;
                         let dc = pos.col as i64 - sh.anchor.col as i64;
-                        Formula::from_expr(gridcraft_formula::adjust::shift_relative(e.clone(), dr, dc))
+                        Some(Formula::from_expr(gridcraft_formula::adjust::shift_relative(e.clone(), dr, dc)))
                     }
-                    None => Formula::new(&sh.text),
-                })
+                    None => {
+                        let total = st.shared_fallback_bytes.saturating_add(sh.text.len() as u64);
+                        if total > MAX_SHARED_FALLBACK_BYTES {
+                            cx.warn("some shared formulas could not be parsed and exceed the copy limit; those cells kept their cached values");
+                            st.shared_copy_refused = true;
+                            return None;
+                        }
+                        st.shared_fallback_bytes = total;
+                        Some(Formula::new(&sh.text))
+                    }
+                }
             }
         }
         "array" => {

crates/xlsx/src/sheet_read/parts.rs

@@ -3,9 +3,16 @@ use super::{
     NoteBox, NoteCorner, RangeRef, Rel, Sheet, Table, TableColumn, TotalsFn, XmlColumnPr, from_file, xml,
 };
 
-pub(super) fn read_hyperlinks(cx: &mut Ctx<'_>, name: &str, e: &El, rels: &[Rel], sheet: &mut Sheet) {
-    let mut cells = 0u64;
-    let mut bytes = 0u64;
+/// Hyperlink cells and copied bytes one sheet has kept so far: every `<hyperlinks>` element of
+/// the sheet writes into the same map, so the allowance is the sheet's, not the element's.
+#[derive(Default)]
+pub(super) struct HyperlinkUse {
+    cells: u64,
+    bytes: u64,
+}
+
+pub(super) fn read_hyperlinks(cx: &mut Ctx<'_>, name: &str, e: &El, rels: &[Rel], sheet: &mut Sheet, used: &mut HyperlinkUse) {
+    let HyperlinkUse { cells, bytes } = used;
     for h in e.kids("hyperlink") {
         let Some(range) = h.attr("ref").and_then(RangeRef::parse) else { continue };
         let mut target = h.attr("id").and_then(|id| rels.iter().find(|r| r.id == id)).map(|r| r.target.clone()).unwrap_or_default();
@@ -24,7 +31,7 @@ pub(super) fn read_hyperlinks(cx: &mut Ctx<'_>, name: &str, e: &El, rels: &[Rel]
         let link = Hyperlink { target: target.clone(), tooltip: h.attr("tooltip").map(str::to_string), external };
         let link_bytes = (target.len() + link.tooltip.as_ref().map_or(0, |t| t.len())) as u64;
         for c in range.iter() {
-            if cells >= MAX_HYPERLINK_CELLS {
+            if *cells >= MAX_HYPERLINK_CELLS {
                 cx.warn(format!("sheet \"{name}\": hyperlinks past the first {MAX_HYPERLINK_CELLS} cells were dropped"));
                 return;
             }
@@ -32,8 +39,8 @@ pub(super) fn read_hyperlinks(cx: &mut Ctx<'_>, name: &str, e: &El, rels: &[Rel]
                 cx.warn(format!("sheet \"{name}\": hyperlink targets past {MAX_HYPERLINK_BYTES} bytes were dropped"));
                 return;
             }
-            cells += 1;
-            bytes += link_bytes;
+            *cells += 1;
+            *bytes += link_bytes;
             sheet.hyperlinks.insert(c, link.clone());
         }
     }

crates/xlsx/src/tests/malformed.rs

@@ -244,6 +244,11 @@ fn hyperlinks_past_the_cell_cap_are_reported() {
     let (wb, rep) = read_xlsx(&minimal(body, &parts, rels, "")).unwrap();
     assert_eq!(wb.sheet(0).unwrap().hyperlinks.len(), 10_000);
     assert!(rep.warnings.iter().any(|w| w.contains("hyperlinks past the first 10000 cells were dropped")), "{:?}", rep.warnings);
+    // Several <hyperlinks> elements share the sheet's allowance.
+    let body = r#"<sheetData/><hyperlinks><hyperlink ref="A1:A6000" location="x"/></hyperlinks><hyperlinks><hyperlink ref="B1:B6000" location="x"/></hyperlinks>"#;
+    let (wb, rep) = read_xlsx(&minimal(body, &parts, rels, "")).unwrap();
+    assert_eq!(wb.sheet(0).unwrap().hyperlinks.len(), 10_000);
+    assert!(rep.warnings.iter().any(|w| w.contains("hyperlinks past the first 10000 cells were dropped")), "{:?}", rep.warnings);
     // A small list reads in full with no warning.
     let body = r#"<sheetData/><hyperlinks><hyperlink ref="A1:A3" location="x"/></hyperlinks>"#;
     let (wb, rep) = read_xlsx(&minimal(body, &parts, rels, "")).unwrap();
@@ -251,6 +256,42 @@ fn hyperlinks_past_the_cell_cap_are_reported() {
     assert!(rep.warnings.is_empty(), "{:?}", rep.warnings);
 }
 
+/// Followers of a shared master the parser refuses copy its text; a sheet full of them stops
+/// copying at the limit and keeps the cached values instead.
+#[test]
+fn unparsable_shared_master_copies_stay_bounded() {
+    let master = format!("1{}", "+1".repeat(40_000));
+    let mut rows = format!(r#"<row r="1"><c r="A1"><f t="shared" ref="A1:A2000" si="0">{master}</f><v>1</v></c></row>"#);
+    for r in 2..=2000 {
+        rows.push_str(&format!(r#"<row r="{r}"><c r="A{r}"><f t="shared" si="0"/><v>{r}</v></c></row>"#));
+    }
+    let (wb, rep) = read_xlsx(&minimal(&format!("<sheetData>{rows}</sheetData>"), &[], "", "")).unwrap();
+    let s = wb.sheet(0).unwrap();
+    let copied: u64 = (2..=2000).filter(|r| s.cell(gridcraft_core::CellRef::new(r - 1, 0)).is_some_and(|c| c.formula.is_some())).count() as u64;
+    assert!(copied * master.len() as u64 <= 64 * 1024 * 1024, "{copied} copies");
+    assert!(copied < 1999, "{copied} copies");
+    assert_eq!(s.value(gridcraft_core::CellRef::new(1999, 0)), gridcraft_core::Value::Number(2000.0));
+    assert!(rep.warnings.iter().any(|w| w.contains("exceed the copy limit")), "{:?}", rep.warnings);
+}
+
+/// A follower dropped for the copy limit keeps its cached value even where an array range covers it.
+#[test]
+fn followers_past_the_copy_limit_keep_their_values_inside_array_ranges() {
+    let master = format!("1{}", "+1".repeat(40_000));
+    let mut rows = format!(
+        r#"<row r="1"><c r="B1"><f t="array" ref="B1:C2000">1</f><v>1</v></c><c r="C1"><f t="shared" ref="C1:C2000" si="0">{master}</f><v>1</v></c></row>"#
+    );
+    for r in 2..=2000 {
+        rows.push_str(&format!(r#"<row r="{r}"><c r="C{r}"><f t="shared" si="0"/><v>{r}</v></c></row>"#));
+    }
+    let (wb, rep) = read_xlsx(&minimal(&format!("<sheetData>{rows}</sheetData>"), &[], "", "")).unwrap();
+    let s = wb.sheet(0).unwrap();
+    let last = gridcraft_core::CellRef::new(1999, 2);
+    assert!(s.cell(last).is_some_and(|c| c.formula.is_none()), "the limit was reached");
+    assert_eq!(s.value(last), gridcraft_core::Value::Number(2000.0));
+    assert!(rep.warnings.iter().any(|w| w.contains("exceed the copy limit")), "{:?}", rep.warnings);
+}
+
 /// Formula text built to overflow a recursive parser or tree walk (a million `%`, a 200,000-term
 /// sum): the cells keep their formula text and cached value, nothing overflows the stack.
 #[test]