From 03a16ddea6d2ca99a55f6d571e2267e3a219ac24 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 4 Nov 2026 04:56:35 +0000 Subject: [PATCH] fix(forms): AFSimple_Calculate folds field values as it reads them, within an allowance --- crates/forms/src/af/calc.rs | 28 ++++++++++++++++------------ crates/forms/src/af/tests.rs | 20 ++++++++++++++++++++ 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/crates/forms/src/af/calc.rs b/crates/forms/src/af/calc.rs index d29956cbc2..3187c4effd 100644 --- a/crates/forms/src/af/calc.rs +++ b/crates/forms/src/af/calc.rs @@ -525,6 +525,9 @@ pub(super) fn eval_notation(src: &str, value_of: &dyn Fn(&str) -> f64) -> Option (i == toks.len() && v.is_finite()).then_some(v) } +/// Most field values one `AFSimple_Calculate` list may expand to (a form has some thousands). +const MAX_EXPANDED_VALUES: usize = 1 << 20; + /// The Calculate event: the new value, given the current values of the other fields /// (`values(name)` returns every field named `name` or below it, for groups). pub fn calculate(c: &Calculate, values: &dyn Fn(&str) -> Vec) -> Option { @@ -532,20 +535,21 @@ pub fn calculate(c: &Calculate, values: &dyn Fn(&str) -> Vec) -> Option< match c { Calculate::None => None, Calculate::Simple { op, fields } => { - let xs: Vec = fields.iter().flat_map(|f| values(f)).map(|v| num(&v)).collect(); + // The values are folded as they are read, never collected: a group name listed many + // times would otherwise be held once per listing. A list that expands past + // MAX_EXPANDED_VALUES is not calculated. + let mut n = 0usize; + let xs = fields.iter().flat_map(|f| values(f)).map(|v| num(&v)).inspect(|_| n += 1).take(MAX_EXPANDED_VALUES + 1); let v = match op { - CalcOp::Sum => xs.iter().sum(), - CalcOp::Product => xs.iter().product(), - CalcOp::Average => { - if xs.is_empty() { - 0.0 - } else { - xs.iter().sum::() / xs.len() as f64 - } - } - CalcOp::Minimum => xs.iter().copied().fold(f64::INFINITY, f64::min), - CalcOp::Maximum => xs.iter().copied().fold(f64::NEG_INFINITY, f64::max), + CalcOp::Sum | CalcOp::Average => xs.sum(), + CalcOp::Product => xs.product(), + CalcOp::Minimum => xs.fold(f64::INFINITY, f64::min), + CalcOp::Maximum => xs.fold(f64::NEG_INFINITY, f64::max), }; + if n > MAX_EXPANDED_VALUES { + return None; + } + let v = if *op == CalcOp::Average { if n == 0 { 0.0 } else { v / n as f64 } } else { v }; Some(if v.is_finite() { js_number(v) } else { "0".into() }) } Calculate::Notation(expr) => { diff --git a/crates/forms/src/af/tests.rs b/crates/forms/src/af/tests.rs index 79c5c7be02..7c5021b63b 100644 --- a/crates/forms/src/af/tests.rs +++ b/crates/forms/src/af/tests.rs @@ -383,3 +383,23 @@ fn validation_and_calculation() { assert_eq!(calculate(&Calculate::Notation("(a + b) * 2 - Unit\\ Price / 4".into()), &vals), Some("10".into())); assert_eq!(calculate(&Calculate::Notation("a +".into()), &vals), None); } + +#[test] +fn simple_calculations_fold_a_repeated_group_without_collecting_it() { + let group = |_: &str| vec!["2".to_string(); 3]; + let list = |op, n: usize| Calculate::Simple { op, fields: vec!["g".to_string(); n] }; + assert_eq!(calculate(&list(CalcOp::Sum, 4), &group).as_deref(), Some("24")); + assert_eq!(calculate(&list(CalcOp::Average, 4), &group).as_deref(), Some("2")); + assert_eq!(calculate(&list(CalcOp::Product, 2), &group).as_deref(), Some("64")); + assert_eq!(calculate(&list(CalcOp::Maximum, 2), &group).as_deref(), Some("2")); + assert_eq!(calculate(&list(CalcOp::Sum, 0), &group).as_deref(), Some("0")); +} + +#[test] +fn a_simple_calculation_that_expands_past_the_allowance_is_not_calculated() { + let thousand = |_: &str| vec!["1".to_string(); 1000]; + let within = Calculate::Simple { op: CalcOp::Sum, fields: vec!["g".to_string(); 1000] }; + let past = Calculate::Simple { op: CalcOp::Sum, fields: vec!["g".to_string(); 2000] }; + assert_eq!(calculate(&within, &thousand).as_deref(), Some("1000000")); + assert_eq!(calculate(&past, &thousand), None); +}