diff --git a/crates/calc/src/eval.rs b/crates/calc/src/eval.rs --- a/crates/calc/src/eval.rs +++ b/crates/calc/src/eval.rs @@ -1821,11 +1821,10 @@ fn binary_scalar(op: BinOp, a: &Value, b: &Value, sys: gridcraft_core::DateSyste }), BinOp::Concat => match (a.to_text(), b.to_text()) { (Ok(x), Ok(y)) => { - if x.len() + y.len() > 32767 * 4 { - Value::Error(CellError::Value) - } else { - Value::text(x + &y) - } + // Excel keeps a concatenation cut to the cell's 32,767 characters, counted in + // UTF-16 units like `LEN`, without an error; the old 4-bytes-per-character bound + // only capped memory, and a longer cell saves a file Excel can't open (#2635). + Value::text(gridcraft_core::addr::cut_to_cell_chars(x + &y)) } (Err(e), _) | (_, Err(e)) => Value::Error(e), }, diff --git a/crates/core/src/addr.rs b/crates/core/src/addr.rs --- a/crates/core/src/addr.rs +++ b/crates/core/src/addr.rs @@ -10,6 +10,36 @@ use serde::{Deserialize, Serialize}; pub const MAX_ROWS: u32 = 1_048_576; pub const MAX_COLS: u32 = 16_384; +/// The most text one cell holds, counted in UTF-16 code units like `LEN` counts — Excel's cell +/// limit. A longer cell saves to a file Excel can't open, so editing refuses more and text +/// imports keep it whole only with a warning. +pub const MAX_CELL_CHARS: usize = 32_767; + +/// The string's length in UTF-16 code units, the count [`MAX_CELL_CHARS`] and `LEN` use: one +/// unit for most characters, two for the ones outside the Basic Multilingual Plane. +pub fn utf16_len(s: &str) -> usize { + s.chars().map(char::len_utf16).sum() +} + +/// The string cut to at most [`MAX_CELL_CHARS`] UTF-16 code units, as Excel stores a longer +/// concatenation (`=REPT("x",20000)&REPT("x",20000)` keeps its first 32,767 characters). +pub fn cut_to_cell_chars(s: String) -> String { + if utf16_len(&s) <= MAX_CELL_CHARS { + return s; + } + let mut out = String::with_capacity(s.len()); + let mut units = 0; + for c in s.chars() { + let w = c.len_utf16(); + if units + w > MAX_CELL_CHARS { + break; + } + units += w; + out.push(c); + } + out +} + /// A cell position (0-based). #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, Serialize, Deserialize, Default)] pub struct CellRef { diff --git a/crates/engine/src/cmd/data.rs b/crates/engine/src/cmd/data.rs --- a/crates/engine/src/cmd/data.rs +++ b/crates/engine/src/cmd/data.rs @@ -967,6 +967,14 @@ fn text_to_columns(s: &mut Session, p: &Json) -> Result { } } for (c, t) in out { + // Checked before input_to_cell, whose error this loop would swallow into a cleared + // cell: an over-limit part must be refused, not dropped. The count is what + // input_to_cell would store — a leading `'` marker is stripped on the parsing path — + // so a marker plus exactly 32,767 units is not refused. + let style = cx.wb.sheet(sheet).map(|sh| sh.style_id(c)).unwrap_or_default(); + let fmt_is_text = cx.wb.styles.get(style).num_fmt.as_str() == "@"; + let stored = if fmt_is_text { t.as_str() } else { t.strip_prefix('\'').unwrap_or(t.as_str()) }; + super::edit::check_cell_text(stored, c)?; let cell = super::edit::input_to_cell(&t, sheet, c, &mut cx.wb).unwrap_or(None); let shm = cx.sheet_mut(sheet)?; match cell { diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -118,16 +118,35 @@ pub fn specs() -> Vec { // ---------------------------------------------------------------- entry +/// A cell's text holds at most Excel's 32,767 characters, counted in UTF-16 units like `LEN` +/// ([`gridcraft_core::addr::MAX_CELL_CHARS`]): more saves a file Excel can't open (#2635). +/// Editing refuses it and names the cell; text files keep the field whole and warn at open. +pub(crate) fn check_cell_text(input: &str, at: CellRef) -> Result<()> { + let units = gridcraft_core::addr::utf16_len(input); + if units > gridcraft_core::addr::MAX_CELL_CHARS { + return Err(EngineError::Other(format!( + "{}: a cell holds at most 32,767 characters, and this is {} — Excel refuses more too", + at.a1(), + units + ))); + } + Ok(()) +} + /// What typing `input` into a cell produces (constant or formula, plus an automatic number /// format). Inherits the destination's cell, row or column style, including its number /// format, before parsing. Errors when the formula can't be parsed. pub(crate) fn input_to_cell(input: &str, sheet: usize, at: CellRef, wb: &mut gridcraft_model::Workbook) -> Result> { let style = wb.sheet(sheet).map(|sh| sh.style_id(at)).unwrap_or_default(); + let fmt_is_text = wb.styles.get(style).num_fmt.as_str() == "@"; + // The limit is on what the cell will hold: the parsing path stores `parse_input`'s text with + // a leading `'` marker stripped, while a text-format cell keeps everything as typed. + let text_for_limit = if fmt_is_text { input } else { input.strip_prefix('\'').unwrap_or(input) }; + check_cell_text(text_for_limit, at)?; if input.is_empty() { let c = Cell { value: Value::Empty, formula: None, style }; return Ok(if c.is_blank() { None } else { Some(c) }); } - let fmt_is_text = wb.styles.get(style).num_fmt.as_str() == "@"; if fmt_is_text { return Ok(Some(Cell { value: Value::text(input), formula: None, style })); } @@ -335,6 +354,9 @@ fn range_set_values(s: &mut Session, p: &Json) -> Result { // text rather than the marker that makes it text. A formula is still a // formula, which is what the ticket asks for and what `=` means in a cell. Json::String(text) if !parse && !text.starts_with('=') => { + // This branch stores the text without input_to_cell, so the cell limit + // is checked here or a 32,768-unit string slips through it (#2635). + check_cell_text(text, at)?; let mut style = cx.wb.sheet(sheet).map(|sh| sh.style_id(at)).unwrap_or_default(); // Line breaks still turn on Wrap Text, as typed input does. if text.contains('\n') && !cx.wb.styles.get(style).align.wrap { @@ -879,7 +901,15 @@ fn paste_text(s: &mut Session, p: &Json, text: &str) -> Result { for (ri, row) in rows.iter().enumerate() { for (ci, v) in row.iter().enumerate() { let Some(c) = at.offset(ri as i64, ci as i64) else { continue }; - let mut cell = input_to_cell(v, sheet, c, &mut cx.wb).unwrap_or_else(|_| Some(Cell::value(Value::text(v.as_str())))); + let mut cell = match input_to_cell(v, sheet, c, &mut cx.wb) { + Ok(cell) => cell, + // The fallback stores the text as given, marker included, so it must fit the + // limit too; the parsed path validated the text it stores, marker stripped. + Err(_) => { + check_cell_text(v, c)?; + Some(Cell::value(Value::text(v.as_str()))) + } + }; if let Some(cl) = cell.as_mut() { numeric_from_html(cl, v, html.get(ri).and_then(|r| r.get(ci)), &mut cx.wb); } @@ -1640,7 +1670,15 @@ fn replace(s: &mut Session, p: &Json) -> Result { } else { replace_ci(&text, &what, &with) }; - let cell = input_to_cell(&new, si, c, &mut cx.wb).unwrap_or_else(|_| Some(Cell::value(Value::text(new.as_str())))); + let cell = match input_to_cell(&new, si, c, &mut cx.wb) { + Ok(cell) => cell, + // The fallback stores the replaced text as given, so it must fit the limit + // too; the parsed path validated the text it stores, marker stripped. + Err(_) => { + check_cell_text(&new, c)?; + Some(Cell::value(Value::text(new.as_str()))) + } + }; let sh = cx.sheet_mut(si)?; match cell { Some(cell) => sh.set_cell(c, cell), diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -177,17 +177,50 @@ fn open_plain(name: &str, bytes: &[u8], csv_as_text: bool) -> Result<(Workbook, // Tab is what makes it a .tsv/.txt; saved as .csv it gets the CSV default. wb.csv_delimiter = None; rename_first_sheet(&mut wb, name); - Ok((wb, vec![])) + let warnings = over_limit_text_warnings(&wb, name); + Ok((wb, warnings)) } _ => { let opts = gridcraft_xlsx::CsvOptions { delimiter: 0, parse_values: !csv_as_text, ..Default::default() }; let mut wb = gridcraft_xlsx::read_csv(bytes, &opts).map_err(|e| EngineError::Other(e.to_string()))?; rename_first_sheet(&mut wb, name); - Ok((wb, vec![])) + let warnings = over_limit_text_warnings(&wb, name); + Ok((wb, warnings)) } } } +/// Text files keep every field whole, so a field past Excel's 32,767-character cell limit comes +/// in as one cell (#2635). The workbook keeps it and this warning says what saving it does: +/// Excel refuses or repairs the file. Naming the first cell keeps the warning actionable. +fn over_limit_text_warnings(wb: &gridcraft_model::Workbook, name: &str) -> Vec { + let mut count = 0usize; + let mut first = None; + for sheet in wb.sheets.iter() { + for (at, cell) in sheet.cells.iter() { + if let gridcraft_core::Value::Text(t) = &cell.value + && gridcraft_core::addr::utf16_len(t) > gridcraft_core::addr::MAX_CELL_CHARS + { + count += 1; + if first.is_none() { + first = Some(format!("{}!{}", sheet.name, at.a1())); + } + } + } + } + match count { + 0 => vec![], + _ => vec![format!( + "{name}: {count} cell{} hold{} more than Excel's 32,767 characters and {} kept whole, as the text file had {}; Excel may refuse or repair the file this saves to (first: {})", + if count == 1 { "" } else { "s" }, + if count == 1 { "s" } else { "" }, + if count == 1 { "was" } else { "were" }, + if count == 1 { "it" } else { "them" }, + first.unwrap_or_default() + )], + } +} + /// What to tell the user when one part of an XLSX package unpacks to more than the reader's /// per-part limit: the sizes, why the limit is there, and what gets the data in. The limit is /// per part, so smaller sheets fit; the shared strings are one part for every sheet, so only a @@ -766,6 +799,7 @@ mod tests { } #[cfg(not(target_arch = "wasm32"))] + #[cfg(unix)] #[test] fn save_fails_upfront_when_the_temp_name_is_taken() { // Another local user who can write the directory must not be able to point the diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -676,7 +676,12 @@ fn rich_clipboard_omits_html_for_large_ranges_without_truncating_text() { fn rich_clipboard_omits_html_when_escaping_exceeds_byte_budget() { let mut s = s(); let text = "&".repeat(1_000_000); - s.execute("range.setValues", json!({"range": "A1", "values": [[text]]})).unwrap(); + // The 32,767-character cell limit refuses a value this long through cell.set (#2635), but a + // file may still hold one; the over-limit cell is built on the model to test what copy makes + // of it. + let wb = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().wb); + let sheet = std::sync::Arc::make_mut(&mut wb.sheets[0]); + sheet.set_value(gridcraft_core::CellRef::new(0, 0), Value::text(text)); let copied = s.execute("edit.copy", json!({"range": "A1", "html": true})).unwrap(); assert!(copied.get("html").is_none(), "an oversized escaped value must not produce partial HTML"); assert_eq!(copied["text"].as_str().unwrap().len(), 1_000_001); @@ -2032,6 +2037,68 @@ fn copies_of_a_long_named_sheet_get_unique_names() { assert_eq!(names, ["Long name of a sheet 31 chars!", "Long name of a sheet 31 cha (2)", "Long name of a sheet 31 cha (3)"]); } +#[test] +fn cell_text_past_excels_limit_is_refused_and_concatenation_is_cut() { + // #2635: a cell holds 32,767 characters, counted in UTF-16 units like `LEN`. More through + // cell.set saved a file Excel can't open, so editing refuses it and names the cell; at the + // limit itself is fine. + let mut s = s(); + let at_limit = "x".repeat(32_767); + s.execute("cell.set", json!({"cell": "B2", "input": at_limit})).unwrap(); + let e = s.execute("cell.set", json!({"cell": "B3", "input": "x".repeat(32_768)})).unwrap_err().to_string(); + assert!(e.contains("B3"), "{e}"); + assert!(e.contains("32,767"), "{e}"); + // The count is UTF-16 units, not characters: 16,384 characters outside the Basic + // Multilingual Plane take 32,768 units and are refused; 16,383 fit. + let astral = '\u{1F600}'.to_string().repeat(16_384); + assert!(s.execute("cell.set", json!({"cell": "C1", "input": astral})).is_err()); + let astral = '\u{1F600}'.to_string().repeat(16_383); + s.execute("cell.set", json!({"cell": "C1", "input": astral})).unwrap(); + // range.setValues refuses too — both its parsed path and the `parse: false` one, which + // stores the text without input_to_cell. + let e = s.execute("range.setValues", json!({"range": "A1", "values": [["x".repeat(40_000)]]})).unwrap_err().to_string(); + assert!(e.contains("32,767"), "{e}"); + let e = s.execute("range.setValues", json!({"range": "A1", "values": [["x".repeat(40_000)]], "parse": false})).unwrap_err().to_string(); + assert!(e.contains("32,767"), "{e}"); + // The limit counts what the cell will hold: a leading `'` marker is not stored, so a marker + // plus 32,767 units fits while a marker plus 32,768 does not. + s.execute("cell.set", json!({"cell": "E1", "input": format!("'{}", "x".repeat(32_767))})).unwrap(); + assert_eq!(v(&s, "E1"), Value::Text("x".repeat(32_767).into()), "the marker itself is not stored"); + assert!(s.execute("cell.set", json!({"cell": "E2", "input": format!("'{}", "x".repeat(32_768))})).is_err()); + // Paste and replace count what they store, too: a marker plus 32,767 units fits, a plain + // 32,768 does not (paste's fallback would store that raw). + s.execute("edit.paste", json!({"at": "F1", "text": format!("'{}", "x".repeat(32_767))})).unwrap(); + assert_eq!(v(&s, "F1"), Value::Text("x".repeat(32_767).into())); + assert!(s.execute("edit.paste", json!({"at": "F2", "text": "x".repeat(32_768)})).is_err()); + s.execute("cell.set", json!({"cell": "G1", "input": "needle"})).unwrap(); + s.execute("edit.replace", json!({"what": "needle", "with": format!("'{}", "x".repeat(32_767))})).unwrap(); + assert_eq!(v(&s, "G1"), Value::Text("x".repeat(32_767).into())); + // `&` cuts to the limit like Excel's, instead of handing the cell 40,000 characters: the + // old 4-bytes-per-character bound only capped memory. + s.execute("cell.set", json!({"cell": "D1", "input": "=REPT(\"x\",20000)&REPT(\"x\",20000)"})).unwrap(); + assert_eq!(v(&s, "D1"), Value::Text("x".repeat(32_767).into()), "LEN(D1) would say 32767"); +} + +#[test] +fn a_csv_field_past_the_cell_limit_is_kept_whole_with_a_warning() { + // #2635: text import keeps the field whole — splitting or cutting it loses the record's + // shape — and warns, because what it saves is a file Excel refuses. + let mut csv = b"id;text\r\n1;".to_vec(); + csv.extend(std::iter::repeat_n(b'x', 100_000)); + csv.extend(b"\r\n"); + let (wb, warnings) = crate::io::open_bytes("long.csv", &csv).unwrap(); + let cell = wb.sheets[0].cells.iter().find(|(at, _)| at.a1() == "B2").map(|(_, c)| &c.value); + assert_eq!(cell, Some(&Value::Text("x".repeat(100_000).into())), "the field is kept whole"); + assert_eq!(warnings.len(), 1, "{warnings:?}"); + assert!(warnings[0].contains("32,767"), "{warnings:?}"); + assert!(warnings[0].contains("B2"), "{warnings:?}"); + assert!(warnings[0].contains("Excel may refuse or repair"), "{warnings:?}"); + // A file inside the limit says nothing. + let (wb, warnings) = crate::io::open_bytes("short.csv", b"id;text\r\n1;hello\r\n".as_slice()).unwrap(); + assert!(warnings.is_empty(), "{warnings:?}"); + assert_eq!(wb.sheets[0].name, "short"); +} + #[test] fn row_and_column_inserts_move_legacy_array_ranges() { // Found by the command-sequence fuzzer: the formula cell moved but its array range stayed,