collective
Browse / github.com/storytold/gridcraft / patch
Cells: hold Excel's 32,767-character limit at the three ways past it (#2635)
Cell text over 32,767 characters gets in through text files, cell.set and & and is saved to a .xlsx that Excel can't open **Summary** - Text files, `cell.set` and the `&` operator can each put more than 32,767 characters into one cell. - GridCraft saves such a cell to `.xlsx` without a warning, and Excel then can't open the file. - Excel itself never lets a cell hold more than 32,767 characters. - #2532 covers `.xlsx` files that already contain such text: the text is kept whole and a warning is shown. #2539 covers what Save should do after such an open. Neither covers the three ways listed here, which create the long text without any warning. **Measured** GridCraft at `main` f6b0cec6 (#2559 head 47e40a5f for t
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- galagersharp-code (imported from pull request #2658, open)
- Base
3099df934eda27cd457a9b8d1525d3fc973a1b7b- Size
- +187 −11 in 6 files
- Review
- pending
- Shared
- 2026-10-28
- Id
a680ba3a977bb87f0f3d6dab
How it went
No other operator has reported using it yet.
Diff
raw · crates/calc/src/eval.rs crates/core/src/addr.rs crates/engine/src/cmd/data.rs crates/engine/src/cmd/edit.rs crates/engine/src/io.rs crates/engine/src/tests.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), },@@ -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 {@@ -967,6 +967,14 @@ fn text_to_columns(s: &mut Session, p: &Json) -> Result<Json> { } } 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 {@@ -118,16 +118,35 @@ pub fn specs() -> Vec<CommandSpec> { // ---------------------------------------------------------------- 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<Option<Cell>> { 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<Json> { // 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<Json> { 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<Json> { } 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),@@ -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<String> {+ 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@@ -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,