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 46 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()));
+10 -19
View File
@@ -254,36 +254,27 @@ pub(crate) fn sheet_names_in(formula: &str) -> Vec<String> {
/// Whether a kept chart sheet can be written back: its chart's series must still refer to sheets
/// that exist under the same names (the chart is kept as it was, so a series on a deleted or
/// renamed sheet would be left pointing at nothing), and no worksheet may have taken its name.
///
/// `usable` caches, by relationship id, whether that chart sheet's graph refers only to sheets
/// that exist (`false` when the relationship is missing): entries sharing one graph are analysed
/// once, and only the verdict is kept, not the references it came from.
fn chart_sheet_usable<'p>(wb: &Workbook, p: &'p Preserved, s: &'p PreservedSheet, usable: &mut HashMap<&'p str, bool>) -> bool {
fn chart_sheet_usable(wb: &Workbook, p: &Preserved, s: &PreservedSheet) -> bool {
let same = |a: &str, b: &str| a.chars().take(31).collect::<String>().eq_ignore_ascii_case(&b.chars().take(31).collect::<String>());
if wb.sheets.iter().any(|w| same(&w.name, &s.name)) {
return false;
}
*usable.entry(s.rel_id.as_str()).or_insert_with(|| {
let Some(root) = p.rels.iter().find(|r| r.id == s.rel_id) else { return false };
let parts = reachable(p, root);
let mut formulas = vec![];
for q in p.parts.iter().filter(|q| parts.contains(&normalize(&q.name)) && q.content_type.contains("chart")) {
if let Ok(x) = xml::parse(&q.data) {
chart_formulas(&x, &mut formulas);
}
let Some(root) = p.rels.iter().find(|r| r.id == s.rel_id) else { return false };
let parts = reachable(p, root);
let mut formulas = vec![];
for q in p.parts.iter().filter(|q| parts.contains(&normalize(&q.name)) && q.content_type.contains("chart")) {
if let Ok(x) = xml::parse(&q.data) {
chart_formulas(&x, &mut formulas);
}
formulas.iter().flat_map(|f| sheet_names_in(f)).all(|n| wb.sheets.iter().any(|w| same(&w.name, &n)))
})
}
formulas.iter().flat_map(|f| sheet_names_in(f)).all(|n| wb.sheets.iter().any(|w| same(&w.name, &n)))
}
/// Writes the workbook's kept parts and relationships. Chart sheets that can't be written back
/// (see [`chart_sheet_usable`]) are left out with the parts only they reach.
pub fn write_workbook(wb: &Workbook, wb_rels: &mut Rels, out: &mut Out) -> WorkbookKept {
let Some(p) = wb.preserved.as_deref() else { return WorkbookKept::default() };
let (live, dropped): (Vec<&PreservedSheet>, Vec<&PreservedSheet>) = {
let mut refs = HashMap::new();
p.sheets.iter().partition(|s| chart_sheet_usable(wb, p, s, &mut refs))
};
let (live, dropped): (Vec<&PreservedSheet>, Vec<&PreservedSheet>) = p.sheets.iter().partition(|s| chart_sheet_usable(wb, p, s));
let root_of = |s: &PreservedSheet| p.rels.iter().find(|r| r.id == s.rel_id);
let dropped_ids: HashSet<&str> = dropped.iter().map(|s| s.rel_id.as_str()).collect();
// Parts reached only by dropped chart sheets go with them.
@@ -696,32 +696,6 @@ fn chartsheet_package() -> Vec<u8> {
])
}
/// Thousands of chart-sheet entries naming one chart sheet's relationship: its graph is walked
/// and its charts parsed once when saving, not once per entry.
#[test]
fn chart_sheet_entries_sharing_a_graph_are_checked_once() {
let bytes = chartsheet_package();
let mut z = zip::ZipArchive::new(std::io::Cursor::new(bytes)).unwrap();
let mut parts: Vec<(String, Vec<u8>)> = Vec::new();
for i in 0..z.len() {
let mut f = z.by_index(i).unwrap();
let mut data = Vec::new();
f.read_to_end(&mut data).unwrap();
parts.push((f.name().to_string(), data));
}
let extra: String = (0..3_000).map(|i| format!(r#"<sheet name="Copy{i}" sheetId="{}" r:id="rId2"/>"#, 100 + i)).collect();
for (name, data) in &mut parts {
if name == "xl/workbook.xml" {
*data = String::from_utf8(data.clone()).unwrap().replace("</sheets>", &format!("{extra}</sheets>")).into_bytes();
}
}
let refs: Vec<(&str, &[u8])> = parts.iter().map(|(n, d)| (n.as_str(), d.as_slice())).collect();
let (wb, _) = read_xlsx(&make_zip(&refs)).unwrap();
let u = Unpacked::new(&write_xlsx(&wb).unwrap());
u.assert_valid();
assert!(written_sheets(&u).iter().any(|(n, _)| n == "Chart1"));
}
/// The sheets of a written workbook: (name, state) in file order.
fn written_sheets(u: &Unpacked) -> Vec<(String, String)> {
let wb = xml::parse(u.parts.get("xl/workbook.xml").unwrap()).unwrap();