From 972ae7888cdeccfa3054b76c41518b5f9c22dbbd Mon Sep 17 00:00:00 2001 From: aiongg Date: Fri, 16 Oct 2026 16:07:16 +0000 Subject: [PATCH 1/4] Close control connections on the first line that is not a request; bound idle time, writes and queued requests A line that is not a JSON object with a string method, including an HTTP request line, gets one error reply and the connection closes before anything after it is read. Connections time out after 10 minutes idle or 30 seconds blocked on a write, and at most 64 requests wait for the app; more are answered busy. --- apps/cadcraft/src/control_server.rs | 300 ++++++++++++++++++++++------ docs/control-protocol.md | 9 + 2 files changed, 248 insertions(+), 61 deletions(-) diff --git a/apps/cadcraft/src/control_server.rs b/apps/cadcraft/src/control_server.rs index c7753ebe..4433d76e 100644 --- a/apps/cadcraft/src/control_server.rs +++ b/apps/cadcraft/src/control_server.rs @@ -1,41 +1,75 @@ //! Loopback JSON-lines control server: one request per line, one reply per line. //! This is the transport the MCP server (`cadcraft-cli mcp --connect`) wraps. //! -//! Lines are bounded ([`MAX_LINE`]): a longer one is refused and the connection closed, without -//! buffering the rest. So is the number of connections served at once ([`MAX_CONNECTIONS`]). +//! The server only reads requests: a line that is not a JSON object with a string `method` (an +//! HTTP request line, a stray `nc`, binary data) gets one error reply and the connection is +//! closed, so nothing sent after it runs. Every resource a client can hold is bounded +//! ([`Limits`]): simultaneous connections, request line length, queued requests, and how long a +//! socket may sit idle or blocked on a write. -use std::io::{BufRead, BufReader, Read, Write}; +use std::io::{BufRead, BufReader, ErrorKind, Read, Write}; use std::net::{TcpListener, TcpStream}; use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering}; -use std::sync::mpsc::{Receiver, Sender, channel}; +use std::sync::mpsc::{Receiver, SyncSender, TrySendError, sync_channel}; use std::time::Duration; use cadcraft_ui_egui::ControlRequest; use serde_json::{Value, json}; -/// Longest accepted request line (bytes, without the newline). Generous, so requests that carry -/// whole drawings or images as base64 still fit. -const MAX_LINE: usize = 256 * 1024 * 1024; -/// Connections served at once; further ones get an error line and are closed. -const MAX_CONNECTIONS: usize = 16; +/// Resource bounds for the control server. +#[derive(Clone, Copy, Debug)] +struct Limits { + /// Connections served at once; more get an error line and are closed. + max_clients: usize, + /// Longest request line in bytes (newline excluded); a longer one closes the connection. + max_line: usize, + /// Requests waiting for the app at once; more are answered `busy`. + queue: usize, + /// How long a connection may go without sending a byte before it is closed. + idle_timeout: Duration, + /// How long a reply write may block before the connection is closed. + write_timeout: Duration, + /// How long a request waits for the app's answer. + reply_timeout: Duration, +} + +impl Default for Limits { + fn default() -> Self { + Self { + max_clients: 16, + // Generous, so requests that carry whole drawings or images as base64 still fit. + max_line: 256 * 1024 * 1024, + queue: 64, + idle_timeout: Duration::from_secs(600), + write_timeout: Duration::from_secs(30), + reply_timeout: Duration::from_secs(60), + } + } +} pub fn start(port: u16, ctx: egui::Context) -> Receiver { - let (tx, rx) = channel::(); - let listener = match TcpListener::bind(("127.0.0.1", port)) { - Ok(l) => l, + match TcpListener::bind(("127.0.0.1", port)) { + Ok(l) => { + log::info!("control server listening on 127.0.0.1:{port}"); + serve_listener(l, Limits::default(), ctx) + } Err(e) => { log::error!("control server failed to bind 127.0.0.1:{port}: {e}"); - return rx; + sync_channel::(0).1 } - }; - log::info!("control server listening on 127.0.0.1:{port}"); - std::thread::spawn(move || accept_loop(listener, tx, ctx)); + } +} + +/// Accept connections on `listener` under `limits`; requests arrive on the returned receiver. +fn serve_listener(listener: TcpListener, limits: Limits, ctx: egui::Context) -> Receiver { + let (tx, rx) = sync_channel::(limits.queue.max(1)); + std::thread::spawn(move || accept_loop(listener, tx, ctx, limits)); rx } -/// A connection's place among [`MAX_CONNECTIONS`], given back when the connection ends (also when -/// its thread can't be started, or panics). +/// A connection's place among [`Limits::max_clients`], given back when the connection ends (also +/// when its thread can't be started, or panics). struct Slot(Arc); impl Drop for Slot { @@ -44,10 +78,11 @@ impl Drop for Slot { } } -fn accept_loop(listener: TcpListener, tx: Sender, ctx: egui::Context) { +fn accept_loop(listener: TcpListener, tx: SyncSender, ctx: egui::Context, limits: Limits) { let open = Arc::new(AtomicUsize::new(0)); for mut stream in listener.incoming().flatten() { - if open.fetch_add(1, Ordering::SeqCst) >= MAX_CONNECTIONS { + let _ = stream.set_write_timeout(Some(limits.write_timeout)); + if open.fetch_add(1, Ordering::SeqCst) >= limits.max_clients { open.fetch_sub(1, Ordering::SeqCst); let _ = writeln!(stream, "{}", json!({"ok": false, "error": "too many control connections"})); continue; @@ -57,7 +92,7 @@ fn accept_loop(listener: TcpListener, tx: Sender, ctx: egui::Con let ctx = ctx.clone(); let _ = std::thread::Builder::new().spawn(move || { let _slot = slot; - serve(stream, tx, ctx); + serve(stream, &tx, &ctx, limits); }); } } @@ -68,7 +103,11 @@ fn read_bounded_line(reader: &mut impl BufRead, buf: &mut Vec, max: usize) - buf.clear(); // `take` bounds the bytes buffered for one line: a peer can't make us allocate more than `max`. let limit = u64::try_from(max).unwrap_or(u64::MAX).saturating_add(1); - let n = reader.take(limit).read_until(b'\n', buf).map_err(|e| e.to_string())?; + let n = reader.take(limit).read_until(b'\n', buf).map_err(|e| match e.kind() { + // The idle timeout ran out (Windows reports it as TimedOut, Unix as WouldBlock). + ErrorKind::TimedOut | ErrorKind::WouldBlock => "request line not completed in time; closing the connection".to_string(), + _ => e.to_string(), + })?; if n == 0 { return Ok(false); } @@ -86,48 +125,96 @@ fn read_bounded_line(reader: &mut impl BufRead, buf: &mut Vec, max: usize) - Ok(true) } -fn serve(stream: TcpStream, tx: Sender, ctx: egui::Context) { +/// One request line, parsed. +enum Line { + Request { + id: Value, + method: String, + params: Value, + }, + Blank, + /// Not a request: reply with this error and close the connection. + Reject(String), +} + +fn parse_line(bytes: &[u8]) -> Line { + let Ok(text) = std::str::from_utf8(bytes) else { + return Line::Reject("not a control request (invalid UTF-8); closing the connection".into()); + }; + let text = text.trim(); + if text.is_empty() { + return Line::Blank; + } + if is_http_request_line(text) { + return Line::Reject("HTTP is not served here: this port takes JSON lines; closing the connection".into()); + } + let msg = match serde_json::from_str::(text) { + Ok(v) => v, + Err(e) => return Line::Reject(format!("bad JSON ({e}); expected one JSON request object per line; closing the connection")), + }; + let Some(obj) = msg.as_object() else { + return Line::Reject("not a control request (expected a JSON object); closing the connection".into()); + }; + let Some(method) = obj.get("method").and_then(Value::as_str) else { + return Line::Reject("not a control request (missing string `method`); closing the connection".into()); + }; + Line::Request { + id: obj.get("id").cloned().unwrap_or(Value::Null), + method: method.to_string(), + params: obj.get("params").cloned().unwrap_or(json!({})), + } +} + +/// An HTTP/1 request line (`POST / HTTP/1.1`) or the HTTP/2 preface (`PRI * HTTP/2.0`). +fn is_http_request_line(text: &str) -> bool { + const METHODS: [&str; 10] = ["GET", "HEAD", "POST", "PUT", "DELETE", "CONNECT", "OPTIONS", "TRACE", "PATCH", "PRI"]; + text.split_once(' ').is_some_and(|(method, _)| METHODS.contains(&method)) + || text.rsplit_once(' ').is_some_and(|(_, version)| version.starts_with("HTTP/")) +} + +fn serve(stream: TcpStream, tx: &SyncSender, ctx: &egui::Context, limits: Limits) { + if stream.set_read_timeout(Some(limits.idle_timeout)).is_err() || stream.set_write_timeout(Some(limits.write_timeout)).is_err() { + return; + } let Ok(read) = stream.try_clone() else { return }; let mut out = stream; let mut reader = BufReader::new(read); let mut buf = Vec::new(); loop { - match read_bounded_line(&mut reader, &mut buf, MAX_LINE) { - Ok(true) => {} + let reject = match read_bounded_line(&mut reader, &mut buf, limits.max_line) { Ok(false) => break, - Err(e) => { - let _ = writeln!(out, "{}", json!({"ok": false, "error": e})); - break; - } - } - // Bytes that aren't UTF-8 end the connection, as `BufRead::lines` did. - let Ok(line) = std::str::from_utf8(&buf) else { break }; - if line.trim().is_empty() { - continue; - } - let reply = match serde_json::from_str::(line) { - Ok(msg) => { - let id = msg.get("id").cloned().unwrap_or(Value::Null); - let method = msg.get("method").and_then(Value::as_str).unwrap_or("").to_string(); - let params = msg.get("params").cloned().unwrap_or(json!({})); - let (req, pending) = ControlRequest::new(method, params); - if tx.send(req).is_err() { - break; + // Nothing of a line arrived (idle timeout, or the peer is gone): close without a word. + // A line nobody asked for would be read as the reply to the client's next request. + Err(_) if buf.is_empty() => break, + Err(e) => e, + Ok(true) => match parse_line(&buf) { + Line::Blank => continue, + Line::Reject(e) => e, + Line::Request { id, method, params } => { + let (req, pending) = ControlRequest::new(method, params); + let mut r = match tx.try_send(req) { + Ok(()) => { + ctx.request_repaint(); + // On timeout the request is cancelled if the app hasn't started it, and + // the reply's `state` says whether it can still have run. + pending.wait(limits.reply_timeout) + } + Err(TrySendError::Full(_)) => json!({"ok": false, "error": "busy: too many queued requests", "state": "not-run"}), + Err(TrySendError::Disconnected(_)) => break, + }; + if let Some(o) = r.as_object_mut() { + o.insert("id".into(), id); + } + if writeln!(out, "{r}").is_err() { + break; + } + continue; } - ctx.request_repaint(); - // On timeout the request is cancelled if the app hasn't started it, and the reply's - // `state` says whether it can still have run. - let mut r = pending.wait(Duration::from_secs(60)); - if let Some(o) = r.as_object_mut() { - o.insert("id".into(), id); - } - r - } - Err(e) => json!({"ok": false, "error": format!("bad JSON: {e}")}), + }, }; - if writeln!(out, "{reply}").is_err() { - break; - } + // The connection closes either way; a peer that can't read the reason loses nothing. + let _ = writeln!(out, "{}", json!({"ok": false, "error": reject})); + break; } } @@ -158,12 +245,26 @@ mod tests { assert_eq!(input.len(), 100 - 11, "the rest of the line is left unread"); } - /// The control server on a loopback port, with a stand-in UI that echoes each method. - fn serve_echo() -> std::net::SocketAddr { + fn short() -> Limits { + Limits { + queue: 1, + idle_timeout: Duration::from_millis(300), + write_timeout: Duration::from_millis(300), + reply_timeout: Duration::from_millis(300), + ..Limits::default() + } + } + + /// The control server on a loopback port under `limits`; requests arrive on the receiver. + fn serve_with(limits: Limits) -> (std::net::SocketAddr, Receiver) { let listener = TcpListener::bind(("127.0.0.1", 0)).unwrap(); let addr = listener.local_addr().unwrap(); - let (tx, rx) = channel::(); - std::thread::spawn(move || accept_loop(listener, tx, egui::Context::default())); + (addr, serve_listener(listener, limits, egui::Context::default())) + } + + /// The control server on a loopback port, with a stand-in UI that echoes each method. + fn serve_echo() -> std::net::SocketAddr { + let (addr, rx) = serve_with(Limits::default()); std::thread::spawn(move || { for req in rx { let _ = req.reply.send(json!({"ok": true, "method": req.method})); @@ -172,6 +273,83 @@ mod tests { addr } + /// Send `input` on a new connection and read reply lines until the server closes it. + fn exchange(addr: std::net::SocketAddr, input: &[u8]) -> Vec { + let mut c = TcpStream::connect(addr).unwrap(); + c.set_read_timeout(Some(Duration::from_secs(5))).unwrap(); + c.write_all(input).unwrap(); + let mut replies = Vec::new(); + for line in BufReader::new(c).lines() { + // A read timeout here means the server kept the connection open. + let line = line.unwrap(); + replies.push(serde_json::from_str(&line).unwrap()); + } + replies + } + + /// A browser can POST to the port: its request line closes the connection before the JSON + /// line in its body is read, so nothing is dispatched. + #[test] + fn an_http_request_closes_the_connection_before_its_body_is_dispatched() { + let (addr, rx) = serve_with(short()); + let http = b"POST / HTTP/1.1\r\nHost: 127.0.0.1\r\nContent-Type: text/plain\r\n\r\n\ + {\"id\":1,\"method\":\"cmdline.input\",\"params\":{\"text\":\"circle 0,0 5\"}}\n"; + let replies = exchange(addr, http); + assert_eq!(replies.len(), 1, "{replies:?}"); + assert_eq!(replies[0]["ok"], false); + assert!(replies[0]["error"].as_str().unwrap().contains("closing the connection"), "{replies:?}"); + assert!(rx.try_recv().is_err(), "the body was dispatched"); + } + + #[test] + fn a_line_that_is_not_a_request_closes_the_connection() { + for junk in [&b"hello\n"[..], b"[1,2]\n", b"{\"id\":1}\n", b"{\"method\":5}\n", b"\xff\xfe\n", b"GET /x HTTP/1.1\n"] { + let addr = serve_echo(); + let mut input = junk.to_vec(); + input.extend_from_slice(b"{\"id\":2,\"method\":\"ping\"}\n"); + let replies = exchange(addr, &input); + assert_eq!(replies.len(), 1, "{replies:?}"); + assert_eq!(replies[0]["ok"], false); + } + assert!(is_http_request_line("OPTIONS * HTTP/1.1")); + assert!(is_http_request_line("PRI * HTTP/2.0")); + assert!(!is_http_request_line("{\"method\":\"GET /\"}")); + } + + #[test] + fn an_idle_connection_is_closed() { + let (addr, _rx) = serve_with(short()); + let mut c = TcpStream::connect(addr).unwrap(); + c.set_read_timeout(Some(Duration::from_secs(5))).unwrap(); + let t = std::time::Instant::now(); + let mut got = Vec::new(); + c.read_to_end(&mut got).unwrap(); + assert!(got.is_empty(), "{}", String::from_utf8_lossy(&got)); + assert!(t.elapsed() < Duration::from_secs(4)); + } + + #[test] + fn a_full_queue_answers_busy() { + let (addr, rx) = serve_with(short()); + // No UI drains the one-place queue: the first request fills it, the second finds it full. + let first = std::thread::spawn(move || exchange(addr, b"{\"id\":1,\"method\":\"m\"}\n")); + let deadline = std::time::Instant::now() + Duration::from_secs(4); + let busy = loop { + let mut c = TcpStream::connect(addr).unwrap(); + c.set_read_timeout(Some(Duration::from_secs(5))).unwrap(); + c.write_all(b"{\"id\":2,\"method\":\"m\"}\n").unwrap(); + let mut line = String::new(); + BufReader::new(c).read_line(&mut line).unwrap(); + let v: Value = serde_json::from_str(&line).unwrap(); + if v["error"].as_str().is_some_and(|e| e.starts_with("busy")) || std::time::Instant::now() > deadline { + break v; + } + }; + assert_eq!((busy["id"].as_i64(), busy["state"].as_str()), (Some(2), Some("not-run")), "{busy}"); + assert_eq!(first.join().unwrap()[0]["state"], "not-run"); + drop(rx); + } + #[test] fn requests_are_answered_with_their_ids() { let addr = serve_echo(); @@ -191,7 +369,7 @@ mod tests { fn extra_connections_are_refused() { let addr = serve_echo(); // Idle connections are accepted in order and held open. - let held: Vec = (0..MAX_CONNECTIONS).map(|_| TcpStream::connect(addr).unwrap()).collect(); + let held: Vec = (0..Limits::default().max_clients).map(|_| TcpStream::connect(addr).unwrap()).collect(); let mut extra = TcpStream::connect(addr).unwrap(); extra.set_read_timeout(Some(Duration::from_secs(5))).unwrap(); let mut line = String::new(); diff --git a/docs/control-protocol.md b/docs/control-protocol.md index e07d758b..1d5a73b1 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -12,6 +12,15 @@ Start the desktop app with `--control PORT` (or `CADCRAFT_CONTROL_PORT=PORT`). I Errors come back as `{"id": …, "ok": false, "error": "message"}`. An unknown method's error points to `control.methods`, which lists the methods the server answers. +A line that is not a JSON object with a string `method` (an HTTP request, text that isn't JSON, +bytes that aren't UTF-8) gets one error reply and the server closes the connection; nothing sent +after it runs. Blank lines are skipped. + +The server bounds what a client can hold: 16 connections at once (more get an error line and are +closed), 256 MiB per request line (longer closes the connection), 64 queued requests (more are +answered `{"ok": false, "error": "busy: …", "state": "not-run"}`), and 10 minutes idle or 30 seconds +per blocked reply write before the socket is closed. + The server waits up to 60 s for the app to answer a request. After that it replies with a timeout, and `state` tells whether sending the request again is safe: From 8fe7c421ea95b986cedaa49314ce74f607a8a388 Mon Sep 17 00:00:00 2001 From: aiongg Date: Fri, 16 Oct 2026 17:18:34 +0000 Subject: [PATCH 2/4] Xrefs and data links: follow stored paths only on this computer or the drawing's share Xref paths saved in a drawing (on open, on Reload, in the External References list, XOPEN, nested xrefs) and table data-link paths re-read by DATALINKUPDATE go through doc_path::resolve (craft-docpath): relative paths resolve beside the drawing that stores them, and paths to other computers, devices and URLs are not opened. Such an xref is Unresolved with the reason, unless a file of its name is beside the host drawing. Paths given to XATTACH, -XREF Path and DATALINK are used as given. One load of xrefs reads at most 1 GB from referenced files in total. --- Cargo.lock | 5 + Cargo.toml | 2 + crates/engine/Cargo.toml | 1 + crates/engine/src/cmd/blocks/xref.rs | 23 ++- crates/engine/src/cmd/blocks/xref/load.rs | 202 +++++++++++++++------- crates/engine/src/cmd/table/breaks.rs | 28 ++- crates/engine/src/doc_path.rs | 38 ++++ crates/engine/src/lib.rs | 1 + crates/engine/tests/table_breaks.rs | 46 +++++ crates/engine/tests/xref.rs | 69 ++++++++ 10 files changed, 334 insertions(+), 81 deletions(-) create mode 100644 crates/engine/src/doc_path.rs diff --git a/Cargo.lock b/Cargo.lock index c054dcfd..cbc5fd45 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -634,6 +634,7 @@ dependencies = [ "cadcraft-geom", "cadcraft-io", "cadcraft-render", + "craft-docpath", "log", "rstar", "serde", @@ -927,6 +928,10 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f8f80099a98041a3d1622845c271458a2d73e688351bf3cb999266764b81d48" +[[package]] +name = "craft-docpath" +version = "0.1.0" + [[package]] name = "craft-ui" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index 9632245d..07dfcb7e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,8 @@ cadcraft-io = { path = "crates/io" } cadcraft-engine = { path = "crates/engine" } cadcraft-ui-egui = { path = "crates/ui-egui" } cadcraft-mcp = { path = "crates/mcp" } +# Which paths stored in a drawing (xrefs, data links) may be opened. +craft-docpath = { path = "../secfix-craft-libs/crates/docpath" } # foundation thiserror = "2" diff --git a/crates/engine/Cargo.toml b/crates/engine/Cargo.toml index 0dad3771..a19da90f 100644 --- a/crates/engine/Cargo.toml +++ b/crates/engine/Cargo.toml @@ -16,6 +16,7 @@ cadcraft-dxf = { workspace = true } cadcraft-constraints = { workspace = true } cadcraft-fonts = { workspace = true } cadcraft-render = { workspace = true } +craft-docpath = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } thiserror = { workspace = true } diff --git a/crates/engine/src/cmd/blocks/xref.rs b/crates/engine/src/cmd/blocks/xref.rs index a3b19fa4..b87e7379 100644 --- a/crates/engine/src/cmd/blocks/xref.rs +++ b/crates/engine/src/cmd/blocks/xref.rs @@ -104,8 +104,8 @@ fn reference_count(d: &Drawing, key: &str) -> usize { fn describe(d: &Drawing, b: &Block, host: Option<&str>) -> Value { let refs = reference_count(d, &b.name); let status = if refs == 0 && b.xref_status == XrefStatus::Loaded { "Unreferenced" } else { b.xref_status.label() }; - let dir = host.and_then(|h| std::path::Path::new(h).parent().map(std::path::Path::to_path_buf)); - let found = b.xref_path.as_deref().and_then(|p| load::find(p, dir.as_deref())).map(|f| f.to_string_lossy().into_owned()); + let found = + b.xref_path.as_deref().and_then(|p| load::find(p, host.map(std::path::Path::new), false).ok()).map(|f| f.to_string_lossy().into_owned()); json!({ "name": b.name, "path": b.xref_path, @@ -146,7 +146,7 @@ fn attach(s: &mut Session, name: &str, path: &str, overlay: bool) -> Result {} Some(o) => return Err(bad("xattach", format!("{path} can't be attached: {}", o.detail))), @@ -227,11 +227,7 @@ fn run_dash_xref(s: &mut Session, p: &Value) -> Result { } "reload" | "r" => { let names = named(s, "-xref", p)?; - let mut out = Vec::new(); - for n in &names { - let d = s.doc_mut()?; - out.extend(load::load(d, n, host.as_deref())); - } + let out = load::load_each(s.doc_mut()?, &names, host.as_deref(), false); echo_all(s, &out); Ok( json!({ "xrefs": out.iter().filter(|o| !o.name.is_empty()).map(|o| json!({"name": o.name, "status": o.status.label(), "detail": o.detail})).collect::>() }), @@ -271,14 +267,13 @@ fn run_dash_xref(s: &mut Session, p: &Value) -> Result { "path" | "p" => { let path = str_param(p, "path").map(str::trim).filter(|t| !t.is_empty()).ok_or_else(|| bad("-xref", "`path` is required"))?.to_string(); let names = named(s, "-xref", p)?; - let mut out = Vec::new(); + let d = s.doc_mut()?; for n in &names { - let d = s.doc_mut()?; if let Some(b) = d.blocks.get_mut(n) { Arc::make_mut(b).xref_path = Some(path.clone()); } - out.extend(load::load(d, n, host.as_deref())); } + let out = load::load_each(d, &names, host.as_deref(), true); echo_all(s, &out); Ok(json!({ "xrefs": names, "path": path })) } @@ -367,8 +362,10 @@ fn run_xopen(s: &mut Session, p: &Value) -> Result { let b = d.block(&name).ok_or_else(|| bad("xopen", format!("no xref \"{name}\"")))?; let path = b.xref_path.clone().ok_or_else(|| bad("xopen", format!("\"{name}\" is not an xref")))?; let host = host(s); - let dir = host.as_deref().and_then(|h| std::path::Path::new(h).parent().map(std::path::Path::to_path_buf)); - let file = load::find(&path, dir.as_deref()).ok_or_else(|| bad("xopen", format!("{path}: file not found")))?; + let file = load::find(&path, host.as_deref().map(std::path::Path::new), false).map_err(|m| match m { + load::Missing::NotFound => bad("xopen", format!("{path}: file not found")), + load::Missing::Refused(r) => bad("xopen", format!("{path}: not opened, {r}")), + })?; s.execute("open", &json!({ "path": file.to_string_lossy() })) } diff --git a/crates/engine/src/cmd/blocks/xref/load.rs b/crates/engine/src/cmd/blocks/xref/load.rs index 36abd364..efe9143a 100644 --- a/crates/engine/src/cmd/blocks/xref/load.rs +++ b/crates/engine/src/cmd/blocks/xref/load.rs @@ -3,8 +3,10 @@ //! `xref|name`. //! //! A path is looked for as given (relative paths against the host drawing's folder), then by its -//! file name in the host drawing's folder. A referenced drawing's own attached xrefs load with it -//! (overlays don't), at most [`MAX_BLOCK_DEPTH`] levels deep; a drawing that is already being +//! file name in the host drawing's folder. A path stored in a drawing is followed only to this +//! computer or the host drawing's network share ([`crate::doc_path`]); one that isn't followed and +//! has no file of its name beside the host leaves the xref unresolved. A referenced drawing's own +//! attached xrefs load with it (overlays don't), at most [`MAX_BLOCK_DEPTH`] levels deep; a drawing that is already being //! loaded further up (a circular reference) is not loaded again. Every failure leaves the xref's //! block empty with a status saying why; nothing here stops the host drawing from opening. @@ -13,6 +15,8 @@ use std::sync::Arc; use cadcraft_doc::{Attrib, Drawing, Entity, EntityKind, EntityStore, MAX_BLOCK_DEPTH, XrefStatus}; +use crate::doc_path::{self, Refused}; + /// Most objects the xrefs of one drawing load in one go (nested xrefs included). pub(crate) const MAX_XREF_OBJECTS: usize = 4_000_000; /// Most referenced files read in one go (nested xrefs included). @@ -20,6 +24,9 @@ const MAX_XREF_FILES: usize = 1000; /// Largest referenced file read. #[cfg(not(target_arch = "wasm32"))] const MAX_XREF_BYTES: u64 = 512 << 20; +/// Most bytes read from referenced files in one go (nested xrefs included). +#[cfg(not(target_arch = "wasm32"))] +const MAX_XREF_TOTAL_BYTES: u64 = 1 << 30; /// The kinds of named symbols an object can refer to. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] @@ -119,16 +126,38 @@ struct Ctx { chain: Vec, objects: usize, files: usize, + #[cfg_attr(target_arch = "wasm32", allow(dead_code))] + bytes: u64, /// Messages about nested xrefs. notes: Vec, } /// The folder of the host drawing at `host` (a file path). -fn folder(host: Option<&str>) -> Option { - let p = Path::new(host?).parent()?; +fn folder(host: Option<&Path>) -> Option { + let p = host?.parent()?; Some(if p.as_os_str().is_empty() { PathBuf::from(".") } else { p.to_path_buf() }) } +/// Why a referenced drawing wasn't found. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum Missing { + NotFound, + /// The saved path isn't followed, and there is no file of its name beside the host. + Refused(Refused), +} + +impl Missing { + fn outcome(self, path: &str) -> (XrefStatus, String) { + match self { + Self::NotFound => { + let why = if cfg!(target_arch = "wasm32") { "files are not available on the web" } else { "file not found" }; + (XrefStatus::NotFound, format!("{path}: {why}")) + } + Self::Refused(r) => (XrefStatus::Unresolved, format!("{path}: not opened, {r}")), + } + } +} + #[cfg(not(target_arch = "wasm32"))] fn canonical(p: &Path) -> PathBuf { std::fs::canonicalize(p).unwrap_or_else(|_| p.to_path_buf()) @@ -138,58 +167,94 @@ fn canonical(p: &Path) -> PathBuf { p.to_path_buf() } -/// Where to look for a referenced drawing saved as `path`: the path itself (a relative one against -/// the host's folder), then its file name in the host's folder. Both separators work everywhere. -fn candidates(path: &str, dir: Option<&Path>) -> Vec { - let norm = if cfg!(windows) { path.to_string() } else { path.replace('\\', "/") }; - let p = PathBuf::from(norm.trim()); +/// Where to look for a referenced drawing at `path` in the drawing at `host`: the path itself (a +/// relative one against the host's folder), then its file name in the host's folder. Both +/// separators work everywhere. A path stored in a drawing goes through [`doc_path::resolve`] +/// (the refusal is returned); a path the user just `chosen` is used as given. +fn candidates(path: &str, host: Option<&Path>, chosen: bool) -> (Vec, Option) { + let path = path.trim(); let mut out = Vec::new(); - match dir { - Some(d) if !p.is_absolute() => out.push(d.join(&p)), - _ => out.push(p.clone()), + let mut refused = None; + if chosen { + let norm = if cfg!(windows) { path.to_string() } else { path.replace('\\', "/") }; + let p = PathBuf::from(norm); + match folder(host) { + Some(d) if !p.is_absolute() => out.push(d.join(&p)), + _ => out.push(p), + } + } else { + match doc_path::resolve(path, host) { + Ok(p) => out.push(p), + Err(Refused::Invalid) => {} + Err(r) => refused = Some(r), + } } - if let (Some(d), Some(f)) = (dir, p.file_name()) { - out.push(d.join(f)); + let name = path.rsplit(['/', '\\']).next().unwrap_or(path); + if host.is_some() + && !name.is_empty() + && let Ok(p) = doc_path::resolve(name, host) + && !out.contains(&p) + { + out.push(p); } - out + (out, refused) } -/// The file a referenced drawing saved as `path` is loaded from, if there is one. -pub(crate) fn find(path: &str, dir: Option<&Path>) -> Option { +/// The file a referenced drawing at `path` (saved in the drawing at `host`, or just `chosen` by +/// the user) is loaded from. +pub(crate) fn find(path: &str, host: Option<&Path>, chosen: bool) -> Result { if path.trim().is_empty() { - return None; + return Err(Missing::NotFound); } + let (files, refused) = candidates(path, host, chosen); #[cfg(not(target_arch = "wasm32"))] - { - candidates(path, dir).into_iter().find(|p| p.is_file()) - } + let found = files.into_iter().find(|p| p.is_file()); #[cfg(target_arch = "wasm32")] - { - let _ = candidates(path, dir); + let found: Option = { + let _ = files; None - } + }; + found.ok_or(refused.map_or(Missing::NotFound, Missing::Refused)) } -/// Read a drawing file through the host's io hooks. -fn read(file: &Path) -> Result { +/// Read a drawing file through the host's io hooks, counting its bytes against the load's total. +fn read(file: &Path, cx: &mut Ctx) -> Result { let hooks = super::super::super::file::io().ok_or("file formats are not available in this build")?; #[cfg(not(target_arch = "wasm32"))] { - let len = std::fs::metadata(file).map_err(|e| e.to_string())?.len(); - if len > MAX_XREF_BYTES { - return Err(format!("the file is larger than {} MB", MAX_XREF_BYTES >> 20)); + use std::io::Read; + let left = MAX_XREF_TOTAL_BYTES.saturating_sub(cx.bytes); + let limit = MAX_XREF_BYTES.min(left); + let mut f = std::fs::File::open(file).map_err(|e| e.to_string())?; + let len = f.metadata().map_err(|e| e.to_string())?.len(); + let too_big = || { + if limit < MAX_XREF_BYTES { + format!("the referenced files are larger than {} MB together", MAX_XREF_TOTAL_BYTES >> 20) + } else { + format!("the file is larger than {} MB", MAX_XREF_BYTES >> 20) + } + }; + if len > limit { + return Err(too_big()); } - let bytes = std::fs::read(file).map_err(|e| e.to_string())?; + let mut bytes = Vec::new(); + f.by_ref().take(limit.saturating_add(1)).read_to_end(&mut bytes).map_err(|e| e.to_string())?; + let n = u64::try_from(bytes.len()).unwrap_or(u64::MAX); + if n > limit { + return Err(too_big()); + } + cx.bytes = cx.bytes.saturating_add(n); (hooks.read)(&bytes, &file.to_string_lossy()) } #[cfg(target_arch = "wasm32")] { - let _ = (hooks, file); + let _ = (hooks, file, cx); Err("files are not available on the web".into()) } } -/// Load every xref of `d` that isn't unloaded; `host` is the drawing's own path. +/// Load every xref of `d` that isn't unloaded from the paths saved in it; `host` is the drawing's +/// own path. pub(crate) fn load_all(d: &mut Drawing, host: Option<&str>) -> Vec { let names: Vec = d .blocks @@ -197,28 +262,44 @@ pub(crate) fn load_all(d: &mut Drawing, host: Option<&str>) -> Vec { .filter(|b| b.xref_path.is_some() && b.xref_status != XrefStatus::Unloaded && !b.name.contains('|')) .map(|b| b.name.clone()) .collect(); - names.iter().flat_map(|n| load(d, n, host)).collect() + load_each(d, &names, host, false) } -/// Load (or reload) xref `name` of `d` from its file; `host` is the drawing's own path. Returns -/// the outcome, then notes about its nested xrefs. -pub(crate) fn load(d: &mut Drawing, name: &str, host: Option<&str>) -> Vec { - let host_file = host.map(Path::new).filter(|p| p.file_name().is_some()); - let mut cx = Ctx { chain: host_file.map(canonical).into_iter().collect(), objects: 0, files: 0, notes: Vec::new() }; - let dir = folder(host); - let (status, detail) = load_in(d, name, dir.as_deref(), &mut cx, 0); +/// Load (or reload) xref `name` of `d` from its file; `host` is the drawing's own path, and +/// `chosen` says the xref's path was just given by the user (not read from a drawing). Returns the +/// outcome, then notes about its nested xrefs. +pub(crate) fn load(d: &mut Drawing, name: &str, host: Option<&str>, chosen: bool) -> Vec { + load_each(d, std::slice::from_ref(&name.to_string()), host, chosen) +} + +/// [`load`] the xrefs `names` within one set of limits. +pub(crate) fn load_each(d: &mut Drawing, names: &[String], host: Option<&str>, chosen: bool) -> Vec { + let mut cx = Ctx::new(host); + names.iter().flat_map(|n| load_with(d, n, host, chosen, &mut cx)).collect() +} + +impl Ctx { + fn new(host: Option<&str>) -> Self { + let host_file = host.map(Path::new).filter(|p| p.file_name().is_some()); + Ctx { chain: host_file.map(canonical).into_iter().collect(), objects: 0, files: 0, bytes: 0, notes: Vec::new() } + } +} + +fn load_with(d: &mut Drawing, name: &str, host: Option<&str>, chosen: bool, cx: &mut Ctx) -> Vec { + let host = host.map(Path::new).filter(|p| p.file_name().is_some()); + let (status, detail) = load_in(d, name, host, chosen, cx, 0); let key = d.block(name).map(|b| b.name.clone()).unwrap_or_else(|| name.to_string()); let mut out = vec![Outcome { name: key, status, detail }]; - out.extend(cx.notes.into_iter().map(|n| Outcome { name: String::new(), status: XrefStatus::Unresolved, detail: n })); + out.extend(cx.notes.drain(..).map(|n| Outcome { name: String::new(), status: XrefStatus::Unresolved, detail: n })); out } -fn load_in(d: &mut Drawing, name: &str, dir: Option<&Path>, cx: &mut Ctx, depth: usize) -> (XrefStatus, String) { +fn load_in(d: &mut Drawing, name: &str, host: Option<&Path>, chosen: bool, cx: &mut Ctx, depth: usize) -> (XrefStatus, String) { let Some(b) = d.block(name) else { return (XrefStatus::NotFound, "no such xref".into()) }; let key = b.name.clone(); let Some(path) = b.xref_path.clone() else { return (XrefStatus::Unresolved, "not an xref".into()) }; clear(d, &key); - let (status, detail) = match fetch(&path, dir, cx, depth) { + let (status, detail) = match fetch(&path, host, chosen, cx, depth) { Ok((x, file)) => match import(d, &key, &x, cx) { Ok(()) => (XrefStatus::Loaded, file.to_string_lossy().into_owned()), Err(e) => (XrefStatus::Unresolved, e), @@ -234,15 +315,12 @@ fn load_in(d: &mut Drawing, name: &str, dir: Option<&Path>, cx: &mut Ctx, depth: } /// Read the drawing referenced as `path` and load its own attached xrefs into it. -fn fetch(path: &str, dir: Option<&Path>, cx: &mut Ctx, depth: usize) -> Result<(Drawing, PathBuf), (XrefStatus, String)> { +fn fetch(path: &str, host: Option<&Path>, chosen: bool, cx: &mut Ctx, depth: usize) -> Result<(Drawing, PathBuf), (XrefStatus, String)> { let unresolved = |m: &str| (XrefStatus::Unresolved, m.to_string()); if depth >= MAX_BLOCK_DEPTH { return Err(unresolved("xrefs are nested too deeply")); } - let Some(file) = find(path, dir) else { - let why = if cfg!(target_arch = "wasm32") { "files are not available on the web" } else { "file not found" }; - return Err((XrefStatus::NotFound, format!("{path}: {why}"))); - }; + let file = find(path, host, chosen).map_err(|m| m.outcome(path))?; let canon = canonical(&file); if cx.chain.contains(&canon) { return Err(unresolved("circular reference")); @@ -251,13 +329,12 @@ fn fetch(path: &str, dir: Option<&Path>, cx: &mut Ctx, depth: usize) -> Result<( return Err(unresolved("too many referenced files")); } cx.files += 1; - let mut x = read(&file).map_err(|e| (XrefStatus::Unresolved, format!("{}: {e}", file.to_string_lossy())))?; + let mut x = read(&file, cx).map_err(|e| (XrefStatus::Unresolved, format!("{}: {e}", file.to_string_lossy())))?; cx.chain.push(canon); let nested: Vec = x.blocks.values().filter(|b| b.xref_path.is_some() && !b.xref_overlay && !b.name.contains('|')).map(|b| b.name.clone()).collect(); - let xdir = file.parent().map(Path::to_path_buf); for n in nested { - let (status, detail) = load_in(&mut x, &n, xdir.as_deref(), cx, depth + 1); + let (status, detail) = load_in(&mut x, &n, Some(&file), false, cx, depth + 1); if status != XrefStatus::Loaded { cx.notes.push(format!("nested xref \"{n}\" not loaded: {detail}")); } @@ -430,18 +507,23 @@ mod tests { #[test] fn paths_resolve_against_the_host_folder_then_by_name() { - let dir = Path::new("/proj/plans"); - let c = candidates("parts/a.dxf", Some(dir)); - assert_eq!(c, vec![dir.join("parts/a.dxf"), dir.join("a.dxf")]); - let c = candidates("/abs/b.dwg", Some(dir)); - assert_eq!(c, vec![PathBuf::from("/abs/b.dwg"), dir.join("b.dwg")]); + let dir = std::env::temp_dir().join("proj").join("plans"); + let host = dir.join("host.dxf"); + let c = |path: &str, chosen| candidates(path, Some(&host), chosen); + assert_eq!(c("parts/a.dxf", false), (vec![dir.join("parts").join("a.dxf"), dir.join("a.dxf")], None)); + let abs = std::env::temp_dir().join("abs").join("b.dwg"); + assert_eq!(c(&abs.to_string_lossy(), false), (vec![abs.clone(), dir.join("b.dwg")], None)); if !cfg!(windows) { // A Windows path saved elsewhere still finds the file next to the host. - let c = candidates("C:\\jobs\\site.dwg", Some(dir)); - assert_eq!(c.last(), Some(&dir.join("site.dwg"))); + assert_eq!(c("C:\\jobs\\site.dwg", false), (vec![dir.join("site.dwg")], None)); } - assert_eq!(candidates("a.dxf", None), vec![PathBuf::from("a.dxf")]); - assert!(find("", Some(dir)).is_none()); + // Paths to other computers are only looked for beside the host. + assert_eq!(c(r"\\192.0.2.1\share\x.dxf", false), (vec![dir.join("x.dxf")], Some(Refused::Network))); + assert_eq!(c("file://192.0.2.1/s/x.dxf", false), (vec![dir.join("x.dxf")], Some(Refused::Network))); + // Without a host drawing a relative path names no file. + assert_eq!(candidates("a.dxf", None, false), (vec![], None)); + assert_eq!(candidates("a.dxf", None, true), (vec![PathBuf::from("a.dxf")], None)); + assert_eq!(find("", Some(&host), false), Err(Missing::NotFound)); } #[test] diff --git a/crates/engine/src/cmd/table/breaks.rs b/crates/engine/src/cmd/table/breaks.rs index 9bf98e56..f7e6ce03 100644 --- a/crates/engine/src/cmd/table/breaks.rs +++ b/crates/engine/src/cmd/table/breaks.rs @@ -224,15 +224,25 @@ fn separator(path: &str, text: &str) -> char { if first.contains(';') && !first.contains(',') { ';' } else { ',' } } +/// The file to read for a data link: a path the user just `chosen` as given, a path stored in the +/// drawing at `host` only through [`crate::doc_path::resolve`]. +fn link_file(id: &str, path: &str, host: Option<&str>, chosen: bool) -> Result { + if chosen { + return Ok(std::path::PathBuf::from(path)); + } + crate::doc_path::resolve(path, host.map(std::path::Path::new)).map_err(|r| bad(id, format!("{path}: not opened, {r}"))) +} + /// Read a linked file through the host (natively the file system; not on the web). -fn read_link(id: &str, path: &str) -> Result { +fn read_link(id: &str, file: &std::path::Path) -> Result { + let path = file.display(); #[cfg(not(target_arch = "wasm32"))] { - let meta = std::fs::metadata(path).map_err(|e| bad(id, format!("{path}: {e}")))?; + let meta = std::fs::metadata(file).map_err(|e| bad(id, format!("{path}: {e}")))?; if meta.len() > MAX_LINK_BYTES { return Err(bad(id, format!("{path}: larger than {} MB", MAX_LINK_BYTES >> 20))); } - let bytes = std::fs::read(path).map_err(|e| bad(id, format!("{path}: {e}")))?; + let bytes = std::fs::read(file).map_err(|e| bad(id, format!("{path}: {e}")))?; Ok(String::from_utf8_lossy(&bytes).into_owned()) } #[cfg(target_arch = "wasm32")] @@ -278,14 +288,16 @@ fn fill(t: &mut Table, start: usize, data: &[Vec]) { } } -/// Re-read the linked file of table `h`. -fn update(s: &mut Session, h: Handle) -> Result<(usize, usize)> { +/// Re-read the linked file of table `h`; `chosen` says its path was just given by the user. +fn update(s: &mut Session, h: Handle, chosen: bool) -> Result<(usize, usize)> { let id = "datalinkupdate"; let link = match s.doc()?.entity(h).map(|e| &e.kind) { Some(EntityKind::Table(t)) => t.data_link.clone().ok_or_else(|| bad(id, "the table has no data link"))?, _ => return Err(bad(id, "object is not a table")), }; - let text = read_link(id, &link.path)?; + let host = s.state()?.path.clone(); + let file = link_file(id, &link.path, host.as_deref(), chosen)?; + let text = read_link(id, &file)?; let data = parse_csv(&text, separator(&link.path, &text)); let mut size = (0, 0); edit(s, &json!({ "handle": h.hex() }), id, |t| { @@ -313,7 +325,7 @@ fn run_datalink(s: &mut Session, p: &Value) -> Result { Ok(()) })?; let h = r.get("handle").and_then(Value::as_str).and_then(Handle::parse_hex).ok_or_else(|| bad(id, "no table"))?; - let (rows, cols) = update(s, h)?; + let (rows, cols) = update(s, h, true)?; Ok(json!({ "handle": h.hex(), "rows": rows, "cols": cols })) } @@ -340,7 +352,7 @@ fn run_update(s: &mut Session, p: &Value) -> Result { let mut updated = 0; let mut errors = Vec::new(); for h in hs { - match update(s, h) { + match update(s, h, false) { Ok(_) => updated += 1, Err(e) => errors.push(e.to_string()), } diff --git a/crates/engine/src/doc_path.rs b/crates/engine/src/doc_path.rs new file mode 100644 index 00000000..06b25c2f --- /dev/null +++ b/crates/engine/src/doc_path.rs @@ -0,0 +1,38 @@ +//! Paths stored in a drawing: xref paths and table data links. +//! +//! Such a path reaches the file system only through [`resolve`]: a relative path is taken from +//! the drawing's folder, and only files on this computer, or on the network share the drawing +//! itself is on, are opened (see `craft_docpath`). A refused path is handled like a missing file, +//! with the reason in its status. Paths the user gives a command (XATTACH, -XREF Path, DATALINK) +//! are their own choice and are used as given. + +use std::path::{Path, PathBuf}; + +pub use craft_docpath::Refused; + +/// The file to open for `stored`, a path or `file:` URL stored in the drawing saved at +/// `document` (`None` for a drawing without a file). +/// +/// A result that isn't absolute is refused as [`Refused::Invalid`]: without a document a +/// relative path would depend on the working directory, and a Windows drive path names no file +/// on other platforms. +pub fn resolve(stored: &str, document: Option<&Path>) -> Result { + let document = document.filter(|d| !d.as_os_str().is_empty()).map(|d| std::path::absolute(d).unwrap_or_else(|_| d.to_path_buf())); + let path = craft_docpath::resolve(stored.trim(), document.as_deref())?; + if path.is_absolute() { Ok(path) } else { Err(Refused::Invalid) } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn relative_paths_resolve_beside_the_drawing_and_other_computers_are_refused() { + let doc = std::env::temp_dir().join("plans").join("host.dxf"); + assert_eq!(resolve("parts/a.dxf", Some(&doc)), Ok(std::env::temp_dir().join("plans").join("parts").join("a.dxf"))); + assert_eq!(resolve("a.dxf", None), Err(Refused::Invalid)); + assert_eq!(resolve(r"\\192.0.2.1\share\a.dxf", Some(&doc)), Err(Refused::Network)); + assert_eq!(resolve("file://192.0.2.1/s/a.dxf", Some(&doc)), Err(Refused::Network)); + assert_eq!(resolve("//192.0.2.1/s/a.dxf", None), Err(Refused::Network)); + } +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 97f4b969..778b9696 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -13,6 +13,7 @@ pub mod audit; pub mod autosave; pub mod blockedit; pub mod cmd; +pub mod doc_path; mod finite; pub mod grip_dims; pub mod grips; diff --git a/crates/engine/tests/table_breaks.rs b/crates/engine/tests/table_breaks.rs index 90b05d7d..80aab078 100644 --- a/crates/engine/tests/table_breaks.rs +++ b/crates/engine/tests/table_breaks.rs @@ -125,3 +125,49 @@ fn data_links_fill_and_update() { assert!(table(&s, &h).data_link.is_none()); let _ = std::fs::remove_dir_all(&dir); } + +#[test] +fn data_links_saved_in_a_drawing_are_read_beside_it_or_on_this_computer_only() { + cadcraft_engine::cmd::file::set_io(cadcraft_engine::cmd::file::IoHooks { + read: |b, name| cadcraft_io::read(b, name).map_err(|e| e.to_string()), + write: |d, name| cadcraft_io::write(d, name).map_err(|e| e.to_string()), + plot: None, + }); + let base = std::env::temp_dir().join(format!("cadcraft-datalink-saved-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + let (dir, elsewhere) = (base.join("plans"), base.join("elsewhere")); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::create_dir_all(&elsewhere).unwrap(); + std::fs::write(dir.join("parts.csv"), "Part,Qty\nbolt,4\n").unwrap(); + std::fs::write(elsewhere.join("far.csv"), "Part,Qty\nnut,8\n").unwrap(); + + let mut links = vec![r"\\192.0.2.1\share\far.csv".to_string(), "file://192.0.2.1/s/far.csv".to_string(), "parts.csv".to_string()]; + if cfg!(unix) { + // Read as a local path by Linux, and as a network path by the resolver. + links.push(format!("/{}", elsewhere.join("far.csv").display())); + } + let mut s = Session::new(); + for (i, link) in links.iter().enumerate() { + let r = s.execute("table", &json!({ "at": [0, 10.0 * i as f64], "cols": 2, "rows": 1 })).unwrap(); + let h = Handle::parse_hex(r["handle"].as_str().unwrap()).unwrap(); + s.doc_mut() + .unwrap() + .modify_entity(h, |e| { + if let EntityKind::Table(t) = &mut e.kind { + t.data_link = Some(cadcraft_engine::doc::DataLink { path: link.clone(), start_row: 0 }); + } + }) + .unwrap(); + } + let host = dir.join("host.dxf"); + s.execute("saveas", &json!({ "path": host.to_str().unwrap() })).unwrap(); + let mut s = Session::new(); + s.execute("open", &json!({ "path": host.to_str().unwrap() })).unwrap(); + + let r = s.execute("datalinkupdate", &json!({})).unwrap(); + assert_eq!(r["updated"], 1, "only the file beside the drawing: {r}"); + let errors: Vec<&str> = r["errors"].as_array().unwrap().iter().filter_map(|e| e.as_str()).collect(); + assert_eq!(errors.len(), links.len() - 1, "{errors:?}"); + assert!(errors.iter().all(|e| e.contains("another computer")), "{errors:?}"); + let _ = std::fs::remove_dir_all(&base); +} diff --git a/crates/engine/tests/xref.rs b/crates/engine/tests/xref.rs index d50fdc46..405f71c6 100644 --- a/crates/engine/tests/xref.rs +++ b/crates/engine/tests/xref.rs @@ -435,3 +435,72 @@ fn hostile_xref_input_never_panics() { let _ = s.execute("xopen", &json!({ "name": "nothing" })); let _ = s.execute("xopen", &json!({})); } + +/// A drawing with xrefs `(name, saved path)`, each placed at 0,0. +fn host_with_paths(xrefs: &[(&str, &str)]) -> Drawing { + let mut d = Drawing::new_metric(); + for (name, path) in xrefs { + let mut b = Block::new(name); + b.xref_path = Some(path.to_string()); + d.blocks.insert(name.to_string(), Arc::new(b)); + let h = d.new_handle(); + d.model.push(Entity::new(h, insert(name, 0.0))); + } + d +} + +#[test] +fn saved_xref_paths_are_followed_beside_the_host_or_on_this_computer_only() { + hooks(); + let base = folder("saved-paths"); + let (dir, elsewhere) = (base.join("plans"), base.join("elsewhere")); + std::fs::create_dir_all(dir.join("parts")).unwrap(); + std::fs::create_dir_all(&elsewhere).unwrap(); + save(&site(), &elsewhere.join("far.dxf")); + // A part beside the host that references drawings the same ways. + let mut nested = vec![("unc", r"\\192.0.2.1\share\far.dxf".to_string())]; + // Read as a local path by Linux, and as a network path by the resolver. + let far_slashes = format!("/{}", elsewhere.join("far.dxf").display()); + if cfg!(unix) { + nested.push(("slashes", far_slashes.clone())); + } + let mut part = host_with_paths(&nested.iter().map(|(n, p)| (*n, p.as_str())).collect::>()); + line(&mut part, "0", (0.0, 0.0), (1.0, 1.0)); + save(&part, &dir.join("parts").join("part.dxf")); + + let mut xrefs = vec![ + ("unc", r"\\192.0.2.1\share\far.dxf".to_string()), + ("url", "file://192.0.2.1/s/far.dxf".to_string()), + ("part", "parts/part.dxf".to_string()), + ]; + if cfg!(unix) { + xrefs.push(("slashes", far_slashes.clone())); + } + let host = dir.join("host.dxf"); + save(&host_with_paths(&xrefs.iter().map(|(n, p)| (*n, p.as_str())).collect::>()), &host); + let mut s = Session::new(); + open(&mut s, &host); + + assert_eq!(status(&s, "part"), Some(XrefStatus::Loaded), "a relative path beside the host: {:?}", s.log); + assert_eq!(objects(&s, "part"), 1 + nested.len(), "the part's line and references, with nothing loaded into them"); + for name in ["unc", "url", "slashes"].into_iter().take(xrefs.len() - 1) { + assert_eq!((status(&s, name), objects(&s, name)), (Some(XrefStatus::Unresolved), 0), "{name}: {:?}", s.log); + assert!(s.log.iter().any(|m| m.contains(&format!("\"{name}\" not loaded")) && m.contains("another computer")), "{name}: {:?}", s.log); + } + assert!(s.log.iter().any(|m| m.contains("nested xref \"unc\" not loaded") && m.contains("another computer")), "{:?}", s.log); + if cfg!(unix) { + assert!(s.log.iter().any(|m| m.contains("nested xref \"slashes\" not loaded")), "{:?}", s.log); + } + let r = s.execute("-xref", &json!({ "option": "?" })).unwrap(); + let found: Vec<(&str, bool)> = r["xrefs"].as_array().unwrap().iter().map(|x| (x["name"].as_str().unwrap(), x["foundAt"].is_string())).collect(); + assert!(found.iter().all(|(n, f)| *f == (*n == "part")), "{found:?}"); + assert!(s.execute("xopen", &json!({ "name": "unc" })).unwrap_err().to_string().contains("another computer")); + // Reloading follows the saved path the same way. + s.execute("-xref", &json!({ "option": "reload", "names": "*" })).unwrap(); + assert_eq!(status(&s, "unc"), Some(XrefStatus::Unresolved)); + + // A path the user gives is used as given. + let far = elsewhere.join("far.dxf"); + s.execute("-xref", &json!({ "option": "path", "names": "unc", "path": far.to_str().unwrap() })).unwrap(); + assert_eq!(status(&s, "unc"), Some(XrefStatus::Loaded)); +} From ba02572b31d3b252b1266f647975d6338af05880 Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 17 Oct 2026 02:20:57 +0000 Subject: [PATCH 3/4] Build: use craft-docpath from craft-libs --- Cargo.lock | 1 + Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index cbc5fd45..dbc465e4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -931,6 +931,7 @@ checksum = "7f8f80099a98041a3d1622845c271458a2d73e688351bf3cb999266764b81d48" [[package]] name = "craft-docpath" version = "0.1.0" +source = "git+https://github.com/aiongg/craft-libs?rev=12281d07fedff5c955bb68344fb287aa5d87971e#12281d07fedff5c955bb68344fb287aa5d87971e" [[package]] name = "craft-ui" diff --git a/Cargo.toml b/Cargo.toml index 07dfcb7e..e695bb40 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,7 +25,7 @@ cadcraft-engine = { path = "crates/engine" } cadcraft-ui-egui = { path = "crates/ui-egui" } cadcraft-mcp = { path = "crates/mcp" } # Which paths stored in a drawing (xrefs, data links) may be opened. -craft-docpath = { path = "../secfix-craft-libs/crates/docpath" } +craft-docpath = { git = "https://github.com/aiongg/craft-libs", version = "0.1.0", rev = "12281d07fedff5c955bb68344fb287aa5d87971e" } # foundation thiserror = "2" From 98d22f0c6810e2822f4319e8ce775306cf934cc5 Mon Sep 17 00:00:00 2001 From: skylar taylor-barrick Date: Sat, 17 Oct 2026 12:04:10 -0600 Subject: [PATCH 4/4] Build: take craft-docpath from the merged craft-libs rev craft-libs#169 is merged, so the fork pin is no longer needed. Pinned to storytold/craft-libs 82a24b006eefa036b43c09a67fb9a3fbd17ffe63 (main after the merge) rather than the older 12281d07, which predates resolve_absolute and the Windows colon refusal. --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dbc465e4..fda2807f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -931,7 +931,7 @@ checksum = "7f8f80099a98041a3d1622845c271458a2d73e688351bf3cb999266764b81d48" [[package]] name = "craft-docpath" version = "0.1.0" -source = "git+https://github.com/aiongg/craft-libs?rev=12281d07fedff5c955bb68344fb287aa5d87971e#12281d07fedff5c955bb68344fb287aa5d87971e" +source = "git+https://github.com/storytold/craft-libs?rev=82a24b006eefa036b43c09a67fb9a3fbd17ffe63#82a24b006eefa036b43c09a67fb9a3fbd17ffe63" [[package]] name = "craft-ui" diff --git a/Cargo.toml b/Cargo.toml index e695bb40..873fdf8d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,7 +25,7 @@ cadcraft-engine = { path = "crates/engine" } cadcraft-ui-egui = { path = "crates/ui-egui" } cadcraft-mcp = { path = "crates/mcp" } # Which paths stored in a drawing (xrefs, data links) may be opened. -craft-docpath = { git = "https://github.com/aiongg/craft-libs", version = "0.1.0", rev = "12281d07fedff5c955bb68344fb287aa5d87971e" } +craft-docpath = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "82a24b006eefa036b43c09a67fb9a3fbd17ffe63" } # foundation thiserror = "2"