From 7776cfcc5caa66923f42b00438ca5279a2694163 Mon Sep 17 00:00:00 2001 From: zyzabruh Date: Mon, 19 Oct 2026 13:16:13 +0200 Subject: [PATCH 1/2] Accounting format: the documented currency `symbol` is honoured `home.accounting` documents `{range?, symbol?}` but ignored the symbol: the command always applied the `$` format, so a script or MCP client asking for lempiras (#1893) or any other currency silently got dollars. The symbol is now part of the code it writes, and a symbol that would break the format string (a quote or a backslash) is refused instead of being pasted in. The default is unchanged, character for character. Tests: `accounting_format_takes_the_symbol_it_is_given` (displayed text for `L` and the default, and the two refused symbols) and `the_default_symbol_reproduces_the_accounting_format` (the built code still equals the named `Accounting` format). `cargo xtask ci`: 8/8 steps, 0 failures. --- crates/engine/src/cmd/format.rs | 23 ++++++++++++++++++++--- crates/engine/src/tests.rs | 21 +++++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs index 4712351ca..7561b6ddc 100644 --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -133,9 +133,7 @@ pub fn specs() -> Vec { has_doc, number_format ), - cmd!("home.accounting", "Accounting Number Format", ["Home", "Number"], None, "{range?, symbol?: \"$\"}", has_doc, |s, p| set_fmt( - s, p, ACCOUNTING - )), + cmd!("home.accounting", "Accounting Number Format", ["Home", "Number"], None, "{range?, symbol?: \"$\"}", has_doc, accounting), cmd!("home.percent", "Percent Style", ["Home", "Number"], Some("Ctrl+Shift+%"), "{range?}", has_doc, |s, p| set_fmt(s, p, "0%")), cmd!("home.comma", "Comma Style", ["Home", "Number"], None, "{range?}", has_doc, |s, p| set_fmt(s, p, COMMA)), cmd!("home.increaseDecimal", "Increase Decimal", ["Home", "Number"], None, "{range?}", has_doc, |s, p| decimals(s, p, 1)), @@ -202,6 +200,20 @@ pub fn specs() -> Vec { const ACCOUNTING: &str = "_(\"$\"* #,##0.00_);_(\"$\"* \\(#,##0.00\\);_(\"$\"* \"-\"??_);_(@_)"; const COMMA: &str = "_(* #,##0.00_);_(* \\(#,##0.00\\);_(* \"-\"??_);_(@_)"; +/// Accounting Number Format with the currency symbol to use (Excel's own button takes the system +/// currency; `symbol` is how the control channel picks another one, e.g. `"L"` for lempiras). +fn accounting(s: &mut Session, p: &Json) -> Result { + let symbol = str_param(p, "symbol").unwrap_or("$"); + if symbol.contains(['"', '\\']) { + return Err(bad("home.accounting", "the symbol can't contain a quote or a backslash")); + } + set_fmt(s, p, &accounting_format(symbol)) +} + +fn accounting_format(symbol: &str) -> String { + format!("_({symbol:?}* #,##0.00_);_({symbol:?}* \\(#,##0.00\\);_({symbol:?}* \"-\"??_);_(@_)") +} + fn with(p: &Json, key: &str, v: Json) -> Json { let mut m = p.as_object().cloned().unwrap_or_default(); m.insert(key.into(), v); @@ -1137,6 +1149,11 @@ mod tests { assert_eq!(v(&mut s, "D1"), json!(4.0)); } + #[test] + fn the_default_symbol_reproduces_the_accounting_format() { + assert_eq!(accounting_format("$"), ACCOUNTING); + } + #[test] fn decimal_steps() { let v = gridcraft_core::Value::Number(1.5); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ae499a189..f4d17bd7e 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1259,6 +1259,27 @@ fn slicer_filters_table() { assert!(s.execute("slicer.toggle", json!({"value": "Nowhere"})).is_err()); } +#[test] +fn accounting_format_takes_the_symbol_it_is_given() { + // The command documents `symbol`; it used to be dropped, so only `$` was ever reachable + // from the control channel (asking for lempiras, #1893, changed nothing). + let text = |s: &Session, a: &str| { + let d = s.doc().unwrap(); + crate::display::cell_text(&d.wb, d.wb.active().unwrap(), CellRef::parse(a).unwrap()) + }; + let mut s = s(); + s.execute("cell.set", json!({"cell": "A1", "input": "1234.5"})).unwrap(); + s.execute("home.accounting", json!({"range": "A1", "symbol": "L"})).unwrap(); + assert_eq!(text(&s, "A1"), " L 1,234.50 "); + // Without a symbol it stays the default `$` format, character for character. + s.execute("home.accounting", json!({"range": "A1"})).unwrap(); + assert_eq!(text(&s, "A1"), " $ 1,234.50 "); + // A quote or a backslash would break the format string, so the symbol is refused. + for bad in ["\"", "\\"] { + assert!(s.execute("home.accounting", json!({"range": "A1", "symbol": bad})).is_err(), "{bad}"); + } +} + #[test] fn table_header_formatted_as_accounting_keeps_its_text_as_the_column_name() { // Found by the command-sequence fuzzer: the accounting format pads text (` 172.4 `), and that From 041bee21688a082fde64de4aef85539c480b7aab Mon Sep 17 00:00:00 2001 From: zyzabruh Date: Mon, 19 Oct 2026 14:03:14 +0200 Subject: [PATCH 2/2] Accounting format: quote the symbol as format text, refuse control characters (review #1930) The symbol was interpolated with Rust's Debug formatting, which escapes a control character into literal `\n`-style text. It is now written into the format's quoted literal directly, and a symbol holding a control character is refused along with a quote or a backslash, so nothing is escaped behind the user's back. Test: `accounting_format_takes_the_symbol_it_is_given` covers `\n` and a tab. `cargo xtask ci`: 8/8 steps, 0 failures. --- crates/engine/src/cmd/format.rs | 9 ++++++--- crates/engine/src/tests.rs | 7 ++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs index 7561b6ddc..a935a6d01 100644 --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -204,14 +204,17 @@ const COMMA: &str = "_(* #,##0.00_);_(* \\(#,##0.00\\);_(* \"-\"??_);_(@_)"; /// currency; `symbol` is how the control channel picks another one, e.g. `"L"` for lempiras). fn accounting(s: &mut Session, p: &Json) -> Result { let symbol = str_param(p, "symbol").unwrap_or("$"); - if symbol.contains(['"', '\\']) { - return Err(bad("home.accounting", "the symbol can't contain a quote or a backslash")); + // The symbol goes into a quoted literal: a quote or a backslash would end it, and a control + // character would reach the format's text as-is. + if symbol.contains(['"', '\\']) || symbol.chars().any(char::is_control) { + return Err(bad("home.accounting", "the symbol can't contain a quote, a backslash or a control character")); } set_fmt(s, p, &accounting_format(symbol)) } fn accounting_format(symbol: &str) -> String { - format!("_({symbol:?}* #,##0.00_);_({symbol:?}* \\(#,##0.00\\);_({symbol:?}* \"-\"??_);_(@_)") + let symbol = format!("\"{symbol}\""); + format!("_({symbol}* #,##0.00_);_({symbol}* \\(#,##0.00\\);_({symbol}* \"-\"??_);_(@_)") } fn with(p: &Json, key: &str, v: Json) -> Json { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index f4d17bd7e..97e460cf9 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1274,9 +1274,10 @@ fn accounting_format_takes_the_symbol_it_is_given() { // Without a symbol it stays the default `$` format, character for character. s.execute("home.accounting", json!({"range": "A1"})).unwrap(); assert_eq!(text(&s, "A1"), " $ 1,234.50 "); - // A quote or a backslash would break the format string, so the symbol is refused. - for bad in ["\"", "\\"] { - assert!(s.execute("home.accounting", json!({"range": "A1", "symbol": bad})).is_err(), "{bad}"); + // A quote or a backslash would end the format's quoted text, and a control character would + // reach it as-is, so those symbols are refused. + for bad in ["\"", "\\", "\n", "L\t"] { + assert!(s.execute("home.accounting", json!({"range": "A1", "symbol": bad})).is_err(), "{bad:?}"); } }