Merge pull request #879 from marcoarnulfo:fix/unknown-save-extension

crates/engine/src/cmd/file.rs

@@ -75,7 +75,11 @@ fn open(s: &mut Session, p: &Json) -> Result<Json> {
     {
         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,11 +109,16 @@ fn save(s: &mut Session, p: &Json) -> Result<Json> {
         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)?;
-    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!(kind, Some(crate::io::FileKind::Xlsx | crate::io::FileKind::Json)) {
         d.saved = d.wb.clone();
         d.path = Some(path.clone());
     }

crates/engine/src/io.rs

@@ -156,8 +156,19 @@ 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<Vec<u8>> {
+    // 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<Vec<u8>> {
     let sheet = wb.active_sheet;
-    match FileKind::from_path(path).unwrap_or(FileKind::Xlsx) {
+    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())),

crates/engine/src/lib.rs

@@ -108,6 +108,9 @@ pub struct DocState {
     pub sheet_selections: Vec<Selection>,
     /// 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<io::FileKind>,
 }
 
 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 {

crates/engine/src/tests.rs

@@ -1590,6 +1590,65 @@ 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();
+}
+
+/// 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(&notes, "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(&notes).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();