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
8 changed files with 56 additions and 44 deletions
+1 -39
View File
@@ -8,13 +8,8 @@ use gridcraft_model::{Sheet, Workbook};
use gridcraft_numfmt::{Formatted, NumberFormat, format_value};
/// Parsed number formats are cached by code.
pub(crate) fn parsed(code: &str) -> NumberFormat {
fn parsed(code: &str) -> NumberFormat {
static CACHE: std::sync::LazyLock<Mutex<HashMap<String, NumberFormat>>> = std::sync::LazyLock::new(|| Mutex::new(HashMap::new()));
// A code of more bytes than 4 × the character limit can't be within it: it shows as General
// without being hashed or scanned again for every cell that shares it.
if code.len() > 4 * gridcraft_numfmt::MAX_FORMAT_CODE_CHARS {
return NumberFormat::parse("General");
}
let lock = || CACHE.lock().unwrap_or_else(std::sync::PoisonError::into_inner);
if let Some(f) = lock().get(code) {
return f.clone();
@@ -29,12 +24,7 @@ pub(crate) fn parsed(code: &str) -> NumberFormat {
f
}
/// The parsed format of `code`; one longer than the character limit is a General format that keeps
/// the code it was made from. The engine itself formats through `parsed`, which doesn't copy it.
pub fn number_format(code: &str) -> NumberFormat {
if code.len() > 4 * gridcraft_numfmt::MAX_FORMAT_CODE_CHARS {
return NumberFormat::parse(code);
}
parsed(code)
}
@@ -236,31 +226,3 @@ pub fn stats(sheet: &Sheet, ranges: &[RangeRef]) -> Stats {
}
s
}
#[cfg(test)]
mod tests {
use super::*;
/// A format code far past Excel's 255 characters shows its values as General, for every cell
/// that shares it, without being hashed into the cache each time.
#[test]
fn an_overlong_code_formats_as_general() {
let wb = Workbook::new();
let code = format!("0.00{}", "\"x\"".repeat(400_000));
for _ in 0..1_000 {
assert_eq!(format(&Value::Number(1.5), &code, &wb).text, "1.5");
}
assert_eq!(format(&Value::Number(1.5), "0.00", &wb).text, "1.50");
}
/// The public `number_format` keeps the code of an overlong format; it is General, but not the
/// same format as a real `General`.
#[test]
fn number_format_keeps_an_overlong_code() {
let code = format!("0.00{}", "\"x\"".repeat(2_000));
let f = number_format(&code);
assert!(f.is_general());
assert_eq!(f.code(), code);
assert_ne!(f, number_format("General"));
}
}
+1 -1
View File
@@ -443,7 +443,7 @@ pub fn csv_values_bytes(wb: &Workbook) -> Result<Vec<u8>> {
let Some(sh) = wb.sheet(wb.active_sheet()) else { return Ok(Vec::new()) };
gridcraft_xlsx::write_csv(sh, wb.csv_delimiter.unwrap_or(b','), |c| match sh.value(c) {
gridcraft_core::Value::Number(n) if n.is_finite() => {
let shape = crate::display::parsed(wb.styles.get(sh.style_id(c)).num_fmt.as_str()).date_shape(n);
let shape = crate::display::number_format(wb.styles.get(sh.style_id(c)).num_fmt.as_str()).date_shape(n);
match shape.zip(gridcraft_core::date::datetime_from_serial(wb.date_system, n)) {
Some(((date, time), d)) => iso(&d, date, time),
None => n.to_string(),
+1 -1
View File
@@ -440,7 +440,7 @@ pub fn infer_input(wb: &Workbook, sheet: usize, r: RangeRef) -> NamedInput {
Some(match v {
Value::Empty => return None,
Value::Text(t) if t.is_empty() => return None,
Value::Number(_) if crate::display::parsed(wb.styles.get(sh.style_id(c)).num_fmt.as_str()).is_date() => ColType::Date,
Value::Number(_) if crate::display::number_format(wb.styles.get(sh.style_id(c)).num_fmt.as_str()).is_date() => ColType::Date,
Value::Number(_) => ColType::Number,
Value::Text(_) => ColType::Text,
Value::Bool(_) => ColType::Bool,
+1 -1
View File
@@ -335,7 +335,7 @@ fn check_input(wb: &Workbook, input: &gridcraft_model::NamedInput, place: Place)
gridcraft_model::ColType::Number => v.is_number(),
gridcraft_model::ColType::Text => v.is_text(),
gridcraft_model::ColType::Bool => matches!(v, Value::Bool(_)),
gridcraft_model::ColType::Date => v.is_number() && crate::display::parsed(&date()).is_date(),
gridcraft_model::ColType::Date => v.is_number() && crate::display::number_format(&date()).is_date(),
};
if !ok {
wrong.push((c, holds(&v, ty)));
+1 -1
View File
@@ -182,7 +182,7 @@ pub fn read_source(wb: &Workbook, text: &str, default_sheet: usize) -> Result<So
for (n, a) in numeric.iter_mut().zip(any.iter()) {
*n = *n && *a;
}
let dates = numeric.iter().zip(formats.iter()).map(|(n, f)| *n && crate::display::parsed(f).is_date()).collect();
let dates = numeric.iter().zip(formats.iter()).map(|(n, f)| *n && crate::display::number_format(f).is_date()).collect();
Ok(Source { sheet: si, range: r, headers, formats, numeric, dates, rows })
}
+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()));