diff --git a/ROADMAP.md b/ROADMAP.md index e8e1c06..3feaaef 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -132,6 +132,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-12 | Number formats: a fraction denominator with 65,536 zeros no longer crashes; format codes are capped at 255 characters like Excel (longer codes display as General, `TEXT` gives `#VALUE!`) (#807) | | 2026-10-12 | File › Print selects a system printer, copies, duplex and grayscale, submits PDF jobs through craft-print on a worker and shows spooler errors; browser printing and PDF export remain available | | 2026-10-11 | Command-sequence fuzzing: random command sequences checked against undo/redo, XLSX round-trip and full recalculation; fixes for the table, legacy-array, spill, AutoSum, Watch Window and sheet-copy bugs it found | | 2026-10-11 | Password-protected workbooks open: GridCraft asks for the password and decrypts Agile and Standard encryption (#11) | diff --git a/crates/numfmt/src/lib.rs b/crates/numfmt/src/lib.rs index 674e844..e380182 100644 --- a/crates/numfmt/src/lib.rs +++ b/crates/numfmt/src/lib.rs @@ -98,13 +98,36 @@ impl Default for NumberFormat { } } +/// Longest number format code, in characters (not bytes), that is parsed: Excel's own limit. +/// A longer code is invalid: [`NumberFormat::parse`] returns a General format (the cell shows +/// its value unformatted; the stored code is kept as is) and `TEXT` returns `#VALUE!`. The cap +/// also bounds every allocation that scales with the number of placeholders in a code. +pub const MAX_FORMAT_CODE_CHARS: usize = 255; + +/// True when `code` is longer than [`MAX_FORMAT_CODE_CHARS`] characters. +fn too_long(code: &str) -> bool { + // A code of more than 255 bytes can still be short in characters; count only when needed. + code.len() > MAX_FORMAT_CODE_CHARS && code.chars().count() > MAX_FORMAT_CODE_CHARS +} + impl NumberFormat { - /// Parses a format code. Never fails: unknown or malformed parts degrade to literal text. + /// Parses a format code. Never fails: unknown or malformed parts degrade to literal text, and + /// a code longer than [`MAX_FORMAT_CODE_CHARS`] characters degrades to General. pub fn parse(code: &str) -> NumberFormat { + Self::parse_inner(code, false) + } + + /// `parse` without the length cap, so tests can reach renderer paths a capped code cannot. + #[cfg(test)] + fn parse_uncapped(code: &str) -> NumberFormat { + Self::parse_inner(code, true) + } + + fn parse_inner(code: &str, uncapped: bool) -> NumberFormat { let trimmed = code.trim(); // `checkbox` marks a checkbox cell (Insert › Checkbox): TRUE/FALSE draw as a box, any other // value displays as General. - if trimmed.is_empty() || trimmed.eq_ignore_ascii_case("general") || trimmed == "checkbox" { + if trimmed.is_empty() || trimmed.eq_ignore_ascii_case("general") || trimmed == "checkbox" || (!uncapped && too_long(code)) { return NumberFormat { inner: Arc::new(Inner { code: code.to_string(), general: true, sections: Vec::new(), num_sections: 0, text_section: None }), }; @@ -301,6 +324,9 @@ pub fn text_function(v: &Value, code: &str, sys: DateSystem) -> Result return Err(e), diff --git a/crates/numfmt/src/render.rs b/crates/numfmt/src/render.rs index 7befe59..969e62c 100644 --- a/crates/numfmt/src/render.rs +++ b/crates/numfmt/src/render.rs @@ -496,7 +496,9 @@ fn render_fraction(sec: &Section, x: f64, pieces: &mut [String], lits: &mut [Opt // Denominator digits are zero padded to the `0` placeholders, then left aligned; `?` // pads with spaces after them. let zeros = den_idx.iter().filter(|&&i| toks.get(i) == Some(&Tok::Digit('0'))).count(); - let padded = format!("{den_s:0>zeros$}"); + // No `{:0>zeros$}`: a format width above u16::MAX panics. + let mut padded = "0".repeat(zeros.saturating_sub(den_s.chars().count())); + padded.push_str(&den_s); let dc: Vec = padded.chars().collect(); let count = den_idx.len(); for (k, &ti) in den_idx.iter().enumerate() { diff --git a/crates/numfmt/src/tests.rs b/crates/numfmt/src/tests.rs index 6c2306c..c70fbe7 100644 --- a/crates/numfmt/src/tests.rs +++ b/crates/numfmt/src/tests.rs @@ -582,3 +582,35 @@ fn decimal_digits_keeps_fifteen_digits_and_truncates_a_tie() { let (d, e) = decimal_digits(999999999999999.6); assert_eq!((d, e), ([1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0], 15)); } + +#[test] +fn fraction_denominator_with_65536_zeros_does_not_panic() { + // `{:0>n$}` panics for a width above u16::MAX; the cap keeps this out of reach of `parse`, + // so the renderer is driven with the uncapped parser. + let code = format!("# ?/{}", "0".repeat(65_536)); + let fmt = NumberFormat::parse_uncapped(&code); + let text = format_value(&Value::Number(0.5), &fmt, S).text; + assert!(text.starts_with(" 1/"), "{text}"); + // And a denominator with fewer digits than zeros is still zero padded. + assert_eq!(f("# ?/000", 0.5), " 1/002"); +} + +#[test] +fn format_codes_are_capped_at_255_characters() { + let ok = format!("0{}", "0".repeat(254)); + let long = format!("0{}", "0".repeat(255)); + assert_eq!(ok.chars().count(), MAX_FORMAT_CODE_CHARS); + assert!(text_function(&Value::Number(1.0), &ok, S).is_ok()); + assert_eq!(text_function(&Value::Number(1.0), &long, S), Err(CellError::Value)); + // Capped codes display as General. + assert_eq!(f(&long, 0.5), "0.5"); + // Characters are counted, not bytes: 255 two-byte characters are fine, 256 are not. + let multi_ok = "é".repeat(255); + assert!(multi_ok.len() > 255); + assert!(text_function(&Value::Number(1.0), &multi_ok, S).is_ok()); + assert_eq!(text_function(&Value::Number(1.0), &"é".repeat(256), S), Err(CellError::Value)); + // 65536 zeros through the public API: no panic, TEXT is #VALUE!. + let huge = format!("# ?/{}", "0".repeat(65_536)); + assert_eq!(text_function(&Value::Number(0.5), &huge, S), Err(CellError::Value)); + assert_eq!(f(&huge, 0.5), "0.5"); +} diff --git a/crates/xlsx/src/tests/malformed.rs b/crates/xlsx/src/tests/malformed.rs index b14e53a..d41de8e 100644 --- a/crates/xlsx/src/tests/malformed.rs +++ b/crates/xlsx/src/tests/malformed.rs @@ -193,3 +193,21 @@ fn hostile_protected_ranges_stay_locked() { assert!(!matches!(h.verify("password"), Ok(true)), "{}", r.title); } } + +#[test] +fn an_overlong_number_format_code_does_not_stop_the_open() { + // A format code far over the 255-character cap: the file still opens, the format code is kept as + // stored (display falls back to General) and the cell keeps its value. + let code = format!("# ?/{}", "0".repeat(65_536)); + let styles = format!( + r#" + "# + ); + let rels = r#""#; + let body = r#"0.5"#; + let (wb, _) = read_xlsx(&minimal(body, &[("xl/styles.xml", &styles)], rels, "")).unwrap(); + let s = wb.sheet(0).unwrap(); + let cell = s.cell(gridcraft_core::CellRef::parse("A1").unwrap()).unwrap(); + assert_eq!(s.value(gridcraft_core::CellRef::parse("A1").unwrap()), gridcraft_core::Value::Number(0.5)); + assert_eq!(wb.styles.get(cell.style).num_fmt.as_str(), code); +}