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 53 additions and 77 deletions
+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()));
-18
View File
@@ -652,21 +652,3 @@ fn xls_short_window2_is_ignored() {
assert_eq!(v(&wb, 0, "A1"), Value::Number(7.0));
}
}
/// Shared-formula followers each keep their own text, charged to the workbook's text allowance:
/// with no allowance left they keep their cached values, and the import still succeeds.
#[test]
fn shared_formula_text_counts_against_the_text_allowance() {
let file = compound("Workbook", &sample(false));
// The smallest allowance that reads every string: nothing is left for expanded formulas.
let left = (0..1000).find(|n| crate::xls::read_xls_with(&file, crate::xls::Budget::with_text_left(*n)).is_ok()).unwrap();
let (wb, report) = crate::xls::read_xls_with(&file, crate::xls::Budget::with_text_left(left)).unwrap();
assert_eq!(ftext(&wb, "B6"), None);
assert_eq!(v(&wb, 0, "B6"), Value::Number(20.0));
assert!(report.warnings.iter().any(|w| w.contains("shared-formula text")), "{:?}", report.warnings);
// an array anchor after the cut-off still gets its formula
assert!(ftext(&wb, "C5").is_some());
// With room, the followers keep their formulas.
let (wb, _) = crate::xls::read_xls_with(&file, crate::xls::Budget::default()).unwrap();
assert!(ftext(&wb, "B6").is_some());
}
+2 -58
View File
@@ -75,13 +75,11 @@ pub(crate) struct Budget {
text: usize,
records: usize,
max_records: usize,
/// A shared formula's text was refused: no later shared formula, in any sheet, is expanded.
shared_text_spent: bool,
}
impl Default for Budget {
fn default() -> Self {
Budget { cells: 0, text: 0, records: 0, max_records: MAX_RECORDS, shared_text_spent: false }
Budget { cells: 0, text: 0, records: 0, max_records: MAX_RECORDS }
}
}
@@ -91,11 +89,6 @@ impl Budget {
pub(crate) fn with_max_records(max_records: usize) -> Self {
Budget { max_records, ..Budget::default() }
}
/// A budget with only `left` bytes of text allowance (tests).
#[cfg(test)]
pub(crate) fn with_text_left(left: usize) -> Self {
Budget { text: MAX_TEXT.saturating_sub(left), ..Budget::default() }
}
fn record(&mut self) -> Result<(), IoError> {
self.records = self.records.saturating_add(1);
if self.records > self.max_records {
@@ -110,23 +103,6 @@ impl Budget {
}
Ok(())
}
/// Charges `len` bytes of text if they fit the allowance, leaving it untouched otherwise:
/// for text the import can do without (expanded shared formulas keep their cached values).
fn try_text(&mut self, len: usize) -> bool {
let total = self.text.saturating_add(len);
let fits = total <= MAX_TEXT;
if fits {
self.text = total;
}
fits
}
/// [`try_text`](Self::try_text) for a shared formula's text. The first refusal ends shared
/// expansion for the whole workbook, so a shorter formula on a later sheet doesn't fit in the
/// room the refused one left, and refused formulas aren't decoded again on each sheet.
fn try_shared_text(&mut self, len: usize) -> bool {
self.shared_text_spent = self.shared_text_spent || !self.try_text(len);
!self.shared_text_spent
}
fn text(&mut self, len: usize) -> Result<(), IoError> {
self.text = self.text.saturating_add(len);
if self.text > MAX_TEXT {
@@ -511,7 +487,6 @@ fn rk(v: u32) -> Value {
/// keep their cached values).
fn resolve_pending(
st: &mut SheetState<'_>,
budget: &mut Budget,
arrays: &HashMap<CellRef, (RangeRef, Parsed)>,
shared: &HashMap<CellRef, Parsed>,
pending: &[Pending],
@@ -524,27 +499,11 @@ fn resolve_pending(
st.attach(p.pos, f, Mode::Cell, Some(*range));
}
} else if let Some(f) = shared.get(&p.anchor) {
if budget.shared_text_spent {
st.unsupported("more shared-formula text than the import allows");
continue;
}
if failed.contains(&p.anchor) || shared_attaches >= MAX_SHARED_ATTACHES {
continue;
}
shared_attaches += 1;
st.attach(p.pos, f, Mode::Shared, None);
let len = st.sheet.cell(p.pos).and_then(|c| c.formula.as_ref()).map(|f| f.text.len());
// Every follower keeps its own text: charge it to the workbook's text allowance, and
// past it drop the formula again so the cell keeps its cached value.
if let Some(len) = len
&& !budget.try_shared_text(len)
{
if let Some(cell) = st.sheet.cells.get_mut(p.pos) {
cell.formula = None;
}
st.unsupported("more shared-formula text than the import allows");
continue;
}
let imported = st.sheet.cell(p.pos).and_then(|c| c.formula.as_ref()).is_some_and(|f| f.text.chars().count() <= MAX_SHARED_TEMPLATE);
if !imported {
failed.insert(p.anchor);
@@ -627,7 +586,7 @@ fn worksheet(
// Shared and array formulas: their definitions follow the first cell that uses them.
let (arrays, shared) = (std::mem::take(&mut st.arrays), std::mem::take(&mut st.shared));
let pending = std::mem::take(&mut st.pending);
resolve_pending(&mut st, budget, &arrays, &shared, &pending);
resolve_pending(&mut st, &arrays, &shared, &pending);
blank_array_followers(&mut st, &pending);
if st.dropped > 0 {
report.warn(format!("{} cells outside the grid were skipped.", st.dropped));
@@ -919,18 +878,3 @@ fn defined_names(g: &Globals, model_index: &[Option<usize>], wb: &mut Workbook,
}
}
}
#[cfg(test)]
mod budget_tests {
use super::*;
#[test]
fn a_refused_shared_formula_ends_shared_expansion_for_later_sheets() {
let mut budget = Budget::with_text_left(10);
assert!(budget.try_shared_text(8));
assert!(!budget.try_shared_text(8), "over the allowance");
// a shorter formula would fit the 2 bytes left, but the cut-off holds
assert!(!budget.try_shared_text(1));
assert!(budget.try_text(1), "text the import needs is still charged normally");
}
}