From 9bb85ab9af88d86c6c33feb41132ab84d11591c9 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Tue, 13 Oct 2026 04:34:03 +0200 Subject: [PATCH 1/2] Refuse to save to an unknown file extension instead of writing XLSX under it (fixes #878) --- crates/engine/src/cmd/file.rs | 3 +-- crates/engine/src/io.rs | 8 +++++++- crates/engine/src/tests.rs | 25 +++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 3c4a101..84ca32b 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -107,9 +107,8 @@ fn save(s: &mut Session, p: &Json) -> Result { let d = s.doc_mut()?; let bytes = crate::io::save_bytes(&d.wb, &path)?; crate::io::write_file(&path, &bytes)?; - let kind = crate::io::FileKind::from_path(&path).unwrap_or(crate::io::FileKind::Xlsx); // Only a full-fidelity format clears the dirty flag and becomes the document's file. - if matches!(kind, crate::io::FileKind::Xlsx | crate::io::FileKind::Json) { + if matches!(crate::io::FileKind::from_path(&path), Some(crate::io::FileKind::Xlsx | crate::io::FileKind::Json)) { d.saved = d.wb.clone(); d.path = Some(path.clone()); } diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index bc7011d..34adece 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -154,7 +154,13 @@ fn rename_first_sheet(wb: &mut Workbook, path: &str) { /// Encodes a workbook in the format chosen by `path`'s extension. pub fn save_bytes(wb: &Workbook, path: &str) -> Result> { let sheet = wb.active_sheet; - match FileKind::from_path(path).unwrap_or(FileKind::Xlsx) { + // An unknown or missing extension is refused rather than written as an XLSX under that name: + // a typo like `.cvs` would otherwise produce a file no other app recognises. + let Some(kind) = FileKind::from_path(path) else { + let name = std::path::Path::new(path).file_name().and_then(|n| n.to_str()).unwrap_or(path); + return Err(EngineError::Other(format!("{name}: unknown file format. Save as .xlsx, .csv, .tsv, .json or .html."))); + }; + match kind { // The extension picks the package kind; only the macro-enabled kinds (.xlsm, .xltm) carry // the VBA project, so any other name saves without it, as Excel does. FileKind::Xlsx => gridcraft_xlsx::write_xlsx_as(wb, gridcraft_xlsx::XlsxKind::from_path(path)).map_err(|e| EngineError::Other(e.to_string())), diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 33fdc11..99a8d8c 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1519,6 +1519,31 @@ fn saving_an_opened_semicolon_csv_keeps_semicolons() { std::fs::remove_dir_all(dir).unwrap(); } +/// A typo like `report.cvs` must not quietly write an XLSX under that name and adopt it as the +/// document's file. +#[test] +#[cfg(not(target_arch = "wasm32"))] +fn saving_to_an_unknown_extension_is_refused() { + #[allow(clippy::disallowed_methods)] // native-only test: a unique temp dir name + let nonce = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos(); + let dir = std::env::temp_dir().join(format!("gridcraft-unknown-ext-{}-{nonce}", std::process::id())); + std::fs::create_dir(&dir).unwrap(); + let mut s = s(); + s.execute("cell.set", json!({"cell": "A1", "input": "1"})).unwrap(); + for name in ["report.cvs", "report"] { + let path = dir.join(name); + let e = s.execute("file.save", json!({"path": path})).unwrap_err().to_string(); + assert!(e.contains(".xlsx"), "{name}: {e}"); + assert!(!path.exists(), "{name} was written"); + } + let d = s.doc().unwrap(); + assert!(d.path.is_none()); + assert!(d.is_dirty()); + assert!(s.execute("file.saveBytes", json!({"format": "foo"})).is_err()); + s.execute("file.save", json!({"path": dir.join("report.csv")})).unwrap(); + std::fs::remove_dir_all(dir).unwrap(); +} + #[test] fn validation_formulas_are_relative_and_see_the_new_value() { let mut s = s(); From 522ede0fca8f40144a6b92aca3518ec1568931f9 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Tue, 13 Oct 2026 05:14:30 +0200 Subject: [PATCH 2/2] Save an XLSX opened under an unknown extension back to its own file as XLSX (#878) --- crates/engine/src/cmd/file.rs | 16 +++++++++++++--- crates/engine/src/io.rs | 7 ++++++- crates/engine/src/lib.rs | 4 ++++ crates/engine/src/tests.rs | 34 ++++++++++++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 84ca32b..2a8b435 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -75,7 +75,11 @@ fn open(s: &mut Session, p: &Json) -> Result { { s.close_document(0); } - let i = s.add_document(DocState::new(wb, path, title)); + let mut doc = DocState::new(wb, path, title); + if crate::io::FileKind::from_path(&name).is_none() && gridcraft_xlsx::sniff(&bytes) == gridcraft_xlsx::Format::Xlsx { + doc.content_kind = Some(crate::io::FileKind::Xlsx); + } + let i = s.add_document(doc); // The decrypted-workbook notice is for the info bar, not a message box. let shown: Vec<&str> = warnings.iter().map(String::as_str).filter(|w| *w != crate::io::DECRYPTED_NOTICE).collect(); if !shown.is_empty() { @@ -105,10 +109,16 @@ fn save(s: &mut Session, p: &Json) -> Result { return super::print::export_pdf(s, &json!({"path": path})); } let d = s.doc_mut()?; - let bytes = crate::io::save_bytes(&d.wb, &path)?; + // The document's own file keeps the format its content had when opened; any other path + // goes by its extension. + let kind = crate::io::FileKind::from_path(&path).or(d.content_kind.filter(|_| d.path.as_deref() == Some(path.as_str()))); + let bytes = match kind { + Some(kind) => crate::io::save_bytes_as(&d.wb, &path, kind)?, + None => crate::io::save_bytes(&d.wb, &path)?, + }; crate::io::write_file(&path, &bytes)?; // Only a full-fidelity format clears the dirty flag and becomes the document's file. - if matches!(crate::io::FileKind::from_path(&path), Some(crate::io::FileKind::Xlsx | crate::io::FileKind::Json)) { + if matches!(kind, Some(crate::io::FileKind::Xlsx | crate::io::FileKind::Json)) { d.saved = d.wb.clone(); d.path = Some(path.clone()); } diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 34adece..8ed9f37 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -153,13 +153,18 @@ fn rename_first_sheet(wb: &mut Workbook, path: &str) { /// Encodes a workbook in the format chosen by `path`'s extension. pub fn save_bytes(wb: &Workbook, path: &str) -> Result> { - let sheet = wb.active_sheet; // An unknown or missing extension is refused rather than written as an XLSX under that name: // a typo like `.cvs` would otherwise produce a file no other app recognises. let Some(kind) = FileKind::from_path(path) else { let name = std::path::Path::new(path).file_name().and_then(|n| n.to_str()).unwrap_or(path); return Err(EngineError::Other(format!("{name}: unknown file format. Save as .xlsx, .csv, .tsv, .json or .html."))); }; + save_bytes_as(wb, path, kind) +} + +/// Encodes a workbook as `kind`, whatever `path`'s extension says. +pub fn save_bytes_as(wb: &Workbook, path: &str, kind: FileKind) -> Result> { + let sheet = wb.active_sheet; match kind { // The extension picks the package kind; only the macro-enabled kinds (.xlsm, .xltm) carry // the VBA project, so any other name saves without it, as Excel does. diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 3aceab0..5a2e3df 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -108,6 +108,9 @@ pub struct DocState { pub sheet_selections: Vec, /// Allow Edit Ranges unlocked with their password in this session: (sheet name, title). pub unlocked_ranges: Vec<(String, String)>, + /// The format `path`'s content was recognised as when its extension doesn't name one (an + /// XLSX called `budget.dat`): Save writes it back in that format. + pub content_kind: Option, } static NEXT_UID: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1); @@ -132,6 +135,7 @@ impl DocState { title, sheet_selections: vec![], unlocked_ranges: vec![], + content_kind: None, } } pub fn is_dirty(&self) -> bool { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 99a8d8c..3b963e8 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1544,6 +1544,40 @@ fn saving_to_an_unknown_extension_is_refused() { std::fs::remove_dir_all(dir).unwrap(); } +/// An XLSX opened by its content under an unknown extension (`budget.dat`) saves back to its own +/// file as XLSX; a Save As to another unknown extension is still refused. +#[test] +#[cfg(not(target_arch = "wasm32"))] +fn an_xlsx_opened_under_an_unknown_extension_saves_back_as_xlsx() { + #[allow(clippy::disallowed_methods)] // native-only test: a unique temp dir name + let nonce = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos(); + let dir = std::env::temp_dir().join(format!("gridcraft-dat-{}-{nonce}", std::process::id())); + std::fs::create_dir(&dir).unwrap(); + let path = dir.join("budget.dat"); + let mut src = s(); + src.execute("cell.set", json!({"cell": "A1", "input": "1"})).unwrap(); + std::fs::write(&path, crate::io::save_bytes(&src.doc().unwrap().wb, "book.xlsx").unwrap()).unwrap(); + let mut s = s(); + s.execute("file.open", json!({"path": path})).unwrap(); + s.execute("cell.set", json!({"cell": "A1", "input": "2"})).unwrap(); + s.execute("file.save", json!({})).unwrap(); + assert!(!s.doc().unwrap().is_dirty()); + let bytes = std::fs::read(&path).unwrap(); + assert_eq!(gridcraft_xlsx::sniff(&bytes), gridcraft_xlsx::Format::Xlsx); + let (wb, _) = crate::io::open_bytes("book.xlsx", &bytes).unwrap(); + assert_eq!(wb.active().unwrap().value(CellRef::parse("A1").unwrap()), Value::Number(2.0)); + assert!(s.execute("file.saveAs", json!({"path": dir.join("copy.foo")})).is_err()); + assert!(!dir.join("copy.foo").exists()); + // A text file under an unknown extension opens as CSV: Save doesn't turn it into an XLSX. + let notes = dir.join("notes.dat"); + std::fs::write(¬es, "a,b\r\n").unwrap(); + s.execute("file.open", json!({"path": notes})).unwrap(); + s.execute("cell.set", json!({"cell": "A1", "input": "c"})).unwrap(); + assert!(s.execute("file.save", json!({})).is_err()); + assert_eq!(std::fs::read_to_string(¬es).unwrap(), "a,b\r\n"); + std::fs::remove_dir_all(dir).unwrap(); +} + #[test] fn validation_formulas_are_relative_and_see_the_new_value() { let mut s = s();