Compare commits

..
Author SHA1 Message Date
Dylan M. Taylor 8ba66ba689 address review: stop the descriptive builder once the grid is too large 2026-11-04 00:51:51 +00:00
Dylan M. Taylor 764c5c1a46 Analysis ToolPak: stop building a report once it outgrows the output limit
Report builders filled their whole output map before write_report
checked its size, so Descriptive Statistics grouped by rows over a
tall, nearly empty column allocated about 16 entries for each of up
to a million variables before refusing the report.

Grid now tracks its extent as values are stored and keeps nothing
once the report's rows x columns pass MAX_OUTPUT_CELLS; write_report
refuses such a report. extent() no longer rescans every entry.
2026-11-02 20:27:53 -05:00
2 changed files with 56 additions and 50 deletions
+42 -4
View File
@@ -272,11 +272,25 @@ struct Grid {
cells: BTreeMap<(u32, u32), Value>,
/// Cells shown as percentages.
pct: Vec<(u32, u32)>,
/// Rows and columns spanned by `cells`.
rows: u32,
cols: u32,
/// A cell was dropped because the report outgrew [`MAX_OUTPUT_CELLS`]: the report is refused.
too_large: bool,
}
impl Grid {
/// Stores a value. Past [`MAX_OUTPUT_CELLS`] (the most a report writes) nothing more is kept,
/// so a builder that would produce millions of entries stops allocating them, and
/// [`write_report`] refuses the report.
fn put(&mut self, r: u32, c: u32, v: Value) {
if !v.is_empty() {
let (rows, cols) = (self.rows.max(r.saturating_add(1)), self.cols.max(c.saturating_add(1)));
if self.too_large || u64::from(rows) * u64::from(cols) > MAX_OUTPUT_CELLS {
self.too_large = true;
return;
}
(self.rows, self.cols) = (rows, cols);
self.cells.insert((r, c), v);
}
}
@@ -296,9 +310,7 @@ impl Grid {
}
/// (rows, cols) spanned.
fn extent(&self) -> (u32, u32) {
let rows = self.cells.keys().map(|(r, _)| r + 1).max().unwrap_or(0);
let cols = self.cells.keys().map(|(_, c)| c + 1).max().unwrap_or(0);
(rows, cols)
(self.rows, self.cols)
}
}
@@ -320,6 +332,10 @@ fn descriptive(s: &Session, p: &Json) -> Result<Grid> {
}
let mut g = Grid::default();
for (i, v) in vars.iter().enumerate() {
// The report is refused once the grid is too large: the rest would only build discarded labels.
if g.too_large {
break;
}
let x = numbers(&v.vals, true)?;
let c = idx(i).saturating_mul(2);
g.txt(0, c, &v.name);
@@ -1749,7 +1765,7 @@ fn sampling(s: &Session, p: &Json, seed: Option<u64>) -> Result<Grid> {
fn write_report(s: &mut Session, p: &Json, tool: &str, g: Grid) -> Result<Json> {
let (rows, cols) = g.extent();
let (rows, cols) = (rows.max(1), cols.max(1));
if u64::from(rows) * u64::from(cols) > MAX_OUTPUT_CELLS {
if g.too_large || u64::from(rows) * u64::from(cols) > MAX_OUTPUT_CELLS {
return Err(bad(CMD, "the report is too large"));
}
let d = s.doc()?;
@@ -1914,6 +1930,28 @@ mod tests {
x
}
#[test]
fn a_report_stops_growing_past_the_output_limit() {
// Entries past the limit are not kept, and the report is refused.
let mut g = super::Grid::default();
g.num(1999, 1999, 1.0);
assert!(!g.too_large);
g.num(0, 2000, 1.0);
assert!(g.too_large);
assert_eq!(g.cells.len(), 1);
assert_eq!(g.extent(), (2000, 2000));
// Row-grouped statistics of a tall, almost empty column: one variable per row would make
// millions of report entries; the report is refused instead of built.
let mut s = s();
s.execute("range.setValues", json!({"range": "A1", "values": [[1]]})).unwrap();
s.execute("range.setValues", json!({"range": "A1000000", "values": [[2]]})).unwrap();
let err = s
.execute("data.analysisToolPak", json!({"tool": "descriptive", "range": "A1:A1000000", "groupedBy": "rows", "output": "C1"}))
.expect_err("report too large");
assert!(err.to_string().contains("too large"), "{err}");
assert_eq!(v(&s, "C1"), Value::Empty);
}
#[test]
fn forecast_standard_errors_match_the_full_window_rescan() {
// #1043: the running sum must give what re-adding every window gave, gaps included.
+14 -46
View File
@@ -53,41 +53,25 @@ fn concat(a: &[Arg], _c: &mut dyn Ctx) -> R<Value> {
check_len(out)
}
/// The error `v.to_text()` would give, found without building any text.
fn text_error(v: &Value) -> Option<CellError> {
match v {
Value::Error(e) => Some(*e),
Value::Array(a) => a.data.first().and_then(text_error),
_ => None,
}
}
fn textjoin(a: &[Arg], _c: &mut dyn Ctx) -> R<Value> {
// Errors are found first, in argument order, so they win as before; text is then built one
// piece at a time into the capped output, so many references to one long text (or a long
// delimiter list) stop at the cap instead of first copying every piece.
let delims = as_array(&arg(a, 0)?.value);
if let Some(e) = delims.iter().find_map(text_error) {
return Err(e);
}
let delims: Vec<String> = as_array(&arg(a, 0)?.value).iter().map(|v| v.to_text()).collect::<R<_>>()?;
let ignore = boolean(a, 1)?;
let items: Vec<_> = a.iter().skip(2).map(|x| as_array(&x.value)).collect();
if let Some(e) = items.iter().flat_map(|x| x.iter()).find_map(text_error) {
return Err(e);
let mut items = Vec::new();
for x in a.iter().skip(2) {
for v in as_array(&x.value).iter() {
let s = v.to_text()?;
if ignore && s.is_empty() {
continue;
}
items.push(s);
}
}
let mut out = String::new();
let mut joined = 0usize;
for v in items.iter().flat_map(|x| x.iter()) {
let s = v.to_text()?;
if ignore && s.is_empty() {
continue;
for (i, s) in items.iter().enumerate() {
if i > 0 && !delims.is_empty() {
push_capped(&mut out, &delims[(i - 1) % delims.len()])?;
}
if joined > 0 && !delims.data.is_empty() {
let d = delims.data.get((joined - 1) % delims.data.len()).map(Value::to_text).transpose()?.unwrap_or_default();
push_capped(&mut out, &d)?;
}
push_capped(&mut out, &s)?;
joined += 1;
push_capped(&mut out, s)?;
}
check_len(out)
}
@@ -1147,22 +1131,6 @@ mod tests {
assert_eq!(rows_of(&ev("CONCATENATE", vec![av(row_t(&["a", "b"])), t("x")])), vec![vec![tv("ax"), tv("bx")]]);
}
/// Security: TEXTJOIN builds its result piece by piece and stops at the text cap, rather
/// than first copying every input (100,000 references to one 32,767-character text were
/// 3 GB of copies). Errors among the inputs still win, in argument order.
#[test]
fn textjoin_stops_at_the_text_cap() {
use gridcraft_core::Array;
let long = tv(&"x".repeat(32_767));
let many = || av(Array::filled(100_000, 1, long.clone()).into());
is_err(ev("TEXTJOIN", vec![t(","), b(true), many()]), CellError::Value);
is_err(ev("TEXTJOIN", vec![av(Array::filled(1, 100_000, long.clone()).into()), b(false), t("a"), t("b")]), CellError::Value);
is_err(ev("TEXTJOIN", vec![t(","), b(true), many(), e(CellError::Div0)]), CellError::Div0);
is_err(ev("TEXTJOIN", vec![av(arr(vec![vec![tv(","), Value::Error(CellError::NA)]])), b(true), t("a")]), CellError::NA);
// Delimiters cycle and are converted only where used.
is_text(ev("TEXTJOIN", vec![av(arr(vec![vec![nv(1.0), tv(";")]])), b(false), t("a"), t("b"), t("c"), t("d")]), "a1b;c1d");
}
fn row_t(xs: &[&str]) -> Value {
arr(vec![xs.iter().map(|x| tv(x)).collect()])
}