From ef5084f48be118d17bea6bb51def21d3924e18bd Mon Sep 17 00:00:00 2001 From: aiongg Date: Fri, 16 Oct 2026 17:36:51 +0000 Subject: [PATCH] Links: follow linked file paths only on this computer or the document's share Paths stored with linked images and placed documents, and the hrefs of SVG images, go through fileio::document_path (craft-docpath): a relative path is taken from the document's folder, and only local files or files on the document's own network share are looked at. Opening a document, Link Info, Package, the linked-file watch and SVG import all use it; the watch stats files through fileio, so automation roots apply to it too. A path that is not followed counts as missing; document.open, links.check and links.info say why in `refused`. --- Cargo.lock | 6 ++ Cargo.toml | 1 + crates/engine/Cargo.toml | 2 + crates/engine/src/cmd/fileio/load.rs | 3 + crates/engine/src/cmd/fileio/mod.rs | 55 +++++++++++- crates/engine/src/cmd/links.rs | 92 +++++++++++--------- crates/engine/src/lib.rs | 2 + crates/engine/src/link_watch.rs | 17 ++-- crates/engine/src/tests_link_paths.rs | 120 ++++++++++++++++++++++++++ crates/svg/Cargo.toml | 2 + crates/svg/src/import/files.rs | 74 +++++++++------- docs/mcp.md | 11 ++- 12 files changed, 301 insertions(+), 84 deletions(-) create mode 100644 crates/engine/src/tests_link_paths.rs diff --git a/Cargo.lock b/Cargo.lock index eca47581b..78b0e378b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -806,6 +806,10 @@ dependencies = [ "libc", ] +[[package]] +name = "craft-docpath" +version = "0.1.0" + [[package]] name = "craft-pdf" version = "0.1.0" @@ -4650,6 +4654,7 @@ dependencies = [ name = "vectorcraft-engine" version = "0.12.0" dependencies = [ + "craft-docpath", "crc32fast", "flate2", "image", @@ -4857,6 +4862,7 @@ dependencies = [ name = "vectorcraft-svg" version = "0.12.0" dependencies = [ + "craft-docpath", "flate2", "image", "kurbo 0.13.1", diff --git a/Cargo.toml b/Cargo.toml index 1c10d551b..9536a0b24 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,6 +13,7 @@ repository = "https://github.com/storytold/vectorcraft" [workspace.dependencies] craft-pdf = { git = "https://github.com/storytold/craft-libs", rev = "ca50d63b773c99f96e75c77ce5a1d21e7c9cda0d", default-features = false, features = ["interpret", "write"] } craft-print = { git = "https://github.com/storytold/craft-libs", rev = "d991bf6792f8ade99c51358bf3c1d902c8cd96b5" } +craft-docpath = { path = "../secfix-craft-libs/crates/docpath" } craft-ui = { git = "https://github.com/storytold/craft-libs", rev = "5c874d51eeb3de70e735c8ce87a59a7dedd47675", features = ["serde"] } # internal vectorcraft-geom = { path = "crates/geom" } diff --git a/crates/engine/Cargo.toml b/crates/engine/Cargo.toml index c51728ef2..4fd4fa52f 100644 --- a/crates/engine/Cargo.toml +++ b/crates/engine/Cargo.toml @@ -15,6 +15,8 @@ default = ["bundled-fonts"] bundled-fonts = ["vectorcraft-text/bundled-fonts"] [dependencies] +# Which paths stored in documents may be opened. +craft-docpath = { workspace = true } vectorcraft-geom = { workspace = true } vectorcraft-color = { workspace = true } vectorcraft-doc = { workspace = true } diff --git a/crates/engine/src/cmd/fileio/load.rs b/crates/engine/src/cmd/fileio/load.rs index 58200162c..841a9f825 100644 --- a/crates/engine/src/cmd/fileio/load.rs +++ b/crates/engine/src/cmd/fileio/load.rs @@ -286,8 +286,11 @@ pub(crate) fn import_svg(text: &str, folder: Option<&str>) -> Result<(Document, // The importer reads on a thread of its own (for its stack): what may be read is what may be // read here. let scope = crate::file_access::ScopeHandle::capture(); + // Paths are checked as stored in a document inside `folder` (only the folder is used). + let document = folder.map(|f| std::path::Path::new(f).join("-").to_string_lossy().into_owned()); let read = |path: &str| { scope.clone().run(|| { + let path = &super::document_path(path, document.as_deref()).ok()?; // Only files (not folders or devices) are read. file_stamp(path)?; let bytes = read_file(path).ok()?; diff --git a/crates/engine/src/cmd/fileio/mod.rs b/crates/engine/src/cmd/fileio/mod.rs index 93282d33d..a14545e0d 100644 --- a/crates/engine/src/cmd/fileio/mod.rs +++ b/crates/engine/src/cmd/fileio/mod.rs @@ -820,9 +820,36 @@ fn formats(_: &mut Session, _: &Value) -> Result { /// `path` checked for reading against the automation roots in force. #[cfg(not(target_arch = "wasm32"))] fn may_read(path: &str) -> Result<()> { + #[cfg(test)] + touch(path); crate::file_access::check_read(path).map_err(EngineError::Other) } +/// Paths read while [`touched`] runs, on any thread (the SVG importer reads on one of its own). +#[cfg(test)] +static TOUCHED: std::sync::Mutex> = std::sync::Mutex::new(Vec::new()); +#[cfg(test)] +static RECORDING: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + +#[cfg(test)] +fn touch(path: &str) { + if RECORDING.load(std::sync::atomic::Ordering::SeqCst) > 0 { + TOUCHED.lock().unwrap_or_else(std::sync::PoisonError::into_inner).push(path.to_string()); + } +} + +/// Run `f` and list the paths read through this module meanwhile (by any test running then). +#[cfg(test)] +pub(crate) fn touched(f: impl FnOnce() -> T) -> (T, Vec) { + use std::sync::atomic::Ordering::SeqCst; + let start = TOUCHED.lock().unwrap_or_else(std::sync::PoisonError::into_inner).len(); + RECORDING.fetch_add(1, SeqCst); + let out = f(); + RECORDING.fetch_sub(1, SeqCst); + let seen = TOUCHED.lock().unwrap_or_else(std::sync::PoisonError::into_inner).get(start..).unwrap_or_default().to_vec(); + (out, seen) +} + /// `path` checked for writing against the automation roots in force. #[cfg(not(target_arch = "wasm32"))] fn may_write(path: &str) -> Result<()> { @@ -977,8 +1004,7 @@ fn list_dir(_: &str, _: usize) -> Result<(Vec, bool)> { /// keeps one); `None` when there is no file at `path` (or automation may not read it). #[cfg(not(target_arch = "wasm32"))] pub(crate) fn file_stamp(path: &str) -> Option<(u64, Option)> { - may_read(path).ok()?; - let m = std::fs::metadata(path).ok().filter(std::fs::Metadata::is_file)?; + let m = file_metadata(path).filter(std::fs::Metadata::is_file)?; let modified = m.modified().ok().and_then(|t| t.duration_since(std::time::UNIX_EPOCH).ok()).map(|d| d.as_millis() as u64); Some((m.len(), modified)) } @@ -987,11 +1013,17 @@ pub(crate) fn file_stamp(path: &str) -> Option<(u64, Option)> { /// (and automation may read it). #[cfg(not(target_arch = "wasm32"))] pub(crate) fn file_created(path: &str) -> Option { - may_read(path).ok()?; - let t = std::fs::metadata(path).ok()?.created().ok()?; + let t = file_metadata(path)?.created().ok()?; t.duration_since(std::time::UNIX_EPOCH).ok().map(|d| d.as_millis() as u64) } +/// The file system's metadata of `path`, following links (when automation may read it). +#[cfg(not(target_arch = "wasm32"))] +pub(crate) fn file_metadata(path: &str) -> Option { + may_read(path).ok()?; + std::fs::metadata(path).ok() +} + #[cfg(target_arch = "wasm32")] pub(crate) fn file_created(_: &str) -> Option { None @@ -1007,6 +1039,21 @@ pub(crate) fn absolute_path(path: &str) -> String { std::path::absolute(path).map_or_else(|_| path.to_string(), |p| p.to_string_lossy().into_owned()) } +/// The file to open for `stored`, a path or `file:` URL stored in a document (a linked file, an +/// SVG image's href), or why it is not followed. `document` is the path of the document it is +/// stored in, when it has one: a relative path is taken from its folder. Only files on this +/// computer, or on the network share the document is on, are followed (see `craft_docpath`); a +/// path that is still relative (no document) is refused, as the working directory isn't the +/// document's choice. Every stored path reaches the file system through this. +pub(crate) fn document_path(stored: &str, document: Option<&str>) -> std::result::Result { + let document = document.map(absolute_path); + let path = craft_docpath::resolve(stored, document.as_deref().map(std::path::Path::new))?; + if !path.is_absolute() { + return Err(craft_docpath::Refused::Invalid); + } + path.into_os_string().into_string().map_err(|_| craft_docpath::Refused::Invalid) +} + #[cfg(target_arch = "wasm32")] pub(crate) fn file_stamp(_: &str) -> Option<(u64, Option)> { None diff --git a/crates/engine/src/cmd/links.rs b/crates/engine/src/cmd/links.rs index 3b72828dc..99eb2af88 100644 --- a/crates/engine/src/cmd/links.rs +++ b/crates/engine/src/cmd/links.rs @@ -39,7 +39,7 @@ pub fn specs() -> Vec { "Check Links", [], None, - "{ids?: [image or placed document ids] (default: every linked image and placed document)} where each linked file is and whether it changed since it was read → {links: [{name, path, status: ok|modified|missing, ids, found?: path (found away from its path: relative to the document, or by name in its folder), preview: true (showing the low-resolution preview, not the file)}], missing, modified}", + "{ids?: [image or placed document ids] (default: every linked image and placed document)} where each linked file is and whether it changed since it was read → {links: [{name, path, status: ok|modified|missing, ids, found?: path (found away from its path: relative to the document, or by name in its folder), preview: true (showing the low-resolution preview, not the file), refused?: why the path is not followed (another computer, a device or a URL: it counts as missing)}], missing, modified}", has_doc, check ), @@ -111,7 +111,7 @@ pub fn specs() -> Vec { "Link Info", [], None, - "{id?} an image's or placed document's Link Info (default: the one selected) → image.info's fields (id, name, linked, link, colorMode, pixelWidth, pixelHeight, width, height) and status: ok|modified|missing|embedded, format, ppi: [x, y] (the file's), effectivePpi: [x, y] (at its placed size), scale: [x%, y%] (of its 100% size), rotation (degrees, counter-clockwise), placement: {preserve, align, clip}; linked images also fileName, location (its folder), page?, fileSize? (bytes), modified?, created? (ms since the Unix epoch); a placed document has document: true, pageWidth and pageHeight (pt) instead of pixels, ppi and colorMode, and its scale is of its artboard's 100% size", + "{id?} an image's or placed document's Link Info (default: the one selected) → image.info's fields (id, name, linked, link, colorMode, pixelWidth, pixelHeight, width, height) and status: ok|modified|missing|embedded, format, ppi: [x, y] (the file's), effectivePpi: [x, y] (at its placed size), scale: [x%, y%] (of its 100% size), rotation (degrees, counter-clockwise), placement: {preserve, align, clip}; linked images also fileName, location (its folder), page?, fileSize? (bytes), modified?, created? (ms since the Unix epoch), refused? (why the path is not followed); a placed document has document: true, pageWidth and pageHeight (pt) instead of pixels, ppi and colorMode, and its scale is of its artboard's 100% size", has_doc, info ), @@ -245,21 +245,24 @@ fn found_at(path: String) -> Option { Some(Found { path, size, modified, bytes: None }) } -/// Where the file of `link` is: at its path, else at its relative path or by name in the -/// document's folder `dir`. -fn locate(link: &LinkInfo, dir: Option<&Path>) -> Option { - let mut paths = vec![link.path.clone()]; - if let Some(dir) = dir { - let near = link.relative.iter().map(|r| dir.join(r)).chain([dir.join(link.name())]); - paths.extend(near.map(|p| p.to_string_lossy().into_owned())); - } - paths.into_iter().find_map(found_at) +/// Where the file of `link` (stored in the document saved at `doc_path`) is: at its path, else at +/// its relative path or by name in the document's folder. Only paths +/// [`fileio::document_path`] follows are looked at. +fn locate(link: &LinkInfo, doc_path: Option<&str>) -> Option { + let stored = [Some(link.path.as_str()), link.relative.as_deref(), Some(link.name())]; + stored.into_iter().flatten().filter_map(|p| fileio::document_path(p, doc_path).ok()).find_map(found_at) } -/// Where the file of group `g` is and whether it still has the content shown. Its bytes are read -/// only when its size or modification time changed. -fn probe(g: &Group, dir: Option<&Path>) -> (Status, Option) { - let Some(mut f) = locate(&g.link, dir) else { return (Status::Missing, None) }; +/// Why the path of `link` (stored in the document saved at `doc_path`) isn't followed, when it +/// names another computer, a device or a URL. +fn refused(link: &LinkInfo, doc_path: Option<&str>) -> Option { + fileio::document_path(&link.path, doc_path).err().filter(|r| *r != craft_docpath::Refused::Invalid) +} + +/// Where the file of group `g` (of the document saved at `doc_path`) is and whether it still has +/// the content shown. Its bytes are read only when its size or modification time changed. +fn probe(g: &Group, doc_path: Option<&str>) -> (Status, Option) { + let Some(mut f) = locate(&g.link, doc_path) else { return (Status::Missing, None) }; let l = &g.link; if l.size == Some(f.size) && l.modified.is_some() && l.modified == f.modified { return (Status::Ok, Some(f)); @@ -274,11 +277,6 @@ fn probe(g: &Group, dir: Option<&Path>) -> (Status, Option) { (if same { Status::Ok } else { Status::Modified }, Some(f)) } -/// The folder of the document saved at `doc_path`. -fn folder(doc_path: Option<&str>) -> Option<&Path> { - doc_path.and_then(|p| Path::new(p).parent()) -} - /// Is image `key` of `d` showing its preview (its file not read)? fn previewing(d: &Document, key: &str) -> bool { d.images.get(key).is_none_or(ImageBlob::is_proxy) @@ -314,7 +312,7 @@ pub(crate) fn linked_files(d: &Document, doc_path: Option<&str>) -> Vec) -> Vec) -> Vec<(LinkInfo, Vec)> { groups(&st.doc, ids) .into_iter() - .filter(|g| !g.ids.is_empty() && (ids.is_some() || probe(g, folder(st.path.as_deref())).0 == Status::Missing)) + .filter(|g| !g.ids.is_empty() && (ids.is_some() || probe(g, st.path.as_deref()).0 == Status::Missing)) .map(|g| (g.link, g.ids)) .collect() } @@ -510,9 +508,23 @@ fn install(d: &mut Document, ids: &[NodeId], (link, img): FileRead, force: Optio clips.into_iter().try_for_each(|(id, clip)| clip_image(d, id, clip)) } -/// Groups as `{name, path, ids}` rows (those with objects in the layers). -fn rows<'a>(gs: impl Iterator) -> Vec { - gs.filter(|g| !g.ids.is_empty()).map(|g| json!({ "name": g.link.name(), "path": g.link.path, "ids": ids_json(&g.ids) })).collect() +/// Groups (of the document saved at `doc_path`) as `{name, path, ids}` rows (those with objects in +/// the layers), with `refused`: why the path isn't followed, when it isn't. +fn rows<'a>(gs: impl Iterator, doc_path: Option<&str>) -> Vec { + gs.filter(|g| !g.ids.is_empty()) + .map(|g| { + let mut row = json!({ "name": g.link.name(), "path": g.link.path, "ids": ids_json(&g.ids) }); + note_refused(&mut row, &g.link, doc_path); + row + }) + .collect() +} + +/// `row["refused"]`: why the path of `link` isn't followed, when it isn't ([`refused`]). +fn note_refused(row: &mut Value, link: &LinkInfo, doc_path: Option<&str>) { + if let Some(r) = refused(link, doc_path) { + row["refused"] = json!(r.to_string()); + } } fn ids_json(ids: &[NodeId]) -> Vec { @@ -545,9 +557,9 @@ impl Resolved { pub fn resolve(d: &mut Document, doc_path: Option<&str>, update: bool) -> Resolved { let mut out = Resolved::default(); for g in groups(d, None) { - let row = rows(std::iter::once(&g)); + let row = rows(std::iter::once(&g), doc_path); let unread = g.document.is_none() && g.link.hash.is_none() && g.link.size.is_none() && previewing(d, &g.key); - match probe(&g, folder(doc_path)) { + match probe(&g, doc_path) { (Status::Missing, _) => out.missing.extend(row), (Status::Modified, Some(mut f)) if update || unread => { match read_for(&mut f, &g, "document.open").and_then(|read| install_read(d, &g.ids, read, unread.then_some(Preserve::Bounds))) { @@ -714,10 +726,11 @@ fn check(s: &mut Session, p: &Value) -> Result { let (mut missing, mut modified) = (0, 0); let mut links = vec![]; for g in groups(&st.doc, ids.as_deref()).iter().filter(|g| !g.ids.is_empty()) { - let (status, found) = probe(g, folder(st.path.as_deref())); + let (status, found) = probe(g, st.path.as_deref()); missing += usize::from(status == Status::Missing); modified += usize::from(status == Status::Modified); let mut row = json!({ "name": g.link.name(), "path": g.link.path, "status": status.as_str(), "ids": ids_json(&g.ids) }); + note_refused(&mut row, &g.link, st.path.as_deref()); if let Some(f) = found.filter(|f| f.path != g.link.path) { row["found"] = json!(f.path); } @@ -734,7 +747,7 @@ fn update(s: &mut Session, p: &Value) -> Result { let ids = ids_param(p, "ids"); let (mut reads, mut missing) = (vec![], vec![]); for g in groups(&st.doc, ids.as_deref()).into_iter().filter(|g| !g.ids.is_empty()) { - match probe(&g, folder(st.path.as_deref())) { + match probe(&g, st.path.as_deref()) { (Status::Missing, _) | (_, None) => missing.extend(ids_json(&g.ids)), // Unchanged and showing the file: nothing to read unless asked for by id. (Status::Ok, Some(_)) if ids.is_none() && !stale(&st.doc, &g) => {} @@ -783,11 +796,9 @@ fn relink(s: &mut Session, p: &Value) -> Result { None => { let selected: Vec = st.selection.objects.iter().copied().filter(|id| image(id).is_some()).collect(); match folder_param { - Some(_) if selected.is_empty() => groups(&st.doc, None) - .into_iter() - .filter(|g| probe(g, folder(st.path.as_deref())).0 == Status::Missing) - .flat_map(|g| g.ids) - .collect(), + Some(_) if selected.is_empty() => { + groups(&st.doc, None).into_iter().filter(|g| probe(g, st.path.as_deref()).0 == Status::Missing).flat_map(|g| g.ids).collect() + } _ => selected, } } @@ -969,7 +980,7 @@ fn linked_param(st: &crate::DocState, p: &Value, cmd: &str) -> Result { fn statuses(st: &crate::DocState) -> BTreeMap, bool)> { let mut out = BTreeMap::new(); for g in groups(&st.doc, None).iter().filter(|g| !g.ids.is_empty()) { - let (status, found) = probe(g, folder(st.path.as_deref())); + let (status, found) = probe(g, st.path.as_deref()); let found = found.map(|f| f.path).filter(|p| *p != g.link.path); let preview = stale(&st.doc, g); out.extend(g.ids.iter().map(|id| (*id, (status, found.clone(), preview)))); @@ -1110,7 +1121,7 @@ fn embed(s: &mut Session, p: &Value) -> Result { done.extend(g.ids); continue; } - match probe(&g, folder(st.path.as_deref())) { + match probe(&g, st.path.as_deref()) { (Status::Ok | Status::Modified, Some(mut f)) => match read_image(&mut f, &g.link) { Ok(read) => { done.extend(&g.ids); @@ -1190,10 +1201,13 @@ fn placement_json(o: PlacementOptions) -> Value { /// Link Info's fields of the linked file `l` (of object `id` of `st`) into `out`. fn file_fields(st: &crate::DocState, id: NodeId, l: &LinkInfo, out: &mut Value) { - let status = groups(&st.doc, Some(&[id])).first().map_or(Status::Missing, |g| probe(g, folder(st.path.as_deref())).0); - let stamp = file_stamp(&l.path); + let status = groups(&st.doc, Some(&[id])).first().map_or(Status::Missing, |g| probe(g, st.path.as_deref()).0); + let doc_path = st.path.as_deref(); + let path = fileio::document_path(&l.path, doc_path).ok(); + let stamp = path.as_deref().and_then(file_stamp); let folder_len = l.path.len().saturating_sub(l.name().len()); out["status"] = json!(status.as_str()); + note_refused(out, l, doc_path); out["path"] = json!(l.path); out["fileName"] = json!(l.name()); out["location"] = json!(l.path.get(..folder_len).unwrap_or_default().trim_end_matches(['/', '\\'])); @@ -1201,7 +1215,7 @@ fn file_fields(st: &crate::DocState, id: NodeId, l: &LinkInfo, out: &mut Value) ("page", l.page.map(u64::from)), ("fileSize", stamp.map(|s| s.0).or(l.size)), ("modified", stamp.and_then(|s| s.1).or(l.modified)), - ("created", file_created(&l.path)), + ("created", path.as_deref().and_then(file_created)), ] { if let Some(v) = v { out[k] = json!(v); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index ced1ccef7..8f2fdcfbd 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -1991,6 +1991,8 @@ mod tests_legacy_ai; #[cfg(test)] mod tests_library; #[cfg(test)] +mod tests_link_paths; +#[cfg(test)] mod tests_linked_stops; #[cfg(test)] mod tests_linkfiles; diff --git a/crates/engine/src/link_watch.rs b/crates/engine/src/link_watch.rs index 8d9df187e..94fc568f4 100644 --- a/crates/engine/src/link_watch.rs +++ b/crates/engine/src/link_watch.rs @@ -30,15 +30,19 @@ pub type Stamp = (u64, u128); /// object ids, stamp)]). pub type LinkScan = Arc, Option)>)>>>; +/// The stamp of the linked file stored as `path` in the document saved at `doc_path`; `None` when +/// there is no file, its path isn't followed ([`crate::cmd::fileio::document_path`]) or +/// automation may not read it. #[cfg(not(target_arch = "wasm32"))] -fn stamp(path: &str) -> Option { - let meta = std::fs::metadata(path).ok()?; +fn stamp(path: &str, doc_path: Option<&str>) -> Option { + let path = crate::cmd::fileio::document_path(path, doc_path).ok()?; + let meta = crate::cmd::fileio::file_metadata(&path)?; let modified = meta.modified().ok()?.duration_since(std::time::UNIX_EPOCH).ok()?; Some((meta.len(), modified.as_nanos())) } #[cfg(target_arch = "wasm32")] -fn stamp(_: &str) -> Option { +fn stamp(_: &str, _: Option<&str>) -> Option { None } @@ -52,7 +56,7 @@ pub fn linked_files(d: &vectorcraft_doc::Document) -> Vec<(String, Vec)> { pub(crate) fn update_changed(s: &mut Session, _: &Value) -> Result { let st = s.doc()?; let (uid, files) = (st.uid, crate::cmd::links::linked_groups(&st.doc)); - let now = files.into_iter().map(|(p, ids)| (p.clone(), ids, stamp(&p))).collect(); + let now = files.into_iter().map(|(p, ids)| (p.clone(), ids, stamp(&p, st.path.as_deref()))).collect(); s.apply_link_stamps(uid, now, true) } @@ -64,14 +68,15 @@ impl Session { return; } let Some(st) = self.active() else { return }; - let (uid, files) = (st.uid, crate::cmd::links::linked_groups(&st.doc)); + let (uid, files, doc_path) = (st.uid, crate::cmd::links::linked_groups(&st.doc), st.path.clone()); if files.is_empty() { return; } let slot: LinkScan = Arc::default(); let out = slot.clone(); + let scope = crate::file_access::ScopeHandle::capture(); let spawned = std::thread::Builder::new().name("link-watch".into()).spawn(move || { - let stamps = files.into_iter().map(|(p, ids)| (p.clone(), ids, stamp(&p))).collect(); + let stamps = scope.run(|| files.into_iter().map(|(p, ids)| (p.clone(), ids, stamp(&p, doc_path.as_deref()))).collect()); *out.lock().unwrap_or_else(PoisonError::into_inner) = Some((uid, stamps)); }); if spawned.is_ok() { diff --git a/crates/engine/src/tests_link_paths.rs b/crates/engine/src/tests_link_paths.rs new file mode 100644 index 000000000..cd15f092b --- /dev/null +++ b/crates/engine/src/tests_link_paths.rs @@ -0,0 +1,120 @@ +//! Paths stored with linked files and SVG images are followed only on this computer or on the +//! document's own share (`fileio::document_path`): opening a document, Link Info, watching linked +//! files, and the images an SVG file links to. + +use serde_json::{Value, json}; +use vectorcraft_doc::{NodeId, NodeKind}; + +use super::*; +use crate::cmd::fileio::touched; +use crate::tests_links::{Folder, RED, place, png, session}; + +/// Paths to other computers (and, on Unix, a local file written as one: `//tmp/…`, which Linux +/// reads as `/tmp/…`). +fn far(dir: &Folder) -> Vec { + let mut out = vec![r"\\192.0.2.1\share\x.png".to_string(), "file://192.0.2.1/s/x.png".to_string()]; + if cfg!(unix) { + out.push(format!("/{}", dir.file("far/far.png"))); + } + out +} + +/// Did any path read reach another computer (or the local file written as one)? +fn far_reads(seen: &[String]) -> Vec<&String> { + seen.iter().filter(|p| p.contains("192.0.2.1") || p.starts_with("//") || p.starts_with(r"\\")).collect() +} + +/// A document saved at `dir/doc.vectorcraft` with one linked image per path in `paths`, each +/// showing `dir/pic.png` until its link is set → the document's path. +fn save_linked(dir: &Folder, paths: &[String]) -> String { + let pic = dir.file("pic.png"); + std::fs::write(&pic, png(8, 8, RED)).unwrap(); + let mut s = session(); + let ids: Vec = paths.iter().map(|_| place(&mut s, &pic)).collect(); + let mut doc = (*s.doc().unwrap().doc).clone(); + for (id, path) in ids.iter().zip(paths) { + let Some(NodeKind::Image(im)) = doc.node_mut(*id).map(|n| &mut n.kind) else { panic!("not an image") }; + im.link = Some(vectorcraft_doc::LinkInfo::new(path.clone())); + } + let path = dir.file("doc.vectorcraft"); + std::fs::write(&path, vectorcraft_format::save(&doc, false)).unwrap(); + path +} + +fn names(rows: &Value) -> Vec<&str> { + rows.as_array().unwrap().iter().map(|r| r["path"].as_str().unwrap()).collect() +} + +#[test] +fn linked_files_on_other_computers_are_never_read() { + let dir = Folder::new("docpath-open"); + std::fs::create_dir_all(dir.0.join("far")).unwrap(); + std::fs::write(dir.file("far/far.png"), png(8, 8, RED)).unwrap(); + std::fs::write(dir.file("near.png"), png(8, 8, RED)).unwrap(); + let far = far(&dir); + let mut paths = far.clone(); + paths.push("near.png".into()); + let doc = save_linked(&dir, &paths); + + let mut s = Session::new(); + let (r, seen) = touched(|| s.execute("document.open", &json!({"path": doc})).unwrap()); + assert_eq!(far_reads(&seen), Vec::<&String>::new()); + // The far paths are missing, saying why; the file beside the document is found. + assert_eq!(names(&r["missingLinks"]), far.iter().map(String::as_str).collect::>(), "{r}"); + for row in r["missingLinks"].as_array().unwrap() { + assert_eq!(row["refused"], "the path is on another computer", "{row}"); + } + let check = s.execute("links.check", &json!({})).unwrap(); + let near = check["links"].as_array().unwrap().iter().find(|l| l["name"] == "near.png").unwrap(); + assert_ne!(near["status"], "missing", "{check}"); + + // Link Info and the linked-file watch don't look at them either. + let ids: Vec = r["missingLinks"].as_array().unwrap().iter().map(|row| row["ids"][0].as_u64().unwrap()).collect(); + let (_, seen) = touched(|| { + for id in &ids { + let info = s.execute("links.info", &json!({"id": id})).unwrap(); + assert_eq!(info["refused"], "the path is on another computer", "{info}"); + } + s.execute("links.updateChanged", &json!({})).unwrap(); + s.start_link_scan(); + let t0 = std::time::Instant::now(); + while s.poll_link_scan().is_none() { + assert!(t0.elapsed().as_secs() < 10, "the look never finished"); + std::thread::yield_now(); + } + }); + assert_eq!(far_reads(&seen), Vec::<&String>::new()); + // The watch kept no stamp of them (`//tmp/…` would have one had it been looked at). + assert!(s.link_stamps.keys().all(|(_, p)| !far.contains(p)), "{:?}", s.link_stamps.keys()); +} + +#[test] +fn svg_images_on_other_computers_are_never_read() { + let dir = Folder::new("docpath-svg"); + std::fs::create_dir_all(dir.0.join("far")).unwrap(); + std::fs::write(dir.file("far/far.png"), png(8, 8, RED)).unwrap(); + std::fs::write(dir.file("near.png"), png(8, 8, RED)).unwrap(); + let mut hrefs = vec![r"\\192.0.2.1\share\x.png".to_string(), "file:////192.0.2.1/s/x.png".to_string(), "file://192.0.2.1/s/x.png".to_string()]; + if cfg!(unix) { + hrefs.push(format!("/{}", dir.file("far/far.png"))); + } + hrefs.push("near.png".into()); + let images: String = hrefs.iter().map(|h| format!("")).collect(); + let svg = format!("{images}"); + let path = dir.file("art.svg"); + std::fs::write(&path, &svg).unwrap(); + + let mut s = Session::new(); + let (r, seen) = touched(|| s.execute("document.open", &json!({"path": path})).unwrap()); + assert_eq!(far_reads(&seen), Vec::<&String>::new()); + // Every far image is a placeholder; only the one beside the SVG is read. + let warnings = r["warnings"].as_array().unwrap(); + assert_eq!(warnings.iter().filter(|w| w.as_str().unwrap().contains("a placeholder keeps its place")).count(), hrefs.len() - 1, "{r}"); + let near = |p: &&String| p.contains("docpath-svg") && p.ends_with("near.png"); + assert!(seen.iter().any(|p| near(&p)), "{seen:?}"); + + // Pasted SVG text has no folder: only absolute local paths are read. + let (_, seen) = touched(|| crate::cmd::fileio::import_svg(&svg, None).unwrap()); + assert_eq!(far_reads(&seen), Vec::<&String>::new()); + assert!(!seen.iter().any(|p| near(&p)), "{seen:?}"); +} diff --git a/crates/svg/Cargo.toml b/crates/svg/Cargo.toml index 7b679365f..27be13e20 100644 --- a/crates/svg/Cargo.toml +++ b/crates/svg/Cargo.toml @@ -9,6 +9,8 @@ description = "VectorCraft SVG import and export" workspace = true [dependencies] +# Which paths stored in documents may be opened. +craft-docpath = { workspace = true } vectorcraft-geom = { workspace = true } vectorcraft-color = { workspace = true } vectorcraft-doc = { workspace = true } diff --git a/crates/svg/src/import/files.rs b/crates/svg/src/import/files.rs index 886072778..b18d5595c 100644 --- a/crates/svg/src/import/files.rs +++ b/crates/svg/src/import/files.rs @@ -79,33 +79,25 @@ fn percent_decoded(s: &str) -> String { String::from_utf8_lossy(&out).into_owned() } -/// Is `p` an absolute path here or on another system (`/…`, `C:/…`, `C:\…`)? -fn is_absolute(p: &str) -> bool { - Path::new(p).is_absolute() || p.starts_with('/') || matches!(p.as_bytes(), [d, b':', b'/' | b'\\', ..] if d.is_ascii_alphabetic()) -} - -/// Where href `h` points (a path, made absolute from `folder`, or a URL) and whether it is a -/// file that can be read. +/// Where href `h` points (a path, or a URL) and whether it is a file that may be read: a local +/// file, or one on the network share the SVG is on. A relative path is taken from `folder`, the +/// SVG's own; without it, only absolute paths are read (see `craft_docpath`). fn target(h: &str, folder: Option<&str>) -> (String, bool) { - let h = percent_decoded(h.trim()); - let path = match h.strip_prefix("file://") { - Some(rest) => { - let rest = rest.strip_prefix("localhost").unwrap_or(rest); - // `file:///C:/art.png` → `C:/art.png`. - match rest.as_bytes() { - [b'/', d, b':', ..] if d.is_ascii_alphabetic() => rest.get(1..).unwrap_or(rest).to_string(), - _ => rest.to_string(), - } - } - None if h.contains("://") => return (h, false), - None => h, - }; - if is_absolute(&path) { - return (path, true); + let h = h.trim(); + // `craft_docpath` decodes a `file:` URL itself; any other href is decoded here, once. + let file_url = h.get(..5).is_some_and(|s| s.eq_ignore_ascii_case("file:")); + let stored = if file_url { h.to_string() } else { percent_decoded(h) }; + if !file_url && stored.contains("://") { + return (stored, false); } - match folder { - Some(f) => (Path::new(f).join(&path).to_string_lossy().into_owned(), true), - None => (path, false), + // Resolved as a document inside `folder`: only the folder is used. + let document = folder.map(|f| Path::new(f).join("-")); + match craft_docpath::resolve(&stored, document.as_deref()) { + Ok(path) => { + let local = path.is_absolute(); + (path.to_string_lossy().into_owned(), local) + } + Err(_) => (stored, false), } } @@ -235,12 +227,32 @@ mod tests { #[test] fn hrefs_resolve_against_the_folder() { let sep = std::path::MAIN_SEPARATOR; - assert_eq!(target("photo%20one.png", Some("/art")), (format!("/art{sep}photo one.png"), true)); - assert_eq!(target("photo.png", None), ("photo.png".into(), false)); - assert_eq!(target("/abs/photo.png", Some("/art")), ("/abs/photo.png".into(), true)); - assert_eq!(target("file:///C:/art/a.png", None), ("C:/art/a.png".into(), true)); - assert_eq!(target("file:///home/a.png", None), ("/home/a.png".into(), true)); - assert_eq!(target("https://example.com/a.png", Some("/art")), ("https://example.com/a.png".into(), false)); + let art = std::env::temp_dir().join("art"); + let art = art.to_str().unwrap(); + assert_eq!(target("photo%20one.png", Some(art)), (format!("{art}{sep}photo one.png"), true)); + assert!(!target("photo.png", None).1); + assert_eq!(target("file:///C:/art/a.png", None), (r"C:\art\a.png".into(), cfg!(windows))); + assert_eq!(target("https://example.com/a.png", Some(art)), ("https://example.com/a.png".into(), false)); + #[cfg(unix)] + { + assert_eq!(target("/abs/photo.png", Some("/art")), ("/abs/photo.png".into(), true)); + assert_eq!(target("file:///home/a.png", None), ("/home/a.png".into(), true)); + } + } + + #[test] + fn hrefs_to_other_computers_are_not_read() { + let art = std::env::temp_dir().join("art"); + for h in [ + r"\\192.0.2.1\share\x.png", + "//192.0.2.1/share/x.png", + "file:////192.0.2.1/share/x.png", + "file://192.0.2.1/share/x.png", + "%5C%5C192.0.2.1%5Cshare%5Cx.png", + ] { + assert!(!target(h, art.to_str()).1, "{h}"); + assert!(!target(h, None).1, "{h}"); + } } #[test] diff --git a/docs/mcp.md b/docs/mcp.md index 1e8e9875b..0c329acb1 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -466,7 +466,8 @@ symbol with strokes) stays art. `feGaussianBlur`, `feDropShadow` and the usual s colour matrix, composite, merge) become Gaussian Blur, Drop Shadow, Outer Glow, Inner Glow and Feather effects; other filters are listed in `warnings`. Undisplayed objects (`display:none`) come back hidden, nested clip paths intersect, and reflected or repeated gradients (`spreadMethod`) are expanded into stops. `` files (relative links are -found in the SVG's folder) stay linked images (see Linked images); SVG files and `data:` SVGs become art; a file that +found in the SVG's folder; only files on this computer or the SVG's network share are read, and pasted SVG reads only +absolute paths) stay linked images (see Linked images); SVG files and `data:` SVGs become art; a file that can't be read becomes a placeholder in its box, named in `warnings` and `missingLinks`. Native files: `run_command document.save {compress: true}` writes a gzip-compressed `.vectorcraft` (the @@ -2372,9 +2373,11 @@ A placed image linked to its file (`file.place {path}`, Link on) records the fil modification time and hash, and keeps a low-resolution preview (at most 256 px a side): a `.vectorcraft` save writes the preview instead of the pixels, plus the path relative to the saved file. `document.open` reads the linked files again, looking for each at its path, then at its relative path and by name in the document's folder -(so a folder moved with its links still opens): the result lists `missingLinks` (their images show the preview), -`modifiedLinks` (left as they were; read again only with the preference `updateLinks: "automatically"`, then they are -in `updatedLinks`), each as `{name, path, ids}`. The desktop app also checks the active document's links when its window +(so a folder moved with its links still opens). Only files on this computer, or on the network share the document is +on, are looked at: a path to another computer, a device or a URL counts as missing. The result lists `missingLinks` +(their images show the preview), `modifiedLinks` (left as they were; read again only with the preference +`updateLinks: "automatically"`, then they are in `updatedLinks`), each as `{name, path, ids}`, with `refused` (why the +path isn't followed) when it isn't. The desktop app also checks the active document's links when its window comes back to the front, following the same preference. `links.check` reports every link's `status` (`ok`, `modified`, `missing`), `links.update {ids?}` reads modified files again and `links.relink {ids?, path | folder}` points images at another file (or each at the file of its name in a folder); images keep their bounds, one undo step each. Without a