diff --git a/.github/workflows/flatpak-pages.yml b/.github/workflows/flatpak-pages.yml index cd8f12ab3..d08ac430a 100644 --- a/.github/workflows/flatpak-pages.yml +++ b/.github/workflows/flatpak-pages.yml @@ -19,6 +19,7 @@ concurrency: jobs: build: + timeout-minutes: 60 name: Build Flatpak repository # Pre-releases (rc builds) never reach the stable branch: a published pre-release is skipped # here, and the validation step below refuses a manual tag with a pre-release suffix. @@ -161,6 +162,7 @@ jobs: include-hidden-files: true deploy: + timeout-minutes: 60 name: Deploy Pages needs: build runs-on: ubuntu-24.04 diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index d31ebe08b..0f1b2de3c 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -1260,6 +1260,9 @@ fn clear(s: &mut Session, p: &Json, what: &str) -> Result { .sheet(sheet) .map(|sh| sh.cells.iter_range(r).filter(|(c, _)| !skip.contains(&c.row)).map(|(c, x)| (c, x.clone())).collect()) .unwrap_or_default(); + // Report every stored cell we touch, even beyond a million coordinates in a sparse + // selection. Removed formula anchors must also reach calc so their spills disappear. + cx.changed.extend(cells.iter().map(|(c, _)| (sheet, *c))); let mut cells = cells; if what == "contents" { for (_, cell) in cells.iter_mut() { @@ -1299,10 +1302,24 @@ fn clear(s: &mut Session, p: &Json, what: &str) -> Result { sh.hyperlinks.retain(|c, _| !cleared(*c)); } if matches!(what, "all" | "formats") { - sh.merges.retain(|m| !r.contains_range(m) || skip.range(m.start.row..=m.end.row).next().is_some()); - } - for c in r.iter().take(1_000_000) { - cx.changed.push((sheet, c)); + let mut unmerged = false; + sh.merges.retain(|m| { + let keep = !r.contains_range(m) || skip.range(m.start.row..=m.end.row).next().is_some(); + unmerged |= !keep; + keep + }); + if unmerged { + // An empty merge can block a spill without storing any cells in this range. + // As in Unmerge, retry blocked formula anchors that could reach the freed area. + let reach = RangeRef::new(CellRef::new(0, 0), r.end); + let blocked: Vec<_> = sh + .cells + .iter_range(reach) + .filter(|(_, cell)| cell.formula.is_some() && cell.value == Value::Error(gridcraft_core::CellError::Spill)) + .map(|(c, _)| (sheet, c)) + .collect(); + cx.changed.extend(blocked); + } } } Ok(Json::Null) diff --git a/crates/engine/tests/sparse_clear_invalidation.rs b/crates/engine/tests/sparse_clear_invalidation.rs new file mode 100644 index 000000000..3ac8f77e3 --- /dev/null +++ b/crates/engine/tests/sparse_clear_invalidation.rs @@ -0,0 +1,87 @@ +//! Clearing a large sparse selection must invalidate every changed cell (issue #1032). + +use gridcraft_core::{CellError, CellRef, Value}; +use gridcraft_engine::Session; +use serde_json::json; + +fn value(s: &Session, address: &str) -> Value { + s.doc().unwrap().wb.active().unwrap().value(CellRef::parse(address).unwrap()) +} + +#[test] +fn large_clear_updates_distant_direct_and_transitive_dependents() { + for command in ["edit.clearContents", "edit.clearAll"] { + let mut s = Session::new(); + s.new_workbook(); + for (cell, input) in [("A1000001", "7"), ("B1", "=A1000001"), ("C1", "=B1*2")] { + s.execute("cell.set", json!({"cell": cell, "input": input})).unwrap(); + } + assert_eq!((value(&s, "B1"), value(&s, "C1")), (Value::Number(7.0), Value::Number(14.0))); + + // B1 stretches the used range to row 1; the changed source lies just past the old cap. + s.execute(command, json!({"range": "A1:A1000001"})).unwrap(); + assert_eq!(value(&s, "A1000001"), Value::Empty, "{command}"); + assert_eq!((value(&s, "B1"), value(&s, "C1")), (Value::Number(0.0), Value::Number(0.0)), "{command}"); + + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(value(&s, "A1000001"), Value::Number(7.0), "{command} undo"); + assert_eq!((value(&s, "B1"), value(&s, "C1")), (Value::Number(7.0), Value::Number(14.0)), "{command} undo"); + s.execute("edit.redo", json!({})).unwrap(); + assert_eq!(value(&s, "A1000001"), Value::Empty, "{command} redo"); + assert_eq!((value(&s, "B1"), value(&s, "C1")), (Value::Number(0.0), Value::Number(0.0)), "{command} redo"); + } +} + +#[test] +fn large_clear_removes_a_distant_formulas_spill_and_updates_its_dependent() { + for command in ["edit.clearContents", "edit.clearAll"] { + let mut s = Session::new(); + s.new_workbook(); + s.execute("cell.set", json!({"cell": "A1000001", "input": "=SEQUENCE(2)"})).unwrap(); + s.execute("cell.set", json!({"cell": "B1", "input": "=A1000002"})).unwrap(); + assert_eq!((value(&s, "A1000002"), value(&s, "B1")), (Value::Number(2.0), Value::Number(2.0))); + + // The spill lies outside the selection, but must disappear when its anchor is cleared. + s.execute(command, json!({"range": "A1:A1000001"})).unwrap(); + assert_eq!(value(&s, "A1000001"), Value::Empty, "{command}"); + assert_eq!(value(&s, "A1000002"), Value::Empty, "{command}"); + assert_eq!(value(&s, "B1"), Value::Number(0.0), "{command}"); + } +} + +#[test] +fn clearing_an_empty_merge_unblocks_a_spill() { + for command in ["edit.clearFormats", "edit.clearAll"] { + let mut s = Session::new(); + s.new_workbook(); + s.execute("cell.set", json!({"cell": "A1", "input": "=SEQUENCE(3)"})).unwrap(); + // Keep the empty merged area inside the used range after the spill becomes blocked. + s.execute("cell.set", json!({"cell": "C4", "input": "1"})).unwrap(); + s.execute("home.mergeCells", json!({"range": "A2:B3"})).unwrap(); + assert_eq!(value(&s, "A1"), Value::Error(CellError::Spill)); + + s.execute(command, json!({"range": "A2:B3"})).unwrap(); + assert_eq!((value(&s, "A1"), value(&s, "A3")), (Value::Number(1.0), Value::Number(3.0)), "{command}"); + assert!(s.doc().unwrap().wb.active().unwrap().merges.is_empty(), "{command}"); + } +} + +#[test] +fn clearing_a_distant_styled_blocker_unblocks_a_spill() { + let mut s = Session::new(); + s.new_workbook(); + s.execute("cell.set", json!({"cell": "A1", "input": "=SEQUENCE(100)"})).unwrap(); + s.execute("cell.set", json!({"cell": "A90", "input": "x"})).unwrap(); + s.execute("home.bold", json!({"range": "A90", "on": true})).unwrap(); + assert_eq!(value(&s, "A1"), Value::Error(CellError::Spill)); + + // The cleared blocker stays stored for its style and lies beyond the local 64-row scan. + s.execute("edit.clearContents", json!({"range": "A2:A100"})).unwrap(); + assert_eq!(value(&s, "A1"), Value::Number(1.0)); + assert_eq!(value(&s, "A90"), Value::Number(90.0)); + assert_eq!(value(&s, "A100"), Value::Number(100.0)); + let wb = &s.doc().unwrap().wb; + let cell = wb.active().unwrap().cell(CellRef::parse("A90").unwrap()).unwrap(); + assert_eq!(cell.value, Value::Empty); + assert!(wb.styles.get(cell.style).font.bold); +} diff --git a/crates/formula/src/lib.rs b/crates/formula/src/lib.rs index e193c3fb4..88d09a8af 100644 --- a/crates/formula/src/lib.rs +++ b/crates/formula/src/lib.rs @@ -12,7 +12,7 @@ pub mod printer; pub use ast::{Anchor, BinOp, Expr, RefKind, Reference, SheetSel, StructItem, StructRef, TrimMode, UnOp}; pub use input::{FormulaLocale, parse_input, print_input}; -pub use parser::{ParseError, normalize_function_name, parse}; +pub use parser::{MAX_FORMULA_LEN, ParseError, normalize_function_name, parse}; pub use printer::{print, print_r1c1, quote_sheet}; #[cfg(test)] diff --git a/crates/formula/src/parser.rs b/crates/formula/src/parser.rs index 3ea97085e..555717be9 100644 --- a/crates/formula/src/parser.rs +++ b/crates/formula/src/parser.rs @@ -17,6 +17,11 @@ pub struct ParseError { pub(crate) const MAX_DEPTH: usize = 256; +/// Longest formula accepted, in characters: Excel's limit for formula contents. It also bounds +/// the depth of the trees long operator chains build (`1+1+…`), which dropping and evaluating +/// walk recursively; without it a formula of a few hundred kilobytes in a file overflows the stack. +pub const MAX_FORMULA_LEN: usize = 8_192; + /// Parses formula text. A leading `=` (or `+`/`-` as Lotus users type) is accepted and skipped /// for `=`. pub fn parse(src: &str) -> Result { @@ -25,6 +30,9 @@ pub fn parse(src: &str) -> Result { pub(crate) fn parse_syntax(src: &str, locale: crate::FormulaLocale) -> Result { let body = src.strip_prefix('=').unwrap_or(src); + if body.len() > MAX_FORMULA_LEN && body.chars().count() > MAX_FORMULA_LEN { + return Err(ParseError { msg: format!("formula is longer than {MAX_FORMULA_LEN} characters"), pos: 0 }); + } let toks = tokenize_locale(body, locale).map_err(|e| ParseError { msg: e.msg, pos: e.pos })?; let toks = significant_spaces(toks); let mut p = Parser { toks, i: 0, depth: 0, input: locale == crate::FormulaLocale::Es }; @@ -160,11 +168,18 @@ impl Parser { return Err(self.err("formula is nested too deeply")); } let mut lhs = self.prefix(allow_union)?; + // Postfix operators (`%`, invoking a result) wrap `lhs` without recursing: they count + // towards the nesting limit, so `1%%%…` can't build a tree deeper than it. + let mut postfix = 0; loop { if *self.peek() == Tok::Op("%") { if 70 < min_bp { break; } + postfix += 1; + if self.depth + postfix > MAX_DEPTH { + return Err(self.err("formula is nested too deeply")); + } self.next(); lhs = Expr::Unary(UnOp::Percent, Box::new(lhs)); continue; @@ -174,6 +189,10 @@ impl Parser { if let Expr::Name(_) = lhs { // A name followed by '(' was lexed as Func; this is `(name)(…)` only. } + postfix += 1; + if self.depth + postfix > MAX_DEPTH { + return Err(self.err("formula is nested too deeply")); + } self.next(); let args = self.args()?; lhs = Expr::Invoke(Box::new(lhs), args); diff --git a/crates/formula/src/tests.rs b/crates/formula/src/tests.rs index 103a26bd1..d23eba0e2 100644 --- a/crates/formula/src/tests.rs +++ b/crates/formula/src/tests.rs @@ -366,3 +366,38 @@ fn prints_long_flat_chain_without_deep_recursion() { assert_eq!(text, vec!["1"; 20001].join("+")); assert_eq!(r1c1, text); } + +#[test] +fn hostile_formulas_are_errors_not_stack_overflows() { + // A small stack: parsing must fail cleanly instead of building trees deeper than the nesting + // limit, which dropping, printing and evaluating walk recursively. + let handle = std::thread::Builder::new() + .stack_size(512 * 1024) + .spawn(|| { + let percents = format!("1{}", "%".repeat(1_000)); + let invokes = format!("LAMBDA(x,x)(1){}", "(1)".repeat(1_000)); + let long_chain = format!("1{}", "+1".repeat(100_000)); + let huge_percents = format!("1{}", "%".repeat(1_000_000)); + (parse(&percents).is_err(), parse(&invokes).is_err(), parse(&long_chain).is_err(), parse(&huge_percents).is_err()) + }) + .expect("spawn"); + assert_eq!(handle.join().expect("parsing must not overflow the stack"), (true, true, true, true)); + // Within the limits these still parse. + assert!(parse(&format!("1{}", "%".repeat(100))).is_ok()); + assert!(parse("LAMBDA(x,x)(1)(2)").is_ok()); +} + +#[test] +fn formulas_longer_than_excel_allows_are_refused() { + let at_limit = format!("1{}", "+1".repeat((crate::MAX_FORMULA_LEN - 1) / 2)); + assert_eq!(at_limit.chars().count(), crate::MAX_FORMULA_LEN - 1); + assert!(parse(&at_limit).is_ok()); + assert!(parse(&format!("={at_limit}")).is_ok(), "the leading = does not count"); + let over = format!("{at_limit}+1"); + let err = parse(&over).unwrap_err(); + assert!(err.msg.contains("8192"), "{}", err.msg); + // Characters, not bytes: 8,000 two-byte characters in a text literal are within the limit. + let text = format!("\"{}\"", "é".repeat(8_000)); + assert!(text.len() > crate::MAX_FORMULA_LEN); + assert!(parse(&text).is_ok()); +} diff --git a/crates/xlsx/src/tests/malformed.rs b/crates/xlsx/src/tests/malformed.rs index 5e47c1f4b..d029acaa4 100644 --- a/crates/xlsx/src/tests/malformed.rs +++ b/crates/xlsx/src/tests/malformed.rs @@ -227,3 +227,32 @@ fn hyperlinks_past_the_cell_cap_are_reported() { assert_eq!(wb.sheet(0).unwrap().hyperlinks.len(), 3); assert!(rep.warnings.is_empty(), "{:?}", 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] +fn hostile_formula_text_does_not_overflow_the_stack() { + let percents = format!("1{}", "%".repeat(1_000_000)); + let chain = format!("1{}", "+1".repeat(100_000)); + let body = format!(r#"{percents}3{chain}4"#); + let names = format!(r#"{percents}"#); + let bytes = minimal(&body, &[], "", &names); + let handle = std::thread::Builder::new() + .stack_size(1024 * 1024) + .spawn(move || { + let (wb, _) = read_xlsx(&bytes).expect("readable"); + let s = wb.sheet(0).expect("sheet"); + let a1 = s.cell(gridcraft_core::CellRef::new(0, 0)).and_then(|c| c.formula.clone()).expect("formula kept"); + let b1 = s.cell(gridcraft_core::CellRef::new(0, 1)).and_then(|c| c.formula.clone()).expect("formula kept"); + let kept = (a1.expr.is_none(), b1.expr.is_none(), a1.text.len(), s.value(gridcraft_core::CellRef::new(0, 0))); + assert_eq!(wb.names.first().map(|n| n.formula.len()), Some(1_000_001)); + let out = write_xlsx(&wb).expect("writable"); + drop(wb); + (kept, !out.is_empty()) + }) + .expect("spawn"); + let ((a_unparsed, b_unparsed, a_len, a_value), written) = handle.join().expect("no stack overflow"); + assert!(a_unparsed && b_unparsed && written); + assert_eq!(a_len, 1_000_001); + assert_eq!(a_value, gridcraft_core::Value::Number(3.0)); +} diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 988472414..158149fb3 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -1,6 +1,6 @@ # File-format parity -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** trivial (XLSX chart sheets are kept on save (#727); previously minor: CSV save keeps the separator it was opened with) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-14 · **Change:** trivial (formulas over Excel's 8,192 characters, and `%` or call chains past the nesting limit, are parse errors instead of stack overflows when a file is opened; previously trivial: XLSX chart sheets are kept on save (#727)) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Every format Excel opens or saves, with GridCraft's read and write support, fidelity and tests. Part of [target-app-parity.md](target-app-parity.md). **File formats: ~50% ready, 80–120 h** @@ -85,6 +85,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here. | Date | Change | Summary | |---|---|---| +| 2026-10-14 | trivial | Formula parser: text longer than Excel's 8,192 characters, and postfix chains (`1%%%…`, `f(1)(1)…`) past the 256-level nesting limit, are parse errors (the cell keeps its formula text and cached value) instead of trees deep enough to overflow the stack when a hostile file is opened, saved or dropped | | 2026-10-12 | trivial | XLSX chart sheets are kept on save (#727) | | 2026-10-12 | trivial | CSV and TSV export is capped at 512 MiB and refuses a larger used range with an error (#528) | | 2026-10-12 | minor | `.xls` import reads the calculation settings (CalcMode, CalcIter, CalcCount, CalcDelta, CalcPrecision), so circular models calculate instead of showing `#CIRC!` (#724) | diff --git a/docs/function-parity.md b/docs/function-parity.md index 760ffd6ac..274800699 100644 --- a/docs/function-parity.md +++ b/docs/function-parity.md @@ -1,6 +1,6 @@ # Function parity: worksheet functions and formulas -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (USDOLLAR added as a compatibility function; MIRR, COUNTIF and INDIRECT edge cases match Excel 365) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-13 · **Change:** minor (clearing large sparse ranges recalculates every changed cell's dependents, #1032) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Excel's worksheet functions, category by category, against GridCraft's function library. Part of [target-app-parity.md](target-app-parity.md); gaps feed [gaps.md](gaps.md). @@ -76,6 +76,7 @@ PIVOTBY, REGEXTEST…) are all present. | Structured references (`Table1[Col]`, `[#Totals]`, `@`; the unbracketed `[[#This Row],Col]` some writers emit) | done | | Defined names (workbook and sheet scope), Create from Selection | done | | Dynamic arrays, spill, `#SPILL!`, spill operator `A1#` | done; stale-spill bugs (#133, #139; PRs #148, #157) | +| Recalculation after clearing large sparse ranges | every stored cell changed by Clear is reported to recalculation, without a million-coordinate cutoff; removed formula spills are cleared, and removing empty merges still unblocks spills (#1032) | | Implicit intersection `@`, legacy CSE array formulas | done; legacy array range fill (PR #156) | | Trim references `A1:.E10` (trailing), `A1.:E10` (leading), `A1.:.E10` (both), also on whole rows/columns (`A:.A`) | done; evaluated as `TRIMRANGE(range, code, code)`, adjusted on insert/delete; files store `_xlfn.TRIMRANGE(range,code,code)` (Excel's own file spelling unconfirmed in public docs), `_xlfn._TRO_ALL/_TRO_LEADING/_TRO_TRAILING(...)` read too | | LET, LAMBDA, named LAMBDAs, ISOMITTED, MAP/REDUCE/SCAN/BYROW/BYCOL/MAKEARRAY | done | @@ -108,6 +109,7 @@ workbooks; results checked in as data, not Excel files) and fixing what it finds | Date | Change | Summary | |---|---|---| +| 2026-10-13 | minor | Clear reports changed stored cells sparsely, including those beyond the first million coordinates; dependent formulas and removed spills update, while clearing empty merges still unblocks spills (#1032) | | 2026-10-12 | minor | USDOLLAR added as a Compatibility function (not on Microsoft's list, so the counts don't change); MIRR with no positive flow is -1, COUNTIF's bare `"<="` / `">="` skip empty strings, INDIRECT with `a1` = FALSE gives `#REF!` for A1-style text (#738) | | 2026-10-11 | minor | TEXT / number formats fixed against Excel-cached results: fraction literals and zero fractions, `0` denominators zero padded, `?` pads the thousands separator, booleans use the text section (226 mismatches → 0 in 352 TEXT cells) | | 2026-10-11 | minor | Iterative calculation: circular references iterate up to Maximum Iterations / Maximum Change when enabled |