Compare commits

..
Author SHA1 Message Date
Dylan M. Taylor 6cdd6db150 engine: show overlong format codes as General without hashing them per cell
The display cache hashed the full format code on every lookup, so one
huge code shared by many cells cost its length for every cell drawn.
A code with more than 4 × 255 bytes can't be within Excel's 255
characters and is now formatted as General straight away.
2026-11-03 14:07:22 +00:00
3 changed files with 33 additions and 45 deletions
+22
View File
@@ -10,6 +10,11 @@ use gridcraft_numfmt::{Formatted, NumberFormat, format_value};
/// Parsed number formats are cached by code.
fn parsed(code: &str) -> NumberFormat {
static CACHE: std::sync::LazyLock<Mutex<HashMap<String, NumberFormat>>> = std::sync::LazyLock::new(|| Mutex::new(HashMap::new()));
// A code of more bytes than 4 × the character limit can't be within it: it shows as General
// without being hashed or scanned again for every cell that shares it.
if code.len() > 4 * gridcraft_numfmt::MAX_FORMAT_CODE_CHARS {
return NumberFormat::parse("General");
}
let lock = || CACHE.lock().unwrap_or_else(std::sync::PoisonError::into_inner);
if let Some(f) = lock().get(code) {
return f.clone();
@@ -226,3 +231,20 @@ pub fn stats(sheet: &Sheet, ranges: &[RangeRef]) -> Stats {
}
s
}
#[cfg(test)]
mod tests {
use super::*;
/// A format code far past Excel's 255 characters shows its values as General, for every cell
/// that shares it, without being hashed into the cache each time.
#[test]
fn an_overlong_code_formats_as_general() {
let wb = Workbook::new();
let code = format!("0.00{}", "\"x\"".repeat(400_000));
for _ in 0..1_000 {
assert_eq!(format(&Value::Number(1.5), &code, &wb).text, "1.5");
}
assert_eq!(format(&Value::Number(1.5), "0.00", &wb).text, "1.50");
}
}
+2 -2
View File
@@ -755,10 +755,10 @@ thread_local! {
/// While one is alive, [`read_file`] and [`write_file`] on this thread refuse every path: a
/// stored script's steps run inside one (#2178), so no command reaches the file system from a
/// script whatever its parameters are called. Dropping it (also on unwind) lifts the refusal.
pub struct ScriptSandbox(());
pub(crate) struct ScriptSandbox(());
impl ScriptSandbox {
pub fn enter() -> Self {
pub(crate) fn enter() -> Self {
let _ = SCRIPT_SANDBOX.try_with(|c| c.set(c.get().saturating_add(1)));
ScriptSandbox(())
}
+9 -43
View File
@@ -77,8 +77,6 @@ fn probe_live_commands(calls: &[(String, String)]) -> Probe {
let params = if params.is_empty() { serde_json::json!({}) } else { serde_json::from_str(params).unwrap_or_default() };
let mut s = Session::new();
s.new_workbook();
// The JSON picks the commands and their params: none of them may touch the file system.
let _sandbox = gridcraft_engine::io::ScriptSandbox::enter();
let status = match s.execute(id, params.clone()) {
Ok(_) => {
if s.journal.iter().any(|(jid, _)| jid == id) {
@@ -183,16 +181,15 @@ fn parse_and_probe(source: &str) -> Result<(Vec<Entry>, Probe), String> {
}
}
}
let excel = e.get("excel").and_then(|v| v.as_str()).unwrap_or("").to_string();
let note = e.get("note").and_then(|v| v.as_str()).unwrap_or("").to_string();
// These are written into generated Rust: a target inside string literals and format
// strings, the annotations after `//`. Refuse anything that could end either.
for (field, text) in [("target", &target), ("excel", &excel), ("note", &note)] {
if text.chars().any(|c| c.is_control() || matches!(c, '"' | '\\' | '{' | '}')) {
return Err(format!("{at}: `{field}` must not hold quotes, backslashes, braces or control characters: {text:?}"));
}
}
entries.push(Entry { seq, kind, target, pending, params, excel, note });
entries.push(Entry {
seq,
kind,
target,
pending,
params,
excel: e.get("excel").and_then(|v| v.as_str()).unwrap_or("").to_string(),
note: e.get("note").and_then(|v| v.as_str()).unwrap_or("").to_string(),
});
}
// JSON order is authoritative and curated: the first entry for a tab is its canonical
// badge (legacy aliases come later), and groups stay together. Do not sort.
@@ -437,34 +434,3 @@ fn reset(h: &mut Harness) {
key(h, Key::Escape); // leave keytip mode entirely
}
"#;
#[cfg(all(test, feature = "engine"))]
mod tests {
use super::*;
/// Text that would end a comment or string literal in the generated Rust is refused.
#[test]
fn annotations_and_targets_cannot_escape_generated_code() {
for entry in [
r#"{"seq": "Q", "kind": "command", "target": "edit.copy", "note": "x\nfn evil() {}"}"#,
r#"{"seq": "Q", "kind": "command", "target": "edit.copy", "excel": "a\rb"}"#,
r#"{"seq": "Q", "kind": "dialog", "target": "x\"); evil(); (\""}"#,
r#"{"seq": "Q", "kind": "control", "target": "{0}"}"#,
] {
let doc = format!(r#"{{"sequences": [{entry}]}}"#);
assert!(parse_and_probe(&doc).is_err(), "{entry}");
}
assert!(parse_and_probe(r#"{"sequences": [{"seq": "Q", "kind": "control", "target": "fontSize", "note": "Font Size box"}]}"#).is_ok());
}
/// A probed command may not write files, whatever its params say.
#[test]
fn probes_cannot_touch_the_file_system() {
let path = std::env::temp_dir().join(format!("keytips-probe-{}.pdf", std::process::id()));
let _ = fs::remove_file(&path);
let calls = vec![("file.exportPdf".to_string(), serde_json::json!({ "path": path.to_string_lossy() }).to_string())];
let probe = probe_live_commands(&calls);
assert_eq!(probe.values().copied().collect::<Vec<_>>(), [2]);
assert!(!path.exists());
}
}