From 76e53085e30f36f059d67b022148c2630db09dd1 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 06:15:08 -0500 Subject: [PATCH] xlsx: keep the cached value of a follower whose shared-formula copy is refused (#3212) * xlsx: one hyperlink allowance per sheet, and a copy limit for unparsable shared masters Every 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. * address review: keep the cached value of a follower dropped for the shared-formula copy limit inside array ranges --- crates/xlsx/src/sheet_read.rs | 8 +++++++- crates/xlsx/src/tests/malformed.rs | 15 +++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/crates/xlsx/src/sheet_read.rs b/crates/xlsx/src/sheet_read.rs index 40d4b1c36..f2ec76683 100644 --- a/crates/xlsx/src/sheet_read.rs +++ b/crates/xlsx/src/sheet_read.rs @@ -55,6 +55,9 @@ struct RowState { dynamic_cells: u64, /// Row-membership checks served so far, for the work cap above. dynamic_checks: u64, + /// The cell being read lost its shared formula to a copy limit: its cached value is kept, even + /// inside an array range (where a formula's cached values otherwise give way to the spill). + shared_copy_refused: bool, dropped: u64, rows: u64, } @@ -607,6 +610,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 @@ -632,7 +636,7 @@ fn read_cell(cx: &mut Ctx<'_>, sheet: &mut Sheet, st: &mut RowState, c: &El, pos if formula.as_ref().is_some_and(|f| !f.dynamic && f.array.is_none() && f.expr_arc().is_some()) { cx.legacy_formulas.push(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. @@ -695,10 +699,12 @@ fn read_formula_text(cx: &mut Ctx<'_>, st: &mut RowState, f: &El, pos: CellRef, "a shared formula is longer than {} characters; kept the cached values of the cells that share it", gridcraft_formula::MAX_FORMULA_LEN )); + st.shared_copy_refused = true; return None; } if !charge(&mut cx.shared_formula_bytes, extra_shared_cost(sh.cost), MAX_SHARED_FORMULA_BYTES) { cx.warn("shared formulas expand to too much data; kept the cached values of the remaining cells that share them"); + st.shared_copy_refused = true; return None; } *unwrapped |= sh.unwrapped; diff --git a/crates/xlsx/src/tests/malformed.rs b/crates/xlsx/src/tests/malformed.rs index 9a533d9ca..870ce316d 100644 --- a/crates/xlsx/src/tests/malformed.rs +++ b/crates/xlsx/src/tests/malformed.rs @@ -539,3 +539,18 @@ fn outline_summary_directions_round_trip() { let xml = String::from_utf8(crate::package::Package::open(&bytes).unwrap().read("xl/worksheets/sheet1.xml").unwrap().unwrap()).unwrap(); assert!(!xml.contains("outlinePr"), "{xml}"); } + +/// A shared follower whose copy is refused keeps its cached value even inside an array range. +#[test] +fn refused_shared_copies_keep_their_values_inside_array_ranges() { + let master = format!("1+(\"{}", "a".repeat(gridcraft_formula::MAX_FORMULA_LEN)); + let rows = format!( + r#"11{master}17"# + ); + let (wb, rep) = read_xlsx(&minimal(&format!("{rows}"), &[], "", "")).unwrap(); + let s = wb.sheet(0).unwrap(); + let c2 = gridcraft_core::CellRef::new(1, 2); + assert!(s.cell(c2).is_some_and(|c| c.formula.is_none())); + assert_eq!(s.value(c2), gridcraft_core::Value::Number(7.0)); + assert!(rep.warnings.iter().any(|w| w.contains("longer than")), "{:?}", rep.warnings); +}