Compare commits

..
Author SHA1 Message Date
Dylan M. Taylor cf7e632ebb address review: stop TEXT rendering past the cell limit and count UTF-16 units 2026-11-04 01:12:39 +00:00
Dylan M. Taylor 7c851d7906 numfmt: TEXT returns #VALUE! when its result passes a cell's 32,767 characters
A text section repeats its input once per @ placeholder, and TEXT handed
back whatever that built, so nested TEXT calls could multiply a long
text until memory ran out. Results past the cell text limit are now
#VALUE!.
2026-11-03 12:03:51 +00:00
5 changed files with 61 additions and 45 deletions
+10 -31
View File
@@ -61,24 +61,6 @@ enum Key {
const MAX_CONSOLIDATE_CELLS: u64 = 5_000_000;
/// The cells a consolidated table writes: every row key meets every column key, plus a label for
/// each key on the sides that have labels.
fn output_cells(rows: usize, cols: usize, top: bool, left: bool) -> u64 {
let (r, c) = (rows as u64, cols as u64);
r.saturating_mul(c).saturating_add(if top { c } else { 0 }).saturating_add(if left { r } else { 0 })
}
#[cfg(test)]
#[test]
fn the_output_limit_counts_label_cells() {
// A million row keys and four column keys are 4 million cells, 5 million with the row labels.
assert_eq!(output_cells(1_000_000, 4, false, false), 4_000_000);
assert_eq!(output_cells(1_000_000, 4, false, true), 5_000_000);
assert!(output_cells(1_000_000, 5, false, true) > MAX_CONSOLIDATE_CELLS);
assert_eq!(output_cells(3, 2, true, true), 6 + 2 + 3);
assert_eq!(output_cells(usize::MAX, usize::MAX, true, true), u64::MAX);
}
pub(super) fn consolidate(s: &mut Session, p: &Json) -> Result<Json> {
const CMD: &str = "data.consolidate";
let func = str_param(p, "function").unwrap_or("sum").to_string();
@@ -107,20 +89,20 @@ pub(super) fn consolidate(s: &mut Session, p: &Json) -> Result<Json> {
}
None => (d.wb.active_sheet(), d.selection.active),
};
// Keys in first-seen order, each with its index: a lookup is one probe, not a scan.
let mut row_keys: Vec<Key> = Vec::new();
let mut col_keys: Vec<Key> = Vec::new();
let mut row_index: HashMap<Key, usize> = HashMap::new();
let mut col_index: HashMap<Key, usize> = HashMap::new();
let mut row_label: HashMap<Key, String> = HashMap::new();
let mut col_label: HashMap<Key, String> = HashMap::new();
let mut aggs: HashMap<(usize, usize), Agg> = HashMap::new();
let mut total: u64 = 0;
let index_of = |keys: &mut Vec<Key>, index: &mut HashMap<Key, usize>, k: Key| -> usize {
*index.entry(k).or_insert_with_key(|k| {
keys.push(k.clone());
keys.len() - 1
})
let index_of = |keys: &mut Vec<Key>, k: Key| -> usize {
match keys.iter().position(|x| *x == k) {
Some(i) => i,
None => {
keys.push(k);
keys.len() - 1
}
}
};
for t in &srcs {
let (si, r) = parse_range(&d.wb, t, CMD)?;
@@ -149,7 +131,7 @@ pub(super) fn consolidate(s: &mut Session, p: &Json) -> Result<Json> {
} else {
Key::Pos(col - c0)
};
cols.push((col, index_of(&mut col_keys, &mut col_index, k)));
cols.push((col, index_of(&mut col_keys, k)));
}
for row in r0..=r.end.row {
let k = if left {
@@ -163,7 +145,7 @@ pub(super) fn consolidate(s: &mut Session, p: &Json) -> Result<Json> {
} else {
Key::Pos(row - r0)
};
let ri = index_of(&mut row_keys, &mut row_index, k);
let ri = index_of(&mut row_keys, k);
for (col, ci) in &cols {
aggs.entry((ri, *ci)).or_default().add(&sh.value(CellRef::new(row, *col)));
}
@@ -175,9 +157,6 @@ pub(super) fn consolidate(s: &mut Session, p: &Json) -> Result<Json> {
{
return Err(EngineError::Other("The consolidated data doesn't fit below and right of the destination.".into()));
}
if output_cells(row_keys.len(), col_keys.len(), top, left) > MAX_CONSOLIDATE_CELLS {
return Err(bad(CMD, "the consolidated table would be too large"));
}
let mut cells: Vec<(CellRef, Value)> = Vec::new();
if top {
for (ci, k) in col_keys.iter().enumerate() {
-13
View File
@@ -297,16 +297,3 @@ fn consolidate_by_position_and_label() {
assert!(s.execute("data.consolidate", json!({"sources": []})).is_err());
assert!(s.execute("data.consolidate", json!({"sources": ["A1"], "function": "median"})).is_err());
}
/// A hundred thousand positional rows take one key lookup each; a tall source and a wide one
/// whose keys would meet in a table of 160 million cells are refused before it is built.
#[test]
fn consolidate_keys_are_indexed_and_the_output_is_bounded() {
let mut s = s();
s.execute("range.setValues", json!({"range": "A1", "values": [[1], [2]]})).unwrap();
let r = s.execute("data.consolidate", json!({"sources": ["A1:A100000"], "destination": "C1"})).unwrap();
assert_eq!(r["rows"], 100_000);
assert_eq!(n(&s, "Sheet1!C2"), 2.0);
let r = s.execute("data.consolidate", json!({"sources": ["A1:A10000", "A1:XFC1"], "destination": "A1"}));
assert!(r.unwrap_err().to_string().contains("too large"));
}
+15 -1
View File
@@ -332,6 +332,14 @@ impl NumberFormat {
None => Formatted { text: t.to_string(), color: None, fill: None, numeric: false },
}
}
/// [`Self::format_text`]'s text, or `None` once it passes `max_units` UTF-16 code units.
fn format_text_within(&self, t: &str, max_units: usize) -> Option<String> {
match self.inner.text_section.and_then(|i| self.inner.sections.get(i)) {
Some(sec) => render::render_text_within(sec, t, max_units).map(|out| out.text),
None => (t.encode_utf16().count() <= max_units).then(|| t.to_string()),
}
}
}
/// Formats a cell value for display.
@@ -425,6 +433,10 @@ impl NumberFormat {
/// The `TEXT()` worksheet function: formats a value with a format code. Numeric text is
/// converted to a number first; other text and booleans (as `TRUE`/`FALSE`) go through the text
/// section, or are returned as is.
/// Longest text a cell holds, in UTF-16 code units (Excel's 32,767 characters, which XLSB also
/// stores that way), and so the longest `TEXT` result.
const MAX_CELL_TEXT_CHARS: usize = 32_767;
pub fn text_function(v: &Value, code: &str, sys: DateSystem) -> Result<String, CellError> {
if code.is_empty() {
return Ok(String::new());
@@ -440,7 +452,9 @@ pub fn text_function(v: &Value, code: &str, sys: DateSystem) -> Result<String, C
Value::Bool(b) => return Ok(fmt.format_text(if b { "TRUE" } else { "FALSE" }).text),
Value::Text(t) => match gridcraft_core::parse::parse_number_text(&t) {
Some(n) => n,
None => return Ok(fmt.format_text(&t).text),
// Every `@` repeats the text: a result past a cell's limit is `#VALUE!`, so nested
// TEXT calls can't multiply it further.
None => return fmt.format_text_within(&t, MAX_CELL_TEXT_CHARS).ok_or(CellError::Value),
},
Value::Array(_) => return Err(CellError::Value),
};
+29
View File
@@ -45,6 +45,35 @@ pub(crate) fn render_text(sec: &Section, text: &str) -> Out {
out
}
/// [`render_text`], but `None` once the output passes `max_units` UTF-16 code units (a cell's
/// limit), checked before each piece is appended, so an oversized result is never built.
pub(crate) fn render_text_within(sec: &Section, text: &str, max_units: usize) -> Option<Out> {
let text_units = text.encode_utf16().count();
let mut out = Out::default();
let mut units = 0usize;
for t in &sec.toks {
match t {
Tok::At => {
units = units.saturating_add(text_units);
if units > max_units {
return None;
}
out.push(text);
}
Tok::Fill(c) => out.fill(*c),
other => {
let literal = other.literal_text();
units = units.saturating_add(literal.encode_utf16().count());
if units > max_units {
return None;
}
out.push(&literal);
}
}
}
Some(out)
}
/// Renders a number through a General section (literals around the General keyword).
pub(crate) fn render_general(sec: &Section, v: f64, width: usize) -> Out {
let mut out = Out::default();
+7
View File
@@ -396,6 +396,13 @@ fn text_fn() {
assert_eq!(text_function(&Value::Number(1234.5), "$#,##0.00", S), Ok("$1,234.50".into()));
assert_eq!(text_function(&Value::from("5"), "0.00", S), Ok("5.00".into()));
assert_eq!(text_function(&Value::from("abc"), "0.00", S), Ok("abc".into()));
// Each `@` repeats the text: up to a cell's 32,767 characters, then #VALUE!.
let at = "@".repeat(255);
assert_eq!(text_function(&Value::from("x".repeat(128).as_str()), &at, S).map(|s| s.len()), Ok(128 * 255));
assert_eq!(text_function(&Value::from("x".repeat(32_000).as_str()), &at, S), Err(CellError::Value));
// The limit counts UTF-16 units: 16,384 emoji are 32,768 of them, one too many.
assert_eq!(text_function(&Value::from("\u{1F600}".repeat(16_383).as_str()), "@", S).map(|s| s.chars().count()), Ok(16_383));
assert_eq!(text_function(&Value::from("\u{1F600}".repeat(16_384).as_str()), "@", S), Err(CellError::Value));
assert_eq!(text_function(&Value::Error(CellError::NA), "0", S), Err(CellError::NA));
assert_eq!(text_function(&Value::Number(46302.0), "yyyy", S), Ok("2026".into()));
assert_eq!(text_function(&Value::Number(0.5), "General", S), Ok("0.5".into()));