Make automation "open" run the same finalization as a UI open

Automation's "open" op called io::load_bytes directly and installed
the result as-is, skipping everything the canonical path-based loader
(load_file_for_open / finalize_loaded_outcome) runs after parsing:
block-origin normalization for legacy non-zero-origin blocks,
relative raster/material path resolution, source_path assignment,
and the corrupt-entity purge the UI open path always applies. The
same file opened two ways behaved differently, and headless/automation
use was more exposed to malformed geometry with no filter at all.

Added io::load_bytes_finalized(path, bytes), which runs load_bytes
plus that same normalization and purge using the real path (not just
its basename, which the old code passed to load_bytes as the "name"
parameter — losing the directory needed for raster-path resolution
and source_path). automation.rs's "open" handler now uses it and
reports the purge count as a "purged" field in the response.

load_bytes itself is unchanged, since the byte round-trip tests
(sketch_persist.rs, named_parameters_persist.rs) deliberately want
the simpler byte-only path with no real filesystem path behind it.

New test constructs a document with one normal and one corrupt
(zero-radius) entity, saves it, opens it through automation, and
checks the corrupt entity was purged and source_path was set —
proving the automation path now matches the UI path.

Co-Authored-By: Claude Sonnet 5 <[email protected]>
This commit is contained in:
Felix RiestraandClaude Sonnet 5 committed 2026-09-09 10:13:06 +02:00
1 parent 8bc9e83c2e
commit d0025d6ddf
2 files changed
+89 -9

No files matched your search

+63 -9
View File
@@ -314,16 +314,19 @@ impl OpenCADStudio {
let Some(path) = req["path"].as_str() else {
return err("open: missing \"path\"");
};
let bytes = match self.read_drawing(std::path::Path::new(path)) {
let path_buf = PathBuf::from(path);
let bytes = match self.read_drawing(&path_buf) {
Ok(b) => b,
Err(e) => return err(format!("open: {e}")),
};
let name = PathBuf::from(path)
.file_name()
.map(|n| n.to_string_lossy().into_owned())
.unwrap_or_else(|| path.to_string());
match crate::io::load_bytes(&name, bytes) {
Ok(doc) => {
// Runs the same finalization (block-origin normalization,
// raster-path resolution, source_path) and corrupt-entity
// purge the UI open path always gets, using the real path —
// not just its basename — so an automation open behaves the
// same as opening the identical file from the UI instead of
// silently skipping that safety/normalization net.
match crate::io::load_bytes_finalized(&path_buf, bytes) {
Ok((doc, dropped)) => {
let i = self.active_tab;
self.tabs[i].scene.document = doc;
self.tabs[i].scene.deselect_all();
@@ -331,10 +334,16 @@ impl OpenCADStudio {
&mut self.tabs[i].scene.document,
);
self.tabs[i].adopt_active_ucs_from_header();
self.tabs[i].current_path = Some(PathBuf::from(path));
self.tabs[i].current_path = Some(path_buf);
self.tabs[i].is_start = false;
self.tabs[i].scene.bump_geometry();
self.entity_summary()
let mut summary = self.entity_summary();
if dropped > 0 {
if let Some(obj) = summary.as_object_mut() {
obj.insert("purged".to_string(), json!(dropped));
}
}
summary
}
Err(e) => err(format!("open: {e}")),
}
@@ -1395,6 +1404,51 @@ mod tests {
let _ = std::fs::remove_file(&path);
}
/// Audit finding: automation's `"open"` used to call `io::load_bytes`
/// directly, skipping the finalization and corrupt-entity purge every UI
/// open runs — so the same file opened two ways behaved differently.
/// Proves both halves of the fix: `source_path` (only ever set by
/// finalization, never by `load_bytes` alone) is populated, and a
/// zero-radius circle — `io::is_entity_corrupt`'s own rejection case —
/// is purged and reported, exactly like a UI open of the same file.
#[test]
fn open_finalizes_and_purges_like_the_ui_open_path() {
let mut app = OpenCADStudio::new_for_test();
let path = std::env::temp_dir().join(format!(
"ocs_automation_finalize_test_{}.dxf",
std::process::id()
));
let _ = std::fs::remove_file(&path);
let mut doc = acadrust::CadDocument::new();
let mut good = acadrust::entities::Circle::new();
good.center = acadrust::types::Vector3::new(5.0, 5.0, 0.0);
good.radius = 2.0;
doc.add_entity(acadrust::EntityType::Circle(good)).unwrap();
let mut corrupt = acadrust::entities::Circle::new();
corrupt.center = acadrust::types::Vector3::new(1.0, 1.0, 0.0);
corrupt.radius = 0.0; // io::is_entity_corrupt rejects a zero-radius circle
doc.add_entity(acadrust::EntityType::Circle(corrupt)).unwrap();
let bytes = crate::io::save_to_bytes(&doc, "dxf", doc.version)
.expect("save a document containing a corrupt entity");
std::fs::write(&path, bytes).unwrap();
let p = path.to_string_lossy().replace('\\', "\\\\");
let result = app.automation_op(&format!(r#"{{"op":"open","path":"{p}"}}"#));
assert_eq!(result["ok"], true, "{}", result["error"]);
assert_eq!(result["total"], 1, "the corrupt circle must not survive the open");
assert_eq!(result["purged"], 1, "the purge count must be reported, matching the UI open path's diagnostics");
let i = app.active_tab;
assert!(
app.tabs[i].scene.document.source_path.is_some(),
"automation open must run the same finalization as a path-based open, which sets source_path (load_bytes alone never does)"
);
drop(app);
let _ = std::fs::remove_file(&path);
}
#[test]
fn test_pline_line_then_arc() {
use crate::app::Message;
+26
View File
@@ -744,6 +744,32 @@ pub fn load_bytes(name: &str, bytes: Vec<u8>) -> Result<CadDocument, String> {
}
}
/// [`load_bytes`] plus the same finalization and corruption handling the
/// canonical path-based open (`load_file_for_open`/`finalize_loaded_outcome`)
/// already runs — for a byte-based open that nonetheless has a real
/// filesystem path behind it (automation's `"open"` op reads the file into
/// memory itself, e.g. to go through an edit lease, but the drawing still
/// lives at `path`). Without this, automation-opened documents kept
/// legacy non-zero block origins un-normalized, left relative raster/material
/// paths unresolved, carried no `source_path`, and skipped the corrupt-entity
/// purge the UI open path always runs — silently behaving differently from a
/// UI open of the exact same file. `load_bytes` itself is unchanged (and
/// still used directly by the byte round-trip tests, which have no real path
/// and don't want this finalization).
#[cfg(not(target_arch = "wasm32"))]
pub fn load_bytes_finalized(path: &Path, bytes: Vec<u8>) -> Result<(CadDocument, usize), String> {
let name = path
.file_name()
.map(|n| n.to_string_lossy().into_owned())
.unwrap_or_else(|| path.to_string_lossy().into_owned());
let mut doc = load_bytes(&name, bytes)?;
normalize_block_origins(&mut doc);
resolve_raster_image_paths(&mut doc, path.parent());
doc.source_path = Some(path.to_string_lossy().into_owned());
let dropped = purge_corrupt_entities(&mut doc);
Ok((doc, dropped))
}
/// Load a DWG or DXF file directly from a path (auto-detect by extension).
/// Peek at a file's leading bytes to tell a DWG (version tag "AC10xx") from a
/// DXF. Used for `.bak` copies, whose extension hides the real format.