Merge pull request #809 from vangel76:fix/numfmt-fraction-width-cap

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) |

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<String, C
     if code.is_empty() {
         return Ok(String::new());
     }
+    if too_long(code) {
+        return Err(CellError::Value);
+    }
     let fmt = NumberFormat::parse_cached(code);
     let n = match v.scalar() {
         Value::Error(e) => return Err(e),

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<char> = padded.chars().collect();
         let count = den_idx.len();
         for (k, &ti) in den_idx.iter().enumerate() {

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");
+}

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#"<styleSheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main"><numFmts count="1"><numFmt numFmtId="164" formatCode="{code}"/></numFmts>
+      <cellXfs count="2"><xf numFmtId="0"/><xf numFmtId="164"/></cellXfs></styleSheet>"#
+    );
+    let rels = r#"<Relationship Id="rId2" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/styles" Target="styles.xml"/>"#;
+    let body = r#"<sheetData><row r="1"><c r="A1" s="1"><v>0.5</v></c></row></sheetData>"#;
+    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.