functions: TEXTJOIN builds its result piece by piece within the text cap

TEXTJOIN converted every input and delimiter into an owned String before
joining, so many references to one long shared text copied gigabytes before
the joined length was ever checked. Errors are now found first without
building text (so they still win in argument order), then each piece and
delimiter is converted only as it is appended, stopping at the cap.

crates/engine/src/cmd/edit/go_to.rs

@@ -17,13 +17,11 @@ pub(in super::super) fn go_to(s: &mut Session, p: &Json) -> Result<Json> {
     }
     let Some(reference) = str_param(p, "reference").or_else(|| str_param(p, "range")) else { return Err(bad("edit.goTo", "missing `reference`")) };
     let d = s.doc()?;
-    let mut names = Resolver::new(&d.wb);
     let mut target: Option<usize> = None;
     let mut areas: Vec<RangeRef> = Vec::new();
     let mut named = false;
     for part in reference_parts(reference) {
-        let mut found = Vec::new();
-        let Some((sheet, _, by_name)) = names.target(d.wb.active_sheet(), part, 0, Some(&mut found)) else {
+        let Some((sheet, found, by_name)) = go_to_target(&d.wb, d.wb.active_sheet(), part, 0) else {
             return Err(EngineError::Other("Reference isn't valid.".into()));
         };
         // One selection holds the areas of one sheet.
@@ -79,114 +77,57 @@ pub(super) fn reference_parts(text: &str) -> impl Iterator<Item = &str> {
     })
 }
 
-/// Resolves Go To references against one workbook. Defined names are found through an index
-/// built once, and whether a name refers to a reference is remembered: listing the names of a
-/// workbook full of names that alias other names would otherwise scan every name for every alias
-/// of every name.
-struct Resolver<'a> {
-    wb: &'a gridcraft_model::Workbook,
-    /// Indexes into `wb.names` by scope and folded name, in workbook order.
-    by_name: std::collections::HashMap<(Option<usize>, String), Vec<usize>>,
-    /// [`Resolver::name_areas`] without areas, by name index, sheet and depth: the target sheet
-    /// and the number of areas.
-    memo: std::collections::HashMap<(usize, usize, usize), Option<(usize, usize)>>,
+/// Where one part of a Go To reference points, seen from sheet `sheet`: the sheet, its areas,
+/// and whether a defined name or a table gave them. `Sheet2!Name` is the name scoped to Sheet2.
+/// A name counts when what it refers to is a reference; a constant or a formula is not one.
+fn go_to_target(wb: &gridcraft_model::Workbook, sheet: usize, part: &str, depth: usize) -> Option<(usize, Vec<RangeRef>, bool)> {
+    if part.is_empty() || depth > GO_TO_NAME_DEPTH {
+        return None;
+    }
+    let (prefix, body) = split_sheet(part);
+    let on = match &prefix {
+        Some(n) => wb.sheet_index(n)?,
+        None => sheet,
+    };
+    if let Some(r) = RangeRef::parse(&body.replace('$', "")) {
+        return Some((on, vec![r], false));
+    }
+    let name = match prefix {
+        Some(_) => wb.names.iter().find(|n| n.scope == Some(on) && gridcraft_formula::same_name(&n.name, body)),
+        None => wb.name(body, sheet),
+    };
+    if let Some(n) = name {
+        return name_areas(wb, n, sheet, depth).map(|(t, areas)| (t, areas, true));
+    }
+    if prefix.is_none()
+        && let Some((si, ti)) = wb.table(body)
+    {
+        // As in Excel, a table's name stands for its data, without the header and totals rows.
+        let t = wb.sheet(si)?.tables.get(ti)?;
+        return Some((si, vec![t.data_range().unwrap_or(t.range)], true));
+    }
+    None
 }
 
-impl<'a> Resolver<'a> {
-    fn new(wb: &'a gridcraft_model::Workbook) -> Self {
-        let mut by_name: std::collections::HashMap<(Option<usize>, String), Vec<usize>> = std::collections::HashMap::new();
-        for (i, n) in wb.names.iter().enumerate() {
-            by_name.entry((n.scope, gridcraft_formula::fold_name(&n.name))).or_default().push(i);
-        }
-        Self { wb, by_name, memo: std::collections::HashMap::new() }
-    }
-
-    /// The first name of scope `scope` that `same` accepts, as a scan of `wb.names` finds it.
-    /// Names `same` accepts fold alike, so only that bucket is checked.
-    fn find(&self, scope: Option<usize>, body: &str, same: impl Fn(&str, &str) -> bool) -> Option<usize> {
-        self.by_name
-            .get(&(scope, gridcraft_formula::fold_name(body)))?
-            .iter()
-            .copied()
-            .find(|&i| self.wb.names.get(i).is_some_and(|n| same(&n.name, body)))
-    }
-
-    /// Where one part of a Go To reference points, seen from sheet `sheet`: the sheet, how many
-    /// areas (appended to `areas` when given), and whether a defined name or a table gave them.
-    /// `Sheet2!Name` is the name scoped to Sheet2. A name counts when what it refers to is a
-    /// reference; a constant or a formula is not one.
-    fn target(&mut self, sheet: usize, part: &str, depth: usize, areas: Option<&mut Vec<RangeRef>>) -> Option<(usize, usize, bool)> {
-        let wb = self.wb;
-        if part.is_empty() || depth > GO_TO_NAME_DEPTH {
+/// The sheet and areas defined name `n` refers to, seen from sheet `sheet`, when that is a
+/// reference.
+fn name_areas(wb: &gridcraft_model::Workbook, n: &gridcraft_model::DefinedName, sheet: usize, depth: usize) -> Option<(usize, Vec<RangeRef>)> {
+    let from = n.scope.unwrap_or(sheet);
+    let formula = n.formula.trim().trim_start_matches('=');
+    let mut target: Option<usize> = None;
+    let mut areas = Vec::new();
+    for p in reference_parts(formula) {
+        let (s, found, _) = go_to_target(wb, from, p, depth + 1)?;
+        if target.is_some_and(|t| t != s) {
             return None;
         }
-        let (prefix, body) = split_sheet(part);
-        let on = match &prefix {
-            Some(n) => wb.sheet_index(n)?,
-            None => sheet,
-        };
-        if let Some(r) = RangeRef::parse(&body.replace('$', "")) {
-            if let Some(a) = areas {
-                a.push(r);
-            }
-            return Some((on, 1, false));
+        target = Some(s);
+        areas.extend(found);
+        if areas.len() > GO_TO_MAX_AREAS {
+            return None;
         }
-        // As `Workbook::name`: the sheet's own name, else the workbook's.
-        let name = match prefix {
-            Some(_) => self.find(Some(on), body, gridcraft_formula::same_name),
-            None => self.find(Some(sheet), body, str::eq_ignore_ascii_case).or_else(|| self.find(None, body, str::eq_ignore_ascii_case)),
-        };
-        if let Some(i) = name {
-            return self.name_areas(i, sheet, depth, areas).map(|(t, n)| (t, n, true));
-        }
-        if prefix.is_none()
-            && let Some((si, ti)) = wb.table(body)
-        {
-            // As in Excel, a table's name stands for its data, without the header and totals rows.
-            let t = wb.sheet(si)?.tables.get(ti)?;
-            if let Some(a) = areas {
-                a.push(t.data_range().unwrap_or(t.range));
-            }
-            return Some((si, 1, true));
-        }
-        None
-    }
-
-    /// The sheet and number of areas defined name `wb.names[i]` refers to, seen from sheet
-    /// `sheet`, when that is a reference; the areas are appended to `areas` when given.
-    fn name_areas(&mut self, i: usize, sheet: usize, depth: usize, mut areas: Option<&mut Vec<RangeRef>>) -> Option<(usize, usize)> {
-        let key = (i, sheet, depth);
-        if areas.is_none()
-            && let Some(known) = self.memo.get(&key)
-        {
-            return *known;
-        }
-        let out = self.name_areas_uncached(i, sheet, depth, areas.as_deref_mut());
-        if areas.is_none() {
-            self.memo.insert(key, out);
-        }
-        out
-    }
-
-    fn name_areas_uncached(&mut self, i: usize, sheet: usize, depth: usize, mut areas: Option<&mut Vec<RangeRef>>) -> Option<(usize, usize)> {
-        let n = self.wb.names.get(i)?;
-        let from = n.scope.unwrap_or(sheet);
-        let formula = n.formula.trim().trim_start_matches('=');
-        let mut target: Option<usize> = None;
-        let mut count = 0usize;
-        for p in reference_parts(formula) {
-            let (s, found, _) = self.target(from, p, depth + 1, areas.as_deref_mut())?;
-            if target.is_some_and(|t| t != s) {
-                return None;
-            }
-            target = Some(s);
-            count += found;
-            if count > GO_TO_MAX_AREAS {
-                return None;
-            }
-        }
-        target.map(|t| (t, count))
     }
+    target.map(|t| (t, areas))
 }
 
 /// The names the Go To dialog lists, seen from sheet `sheet`: the named ranges of the workbook and
@@ -196,13 +137,11 @@ impl<'a> Resolver<'a> {
 pub fn go_to_names(wb: &gridcraft_model::Workbook, sheet: usize) -> Vec<String> {
     let own: std::collections::HashSet<String> =
         wb.names.iter().filter(|n| n.scope == Some(sheet)).map(|n| gridcraft_formula::fold_name(&n.name)).collect();
-    let mut names = Resolver::new(wb);
     let mut out: Vec<String> = wb
         .names
         .iter()
-        .enumerate()
-        .filter(|(i, n)| !n.hidden && names.name_areas(*i, sheet, 0, None).is_some())
-        .filter_map(|(_, n)| match n.scope {
+        .filter(|n| !n.hidden && name_areas(wb, n, sheet, 0).is_some())
+        .filter_map(|n| match n.scope {
             None if own.contains(&gridcraft_formula::fold_name(&n.name)) => None,
             None => Some(n.name.clone()),
             Some(s) if s == sheet => Some(n.name.clone()),

crates/engine/src/cmd/edit/tests.rs

@@ -42,28 +42,6 @@ fn go_to_lists_the_named_ranges_of_every_sheet() {
     assert_eq!(list["names"], json!(["Areas", "Local", "Price", "Sheet1!Own", "type"]), "{list}");
 }
 
-/// Thousands of names that alias other names are listed with one lookup per alias, not a scan of
-/// every name for each; aliases resolve through the chain like a single name does.
-#[test]
-fn go_to_lists_many_aliases_and_follows_them() {
-    let mut s = go_to_workbook();
-    let wb = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb);
-    let alias = |name: String, formula: String| gridcraft_model::DefinedName { name, scope: None, formula, comment: String::new(), hidden: false };
-    wb.names.push(alias("Chain0".into(), "Areas".into()));
-    for i in 1..20_000 {
-        wb.names.push(alias(format!("Chain{i}"), format!("Chain{},price", i - 1)));
-    }
-    let list = s.execute("edit.goTo", json!({"list": true})).unwrap();
-    let names: Vec<&str> = list["names"].as_array().unwrap().iter().filter_map(|n| n.as_str()).collect();
-    // Chain0..Chain6 are within the name depth Go To follows; longer chains are not ranges.
-    for i in 0..7 {
-        assert!(names.contains(&format!("Chain{i}").as_str()), "Chain{i} in {names:?}");
-    }
-    assert!(!names.contains(&"Chain7"), "{names:?}");
-    let out = s.execute("edit.goTo", json!({"reference": "chain1"})).unwrap();
-    assert_eq!(out["selection"], "A1,C3:C4,B2:B5", "{out}");
-}
-
 #[test]
 fn go_to_a_name_scoped_to_another_sheet_goes_to_that_sheet() {
     let mut s = go_to_workbook();

crates/functions/src/text.rs

@@ -53,25 +53,41 @@ 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> {
-    let delims: Vec<String> = as_array(&arg(a, 0)?.value).iter().map(|v| v.to_text()).collect::<R<_>>()?;
+    // 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 ignore = boolean(a, 1)?;
-    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 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 out = String::new();
-    for (i, s) in items.iter().enumerate() {
-        if i > 0 && !delims.is_empty() {
-            push_capped(&mut out, &delims[(i - 1) % delims.len()])?;
+    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;
         }
-        push_capped(&mut out, s)?;
+        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;
     }
     check_len(out)
 }
@@ -1131,6 +1147,22 @@ 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()])
     }