diff --git a/apps/cadcraft-cli/src/output.rs b/apps/cadcraft-cli/src/output.rs index 5716c486c..83fa17dfd 100644 --- a/apps/cadcraft-cli/src/output.rs +++ b/apps/cadcraft-cli/src/output.rs @@ -122,10 +122,21 @@ pub(crate) fn export_window(s: &Session, framing: Framing, before: Option<&[(u64 /// Write `bytes` to `out` through the engine's atomic writer (an exclusively created sibling file /// renamed over `out`), so a failed or interrupted write leaves the previous file untouched. An /// existing `out` is resolved first: the rename would otherwise replace a symlink with a file and -/// leave the file it points to (`--in-place` through a link) unchanged. -pub(crate) fn write_atomic(out: &Path, bytes: &[u8]) -> Result<(), OutputError> { +/// leave the file it points to (`--in-place` through a link) unchanged. Folders missing above `out` +/// are created, and the topmost one returned for the caller to report. +pub(crate) fn write_atomic(out: &Path, bytes: &[u8]) -> Result, OutputError> { let target = std::fs::canonicalize(out).unwrap_or_else(|_| out.to_path_buf()); - cadcraft_engine::atomic_write::write_atomic(&target, bytes).map_err(|source| OutputError::Write { path: out.to_path_buf(), source }) + let folder = cadcraft_engine::atomic_write::missing_folder(&target); + cadcraft_engine::atomic_write::write_atomic_making_folders(&target, bytes) + .map_err(|source| OutputError::Write { path: out.to_path_buf(), source })?; + Ok(folder) +} + +/// Say that `out` went into a folder that didn't exist, so a mistyped path is noticed. +pub(crate) fn report_new_folder(folder: Option, io: &mut Streams) { + if let Some(folder) = folder { + io.errln(format_args!("created the folder {}, which did not exist", folder.display())); + } } pub(crate) fn export(s: &Session, out: &Path, io: &mut Streams) -> Result { @@ -135,7 +146,8 @@ pub(crate) fn export(s: &Session, out: &Path, io: &mut Streams) -> Result { pub(crate) fn export_framed(s: &Session, out: &Path, window: Option, io: &mut Streams) -> Result { let d = s.doc()?; let bytes = cadcraft_io::write_framed(d, out, window)?; - write_atomic(out, &bytes)?; + let folder = write_atomic(out, &bytes)?; + report_new_folder(folder, io); io.errln(format_args!("wrote {} ({} bytes)", out.display(), bytes.len())); if cadcraft_engine::cmd::file::drops_unknown(out) { let lost = cadcraft_engine::cmd::file::not_saved(d); diff --git a/apps/cadcraft-cli/src/sample.rs b/apps/cadcraft-cli/src/sample.rs index 9b6405cb5..3f93bc5cf 100644 --- a/apps/cadcraft-cli/src/sample.rs +++ b/apps/cadcraft-cli/src/sample.rs @@ -1,6 +1,6 @@ use super::args::{SampleArgs, SampleName}; use super::error::Result; -use super::output::{Streams, write_atomic}; +use super::output::{Streams, report_new_folder, write_atomic}; pub(crate) fn sample(a: &SampleArgs, io: &mut Streams) -> Result { let d = match a.name { @@ -8,7 +8,8 @@ pub(crate) fn sample(a: &SampleArgs, io: &mut Streams) -> Result { SampleName::Floorplan => cadcraft_engine::sample::floor_plan(), }; let bytes = cadcraft_io::write(&d, &a.output)?; - write_atomic(&a.output, &bytes)?; + let folder = write_atomic(&a.output, &bytes)?; + report_new_folder(folder, io); io.errln(format_args!("wrote {} ({} bytes)", a.output.display(), bytes.len())); Ok(()) } diff --git a/crates/engine/src/atomic_write.rs b/crates/engine/src/atomic_write.rs index 0351cef34..2ec83b006 100644 --- a/crates/engine/src/atomic_write.rs +++ b/crates/engine/src/atomic_write.rs @@ -42,6 +42,27 @@ pub fn write_atomic(path: &Path, bytes: &[u8]) -> io::Result<()> { write_atomic_with(path, permissions, |file| file.write_all(bytes)) } +/// The topmost folder missing above `path`: what [`write_atomic_making_folders`] will create. +/// `None` when the file's folder exists, or when `path` itself exists (a dangling symlink's target +/// folder is not ours to make). +pub fn missing_folder(path: &Path) -> Option { + if std::fs::symlink_metadata(path).is_ok() { + return None; + } + path.ancestors().skip(1).filter(|a| !a.as_os_str().is_empty()).take_while(|a| std::fs::symlink_metadata(a).is_err()).last().map(Path::to_path_buf) +} + +/// [`write_atomic`] to a path the user named, first creating the folders missing above it: a +/// mistyped folder costs less than a drawing that could not be written. The caller reports +/// [`missing_folder`], asked before this call, so the new folder is never a surprise. +pub fn write_atomic_making_folders(path: &Path, bytes: &[u8]) -> io::Result<()> { + if let Some(folder) = missing_folder(path) { + let dir = path.parent().unwrap_or(&folder); + std::fs::create_dir_all(dir).map_err(|e| io::Error::new(e.kind(), format!("the folder {} could not be created: {e}", dir.display())))?; + } + write_atomic(path, bytes) +} + /// [`write_atomic`] filling the temporary file through `fill` (streamed rather than one buffer), /// with `permissions` given to it from its creation (`None`: the usual creation mode). A symlink /// at `path` is replaced by the new file, never written through. @@ -159,6 +180,30 @@ fn create_exclusive(path: &Path, permissions: Option<&Permissions>) -> io::Resul mod tests { use super::*; + #[test] + fn creates_missing_parent_directories() { + let dir = std::env::temp_dir().join(format!("cadcraft-aw-mkdir-{}-{}", std::process::id(), SEQ.fetch_add(1, Ordering::Relaxed))); + let dest = dir.join("new_folder").join("deeper").join("out.bin"); + std::fs::create_dir_all(&dir).unwrap(); + assert_eq!(missing_folder(&dest), Some(dir.join("new_folder"))); + assert!(write_atomic(&dest, b"created").is_err(), "the plain writer never makes folders"); + write_atomic_making_folders(&dest, b"created").unwrap(); + assert_eq!(std::fs::read(&dest).unwrap(), b"created"); + assert_eq!(missing_folder(&dest), None); + std::fs::remove_dir_all(&dir).ok(); + } + + #[test] + fn a_folder_that_cannot_be_made_is_named_in_the_error() { + let dir = std::env::temp_dir().join(format!("cadcraft-aw-nomk-{}-{}", std::process::id(), SEQ.fetch_add(1, Ordering::Relaxed))); + std::fs::create_dir_all(&dir).unwrap(); + let blocker = dir.join("a_file"); + std::fs::write(&blocker, b"x").unwrap(); + let e = write_atomic_making_folders(&blocker.join("sub").join("out.bin"), b"new").unwrap_err(); + assert!(e.to_string().contains("could not be created"), "{e}"); + std::fs::remove_dir_all(&dir).ok(); + } + #[test] fn replaces_contents_atomically() { let dir = std::env::temp_dir().join(format!("cadcraft-aw-{}-{}", std::process::id(), SEQ.fetch_add(1, Ordering::Relaxed))); @@ -259,6 +304,8 @@ mod tests { let link = dir.join("link.dxf"); symlink("missing/real.dxf", &link).unwrap(); assert!(write_atomic(&link, b"new").is_err()); + assert!(write_atomic_making_folders(&link, b"new").is_err()); + assert!(!dir.join("missing").exists(), "a link's unavailable target folder must not be created"); assert!(std::fs::symlink_metadata(&link).unwrap().file_type().is_symlink()); std::fs::remove_dir_all(dir).unwrap(); } diff --git a/crates/engine/src/cmd/blocks/wblock.rs b/crates/engine/src/cmd/blocks/wblock.rs index fb03fe1af..373e471bf 100644 --- a/crates/engine/src/cmd/blocks/wblock.rs +++ b/crates/engine/src/cmd/blocks/wblock.rs @@ -8,7 +8,7 @@ use cadcraft_geom::{Mat3, Vec2, Vec3}; use serde_json::{Value, json}; use super::super::curves::KW; -use super::super::file::{drops_unknown, not_saved, not_saved_message, report_not_saved}; +use super::super::file::{drops_unknown, new_folder, not_saved, not_saved_message, report_new_folder, report_not_saved}; use super::super::machines::{SelOutcome, SelectPhase}; use super::super::modify::erase_kept; use super::super::{CommandSpec, bad, point_param, str_param, targets}; @@ -43,11 +43,13 @@ enum Source<'a> { fn run_wblock(s: &mut Session, p: &Value) -> Result { let path = str_param(p, "path").ok_or_else(|| bad("wblock", "`path` is required"))?.to_string(); let base = point_param(p, "base"); + let folder = new_folder(&path); let (entities, bytes, lost) = match str_param(p, "name") { Some(name) => write(s, "wblock", &path, Source::Block(name, base))?, None => write(s, "wblock", &path, Source::Objects(&targets(s, p)?, base.unwrap_or(Vec2::ZERO)))?, }; let mut r = json!({ "path": path, "entities": entities, "bytes": bytes }); + report_new_folder(s, &mut r, folder); report_not_saved(s, &mut r, &lost); Ok(r) } @@ -58,6 +60,7 @@ fn run_dash_wblock(s: &mut Session, p: &Value) -> Result { } let path = str_param(p, "path").ok_or_else(|| bad("-wblock", "`path` is required"))?.to_string(); let base = point_param(p, "base"); + let folder = new_folder(&path); let (entities, bytes, lost) = match str_param(p, "name") { Some(name) => write(s, "-wblock", &path, Source::Block(name, base))?, None => { @@ -72,6 +75,7 @@ fn run_dash_wblock(s: &mut Session, p: &Value) -> Result { } }; let mut r = json!({ "path": path, "entities": entities, "bytes": bytes }); + report_new_folder(s, &mut r, folder); report_not_saved(s, &mut r, &lost); Ok(r) } @@ -101,7 +105,7 @@ fn write(s: &Session, cmd: &str, path: &str, what: Source) -> Result<(usize, usi let bytes = (hooks.write)(&d, path).map_err(|e| bad(cmd, e))?; // Through a temporary file, as SAVE writes, so a failed write keeps the file it replaces. #[cfg(not(target_arch = "wasm32"))] - crate::atomic_write::write_atomic(std::path::Path::new(path), &bytes).map_err(|e| bad(cmd, format!("{path}: {e}")))?; + crate::atomic_write::write_atomic_making_folders(std::path::Path::new(path), &bytes).map_err(|e| bad(cmd, format!("{path}: {e}")))?; let lost = if drops_unknown(path) { not_saved(&d) } else { BTreeMap::new() }; Ok((d.model.len(), bytes.len(), lost)) } diff --git a/crates/engine/src/cmd/exchange.rs b/crates/engine/src/cmd/exchange.rs index 1be19a408..2a6a01f67 100644 --- a/crates/engine/src/cmd/exchange.rs +++ b/crates/engine/src/cmd/exchange.rs @@ -82,7 +82,7 @@ fn write_file(cmd: &str, path: &str, bytes: &[u8]) -> Result<()> { { // Exclusively create a fresh temporary file and rename it into place so a planted // `.cadcraft-tmp` symlink cannot redirect the export write (CWE-59). - crate::atomic_write::write_atomic(std::path::Path::new(path), bytes).map_err(|e| bad(cmd, format!("{path}: {e}"))) + crate::atomic_write::write_atomic_making_folders(std::path::Path::new(path), bytes).map_err(|e| bad(cmd, format!("{path}: {e}"))) } #[cfg(target_arch = "wasm32")] { @@ -119,6 +119,7 @@ fn run_export(s: &mut Session, p: &Value) -> Result { } let file = format!("export.{format}"); let bytes = (hooks.write)(s.doc()?, &file).map_err(|e| bad("export", e))?; + let folder = path.and_then(super::file::new_folder); let mut r = match path { Some(path) => { write_file("export", path, &bytes)?; @@ -126,6 +127,7 @@ fn run_export(s: &mut Session, p: &Value) -> Result { } None => json!({ "data": base64_encode(&bytes), "format": format, "bytes": bytes.len() }), }; + super::file::report_new_folder(s, &mut r, folder); super::file::report_loss(s, &mut r, &file); Ok(r) } diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 012a1d381..72d4ef692 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -446,7 +446,7 @@ fn store(path: &str, bytes: &[u8]) -> Result<()> { } // Exclusively create a fresh temporary file and rename it over `path`, so a planted // `.cadcraft-tmp` symlink cannot redirect the write onto another file (CWE-59). - crate::atomic_write::write_atomic(std::path::Path::new(path), bytes).map_err(|e| bad("save", format!("{path}: {e}"))) + crate::atomic_write::write_atomic_making_folders(std::path::Path::new(path), bytes).map_err(|e| bad("save", format!("{path}: {e}"))) } #[cfg(target_arch = "wasm32")] { @@ -851,6 +851,28 @@ fn report_sat_text_dropped(s: &mut Session, r: &mut Value) { } } +/// The folder a write to `path` has to create, asked before writing so it can be reported after. +/// The web has no folders. +pub(super) fn new_folder(path: &str) -> Option { + #[cfg(not(target_arch = "wasm32"))] + return crate::atomic_write::missing_folder(std::path::Path::new(path)).map(|f| f.display().to_string()); + #[cfg(target_arch = "wasm32")] + { + let _ = path; + None + } +} + +/// Say on the command line that the file went into a folder that didn't exist, so a mistyped path +/// is noticed, and name the folder in the result's `createdFolder`. +pub(super) fn report_new_folder(s: &mut Session, r: &mut Value, folder: Option) { + let Some(folder) = folder else { return }; + s.echo(format!("Created the folder {folder}, which did not exist. Check the path if you did not mean to.")); + if let Some(o) = r.as_object_mut() { + o.insert("createdFolder".into(), json!(folder)); + } +} + /// Say on the command line what the file couldn't keep, and list it in the result's `notSaved`. pub(super) fn report_not_saved(s: &mut Session, r: &mut Value, lost: &std::collections::BTreeMap) { let Some(msg) = not_saved_message(lost) else { return }; @@ -867,7 +889,9 @@ fn save_reporting(s: &mut Session, p: &Value, run: fn(&mut Session, &Value) -> R let path = str_param(p, "path").map(str::to_string).or_else(|| s.state().ok().and_then(|st| st.path.clone())).unwrap_or_default(); let version = asked.unwrap_or_else(|| drawing_version(s, &path)); let lost = s.doc().map(|d| not_saved_as(d, version)).unwrap_or_default(); + let folder = new_folder(&path); let mut r = run(s, p)?; + report_new_folder(s, &mut r, folder); if r.get("path").and_then(Value::as_str).is_some_and(drops_unknown) { report_not_saved(s, &mut r, &lost); } else if version >= Version::R2013 && r.get("path").and_then(Value::as_str).is_some_and(is_dxf_file) { diff --git a/crates/engine/src/cmd/layout.rs b/crates/engine/src/cmd/layout.rs index 0c78e19cb..ba5e711e4 100644 --- a/crates/engine/src/cmd/layout.rs +++ b/crates/engine/src/cmd/layout.rs @@ -840,8 +840,11 @@ fn run_plot(s: &mut Session, p: &Value, cmd: &str) -> Result { let path = _path; // Exclusively create a fresh temporary file and rename it into place so a planted // `.cadcraft-tmp` symlink cannot redirect the plot write (CWE-59). - crate::atomic_write::write_atomic(std::path::Path::new(path), &bytes).map_err(|e| bad(cmd, format!("{path}: {e}")))?; - Ok(json!({ "path": path, "bytes": bytes.len(), "layout": layout, "pages": pages })) + let folder = super::file::new_folder(path); + crate::atomic_write::write_atomic_making_folders(std::path::Path::new(path), &bytes).map_err(|e| bad(cmd, format!("{path}: {e}")))?; + let mut r = json!({ "path": path, "bytes": bytes.len(), "layout": layout, "pages": pages }); + super::file::report_new_folder(s, &mut r, folder); + Ok(r) } #[cfg(target_arch = "wasm32")] { diff --git a/crates/engine/src/cmd/layout/saveas.rs b/crates/engine/src/cmd/layout/saveas.rs index b6d3f6873..0a0b2b5f8 100644 --- a/crates/engine/src/cmd/layout/saveas.rs +++ b/crates/engine/src/cmd/layout/saveas.rs @@ -104,8 +104,10 @@ pub(super) fn run_saveas(s: &mut Session, p: &Value) -> Result { let t = template_of(d, &name)?; let hooks = io().ok_or_else(|| bad("layout.saveas", "file formats are not available in this build"))?; let bytes = (hooks.write)(&t, &path).map_err(|e| bad("layout.saveas", e))?; + let folder = super::super::file::new_folder(&path); store(&path, &bytes)?; let mut r = json!({ "name": name, "path": path, "bytes": bytes.len() }); + super::super::file::report_new_folder(s, &mut r, folder); let lost = not_saved(&t); if drops_unknown(&path) && let Some(msg) = not_saved_message(&lost) @@ -124,7 +126,7 @@ fn store(path: &str, bytes: &[u8]) -> Result<()> { { // Exclusively create a fresh temporary file and rename it into place so a planted // `.cadcraft-tmp` symlink cannot redirect the write (CWE-59). - crate::atomic_write::write_atomic(std::path::Path::new(path), bytes).map_err(|e| bad("layout.saveas", format!("{path}: {e}"))) + crate::atomic_write::write_atomic_making_folders(std::path::Path::new(path), bytes).map_err(|e| bad("layout.saveas", format!("{path}: {e}"))) } #[cfg(target_arch = "wasm32")] { diff --git a/crates/engine/tests/save_creates_missing_folders.rs b/crates/engine/tests/save_creates_missing_folders.rs new file mode 100644 index 000000000..4a44e58c6 --- /dev/null +++ b/crates/engine/tests/save_creates_missing_folders.rs @@ -0,0 +1,62 @@ +//! A save to a folder that doesn't exist writes the file anyway and says what it created, so a +//! mistyped path costs a stray folder, not the drawing (#2215). + +use cadcraft_engine::Session; +use cadcraft_engine::cmd::file::{IoHooks, set_io}; +use cadcraft_engine::doc::Drawing; +use serde_json::json; +use std::path::PathBuf; + +fn scratch(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!("cadcraft-new-folder-{}-{name}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + dir +} + +fn session() -> Session { + set_io(IoHooks { read: |_, _| Err("no reader in this test".into()), write: |_, _| Ok(b"0\nEOF\n".to_vec()), plot: None }); + let mut s = Session::empty(); + s.open_drawing(Drawing::new_imperial(), "a.dxf", None); + s +} + +#[test] +fn saveas_and_export_write_the_file_and_report_the_new_folder() { + let dir = scratch("cmds"); + let mut s = session(); + for (cmd, name) in [("saveas", "one"), ("export", "two")] { + let top = dir.join(name); + let path = top.join("deeper").join("a.dxf"); + let r = s.execute(cmd, &json!({ "path": path.to_string_lossy() })).unwrap(); + assert!(path.is_file(), "{cmd} wrote no file"); + assert_eq!(r["createdFolder"], json!(top.to_string_lossy()), "{cmd}"); + let said = top.display().to_string(); + assert!(s.log.iter().any(|l| l.contains(&said) && l.starts_with("Created the folder")), "{cmd}: {:?}", s.log); + } + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn a_save_into_an_existing_folder_reports_nothing() { + let dir = scratch("quiet"); + let mut s = session(); + let path = dir.join("a.dxf"); + let r = s.execute("saveas", &json!({ "path": path.to_string_lossy() })).unwrap(); + assert!(r.get("createdFolder").is_none()); + assert!(!s.log.iter().any(|l| l.starts_with("Created the folder")), "{:?}", s.log); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn a_folder_that_cannot_be_made_fails_and_says_so() { + let dir = scratch("blocked"); + let blocker = dir.join("a_file"); + std::fs::write(&blocker, b"x").unwrap(); + let mut s = session(); + let path = blocker.join("sub").join("a.dxf"); + let e = s.execute("saveas", &json!({ "path": path.to_string_lossy() })).unwrap_err().to_string(); + assert!(e.contains("could not be created"), "{e}"); + assert!(!s.log.iter().any(|l| l.starts_with("Created the folder")), "{:?}", s.log); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/crates/mcp/src/backend.rs b/crates/mcp/src/backend.rs index 1a36144ca..913569e82 100644 --- a/crates/mcp/src/backend.rs +++ b/crates/mcp/src/backend.rs @@ -415,10 +415,17 @@ impl Backend for Headless { let (w, h) = (view.width, view.height); let png = cadcraft_render::raster::render_png(&list, &view, &cadcraft_render::raster::RasterOptions::default()).ok_or("render failed")?; + let mut r = + json!({"pngBase64": cadcraft_engine::cmd::file::base64_encode(&png), "width": w, "height": h, "truncated": list.truncated}); if let Some(path) = str_p("path") { - std::fs::write(&path, &png).map_err(|e| format!("{path}: {e}"))?; + let file = std::path::Path::new(&path); + let folder = cadcraft_engine::atomic_write::missing_folder(file); + cadcraft_engine::atomic_write::write_atomic_making_folders(file, &png).map_err(|e| format!("{path}: {e}"))?; + if let Some(folder) = folder { + r["createdFolder"] = json!(folder.display().to_string()); + } } - Ok(json!({"pngBase64": cadcraft_engine::cmd::file::base64_encode(&png), "width": w, "height": h, "truncated": list.truncated})) + Ok(r) } "app.open" => { let path = str_p("path").ok_or("missing path")?; diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 4abc6b1f4..9c806a96f 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -489,10 +489,16 @@ pub fn handle(app: &mut CadApp, ctx: &egui::Context, req: &ControlRequest) -> Ou }; #[cfg(not(target_arch = "wasm32"))] if let Some(path) = s("path") { - if let Err(e) = std::fs::write(path, &png) { + let file = std::path::Path::new(&path); + let folder = cadcraft_engine::atomic_write::missing_folder(file); + if let Err(e) = cadcraft_engine::atomic_write::write_atomic_making_folders(file, &png) { return err(e); } - return ok(render_reply(&png, w, h, Some(path), list.truncated)); + let mut reply = render_reply(&png, w, h, Some(path), list.truncated); + if let Some(folder) = folder { + reply["createdFolder"] = json!(folder.display().to_string()); + } + return ok(reply); } ok(render_reply(&png, w, h, None, list.truncated)) }