Compare commits

..
Author SHA1 Message Date
Dylan M. Taylor 4a6386b766 xlsx: drop stray worktree entries added by mistake 2026-11-03 12:02:04 +00:00
Dylan M. Taylor f8b80d4eef xlsx: share each custom number format across cell formats, cap font names at 31 characters
Every cell format record copied its font (name included) and built its
own copy of its number-format code, so many short records naming one
long font or code multiplied it in memory. Custom codes are now built
once and shared, and font names are cut to Excel's 31 characters.
2026-11-03 12:01:19 +00:00
4 changed files with 49 additions and 55 deletions
+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(())
}
+19 -10
View File
@@ -59,11 +59,14 @@ fn bool_prop(e: &El, name: &str) -> bool {
const DEFAULT_FONT_FAMILY: u8 = 2;
const MAX_FONT_FAMILY: u8 = 14;
/// Longest font name kept, in characters (Excel's own limit). Every cell format copies its
/// font, so a long name repeated by many formats would otherwise multiply in memory.
const MAX_FONT_NAME: usize = 31;
pub fn read_font(e: &El, palette: &[u32]) -> Font {
let mut f = Font::default();
if let Some(n) = e.child("name").or_else(|| e.child("rFont")).and_then(|n| n.attr("val")) {
f.name = n.to_string();
f.name = n.chars().take(MAX_FONT_NAME).collect();
}
if let Some(sz) = e.child("sz").and_then(|s| s.attr_f64("val")) {
f.size = sz.clamp(1.0, 409.0) as f32;
@@ -277,14 +280,14 @@ fn read_protection(p: &El) -> Protection {
Protection { locked: p.flag("locked", true), hidden: p.flag("hidden", false) }
}
fn num_fmt_code(id: u32, custom: &HashMap<u32, String>) -> NumFmt {
fn num_fmt_code(id: u32, custom: &HashMap<u32, NumFmt>) -> NumFmt {
if let Some(c) = custom.get(&id) {
return NumFmt::new(c);
return c.clone();
}
NumFmt::new(builtin_format(id).unwrap_or("General"))
}
fn read_xf(xf: &El, fonts: &[Font], fills: &[Fill], borders: &[Borders], fmts: &HashMap<u32, String>, checkboxes: &[bool]) -> Style {
fn read_xf(xf: &El, fonts: &[Font], fills: &[Fill], borders: &[Borders], fmts: &HashMap<u32, NumFmt>, checkboxes: &[bool]) -> Style {
let mut s = Style::default();
if let Some(f) = xf.attr_u32("fontId").and_then(|i| fonts.get(i as usize)) {
s.font = f.clone();
@@ -336,6 +339,8 @@ pub fn read_styles(root: &El, checkboxes: &[bool], minor_font: &str) -> StylesIn
}
}
}
// One shared copy of each custom code, which every format using it points to.
let shared_fmts: HashMap<u32, NumFmt> = fmts.iter().map(|(id, code)| (*id, NumFmt::new(code))).collect();
// The scheme link only means something on a font in the body face; elsewhere it stays as a new font has it.
let body_font = |e: &El| {
let mut f = read_font(e, &palette);
@@ -345,10 +350,14 @@ pub fn read_styles(root: &El, checkboxes: &[bool], minor_font: &str) -> StylesIn
let fonts: Vec<Font> = root.child("fonts").map(|f| f.kids("font").map(body_font).collect()).unwrap_or_default();
let fills: Vec<Fill> = root.child("fills").map(|f| f.kids("fill").map(|e| read_fill(e, &palette, false)).collect()).unwrap_or_default();
let borders: Vec<Borders> = root.child("borders").map(|f| f.kids("border").map(|e| read_border(e, &palette)).collect()).unwrap_or_default();
let xfs =
root.child("cellXfs").map(|x| x.kids("xf").map(|xf| read_xf(xf, &fonts, &fills, &borders, &fmts, checkboxes)).collect()).unwrap_or_default();
let style_xfs: Vec<Style> =
root.child("cellStyleXfs").map(|x| x.kids("xf").map(|xf| read_xf(xf, &fonts, &fills, &borders, &fmts, &[])).collect()).unwrap_or_default();
let xfs = root
.child("cellXfs")
.map(|x| x.kids("xf").map(|xf| read_xf(xf, &fonts, &fills, &borders, &shared_fmts, checkboxes)).collect())
.unwrap_or_default();
let style_xfs: Vec<Style> = root
.child("cellStyleXfs")
.map(|x| x.kids("xf").map(|xf| read_xf(xf, &fonts, &fills, &borders, &shared_fmts, &[])).collect())
.unwrap_or_default();
let mut named = Vec::new();
if let Some(cs) = root.child("cellStyles") {
for c in cs.kids("cellStyle") {
@@ -359,12 +368,12 @@ pub fn read_styles(root: &El, checkboxes: &[bool], minor_font: &str) -> StylesIn
}
}
}
let dxfs = root.child("dxfs").map(|d| d.kids("dxf").map(|x| read_dxf(x, &palette, &fmts)).collect()).unwrap_or_default();
let dxfs = root.child("dxfs").map(|d| d.kids("dxf").map(|x| read_dxf(x, &palette, &shared_fmts)).collect()).unwrap_or_default();
StylesIn { xfs, dxfs, palette, named, num_fmts: fmts }
}
/// One differential format (`dxf`): the style it sets over a cell's own.
fn read_dxf(x: &El, palette: &[u32], fmts: &HashMap<u32, String>) -> Style {
fn read_dxf(x: &El, palette: &[u32], fmts: &HashMap<u32, NumFmt>) -> Style {
let mut s = Style::default();
if let Some(e) = x.child("font") {
let f = read_font(e, palette);
+19
View File
@@ -385,3 +385,22 @@ fn outline_summary_directions_round_trip() {
let xml = String::from_utf8(crate::package::Package::open(&bytes).unwrap().read("xl/worksheets/sheet1.xml").unwrap().unwrap()).unwrap();
assert!(!xml.contains("outlinePr"), "{xml}");
}
/// Many cell formats naming one long font and one long custom number format: the code is shared
/// by every format and the font name is cut to Excel's 31 characters, so neither is copied per
/// format.
#[test]
fn cell_formats_share_their_number_format_and_keep_short_font_names() {
let xml = format!(
r#"<styleSheet><numFmts><numFmt numFmtId="164" formatCode="{}"/></numFmts><fonts><font><name val="{}"/></font></fonts><cellXfs>{}</cellXfs></styleSheet>"#,
"0".repeat(100_000),
"F".repeat(100_000),
r#"<xf numFmtId="164" fontId="0"/>"#.repeat(20_000)
);
let root = crate::xml::parse(xml.as_bytes()).unwrap();
let st = crate::styles::read_styles(&root, &[], "Calibri");
assert_eq!(st.xfs.len(), 20_000);
assert!(st.xfs.iter().all(|s| std::sync::Arc::ptr_eq(&s.num_fmt.0, &st.xfs[0].num_fmt.0)));
assert_eq!(st.xfs[0].num_fmt.as_str().len(), 100_000);
assert!(st.xfs.iter().all(|s| s.font.name.chars().count() == 31));
}
+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());
}
}