diff --git a/ROADMAP.md b/ROADMAP.md index b0e3003..351db07 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 | Security: cell hyperlinks open only http, https and mailto, after a confirmation; other schemes (file:, smb:, UNC, custom handlers) are refused (#777, part 1) | | 2026-10-12 | Deleting several Ctrl-selected rows or columns removes them all in one undo step (#758) | | 2026-10-12 | XLSX chart sheets are kept on open and save: the sheet, its drawing and chart (with style and colours) are written back unchanged in their place among the worksheets, and left out when the sheet they plot is deleted or renamed or a worksheet took their name (#727) | diff --git a/crates/numfmt/src/lib.rs b/crates/numfmt/src/lib.rs index e5c65bd..6280e1d 100644 --- a/crates/numfmt/src/lib.rs +++ b/crates/numfmt/src/lib.rs @@ -98,15 +98,38 @@ 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 { #[cfg(test)] parse_cache::count_parse(); 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 }), }; @@ -376,6 +399,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 64a4fca..4408d38 100644 --- a/crates/numfmt/src/render.rs +++ b/crates/numfmt/src/render.rs @@ -509,7 +509,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 c335ed8..98fb765 100644 --- a/crates/numfmt/src/tests.rs +++ b/crates/numfmt/src/tests.rs @@ -785,3 +785,35 @@ fn parse_cache_is_bounded() { } assert_eq!(parse_cache::parses() - before, 3); } + +#[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 4700f8b..5e47c1f 100644 --- a/crates/xlsx/src/tests/malformed.rs +++ b/crates/xlsx/src/tests/malformed.rs @@ -194,6 +194,24 @@ fn hostile_protected_ranges_stay_locked() { } } +#[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); +} + #[test] fn hyperlinks_past_the_cell_cap_are_reported() { // A styles part, so the only possible warning is the hyperlink one.