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 <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.

* address review: keep the cached value of a follower dropped for the shared-formula copy limit inside array ranges
This commit is contained in:
Dylan M. Taylor authored and GitHub committed 2026-11-11 06:15:08 -05:00
1 parent bc2dabc7ea
commit 76e53085e3
2 files changed
+22 -1

No files matched your search

+7 -1
View File
@@ -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;
+15
View File
@@ -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#"<row r="1"><c r="B1"><f t="array" ref="B1:C3">1</f><v>1</v></c><c r="C1"><f t="shared" ref="C1:C3" si="0">{master}</f><v>1</v></c></row><row r="2"><c r="C2"><f t="shared" si="0"/><v>7</v></c></row>"#
);
let (wb, rep) = read_xlsx(&minimal(&format!("<sheetData>{rows}</sheetData>"), &[], "", "")).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);
}