mirror of
https://github.com/storytold/gridcraft.git
synced 2026-10-29 21:26:48 -06:00
Formula parser: Excel's 8,192-character limit and postfix nesting (never crash) (#1330)
A hostile file could abort any process that opens it: a cell formula of a million `%` (`1%%%…`) or a 200,000-term sum parsed into trees deep enough to overflow the stack when printed, saved, evaluated or dropped, even in release builds. Postfix operators (`%`, invoking a result) built their nesting in a loop that the 256-level limit did not see. - `parse` refuses formula text longer than Excel's 8,192 characters (`MAX_FORMULA_LEN`, counted in characters, the leading `=` excluded). This also bounds the depth of the left-deep trees long operator chains build. - Each postfix `%` and each call of a result counts towards the nesting limit. - A file's formula that is refused keeps its text and cached value, as other unparsable formulas do. Regression tests: formula (small-stack thread: long `%`, invoke and operator chains are errors; the limit counts characters, not bytes) and xlsx (`hostile_formula_text_does_not_overflow_the_stack`: open, save and drop a workbook with such formulas and a defined name on a 1 MiB stack). Both abort with a stack overflow without the fix. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
79e85c1b80
commit
82ee02aac4
@@ -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)]
|
||||
|
||||
@@ -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<Expr, ParseError> {
|
||||
@@ -25,6 +30,9 @@ pub fn parse(src: &str) -> Result<Expr, ParseError> {
|
||||
|
||||
pub(crate) fn parse_syntax(src: &str, locale: crate::FormulaLocale) -> Result<Expr, ParseError> {
|
||||
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);
|
||||
|
||||
@@ -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());
|
||||
}
|
||||
|
||||
@@ -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#"<sheetData><row r="1"><c r="A1"><f>{percents}</f><v>3</v></c><c r="B1"><f>{chain}</f><v>4</v></c></row></sheetData>"#);
|
||||
let names = format!(r#"<definedNames><definedName name="Deep">{percents}</definedName></definedNames>"#);
|
||||
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));
|
||||
}
|
||||
|
||||
@@ -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) |
|
||||
|
||||
Reference in New Issue
Block a user