From fb68b7cc2d7f3b0257ef775108cb550d2e680103 Mon Sep 17 00:00:00 2001 From: DWAA1660 Date: Fri, 9 Oct 2026 23:50:58 -0600 Subject: [PATCH] M5: rebind embedded image links when opening relocated packages (#235) (#244) Reuse packaged-link candidates for embedded IDML images and native files. Resolve missing original paths beside the opened document, retaining the embedded image bytes and any original link that still exists. Bound path lookup allocations and cover actual package relocation through commands. Make existing data-merge test path assertions portable to Windows. Co-authored-by: Codex --- crates/engine/src/cmd/datamerge/tests.rs | 5 +- crates/engine/src/cmd/file.rs | 1 + crates/engine/src/cmd/interchange.rs | 88 ++++++++++++++++++++++-- crates/engine/src/cmd/package.rs | 25 +++++++ 4 files changed, 111 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/cmd/datamerge/tests.rs b/crates/engine/src/cmd/datamerge/tests.rs index 6153a1d61..64f2f5d08 100644 --- a/crates/engine/src/cmd/datamerge/tests.rs +++ b/crates/engine/src/cmd/datamerge/tests.rs @@ -714,7 +714,8 @@ fn relative_path_and_reopen() { std::fs::create_dir_all(&b).unwrap(); let csv = b.join("data.csv"); write(&csv, "Name\nAda\n"); - assert_eq!(relative_between(&a, &csv), "../b/data.csv"); + let relative = Path::new("..").join("b").join("data.csv").to_string_lossy().into_owned(); + assert_eq!(relative_between(&a, &csv), relative); assert_eq!(relative_between(&b, &csv), "data.csv"); let mut doc = designcraft_doc::Document::new(&designcraft_doc::build::NewDocument::default()); @@ -743,7 +744,7 @@ fn relative_path_and_reopen() { let mut s2 = Session::new(); exec(&mut s2, "file.open", json!({"path": saved})); let src = &s2.documents()[s2.active_index().unwrap()].doc.data_merge.sources[0]; - assert_eq!(src.relative_path.as_deref(), Some("../b/data.csv")); + assert_eq!(src.relative_path.as_deref(), Some(relative.as_str())); assert_eq!(src.rows[0][0], "Ada"); } diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 267ea5d88..d475cafc5 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -164,6 +164,7 @@ fn file_open(s: &mut Session, p: &Value) -> Result { } let bytes = std::fs::read(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; let mut d = from_bytes(&bytes)?; + super::interchange::resolve_packaged_links(&mut d, std::path::Path::new(path).parent()); super::datamerge::resolve_sources_on_open(&mut d, Some(std::path::Path::new(path))); let (fonts, faces, warnings) = load_document_fonts(&mut d, path); let mut st = DocState::new(d, Some(path.to_string())); diff --git a/crates/engine/src/cmd/interchange.rs b/crates/engine/src/cmd/interchange.rs index 030cf1b6c..8a65ec913 100644 --- a/crates/engine/src/cmd/interchange.rs +++ b/crates/engine/src/cmd/interchange.rs @@ -59,13 +59,22 @@ pub fn import(bytes: &[u8], dir: Option<&std::path::Path>) -> Result { std::sync::Arc::make_mut(asset).link = Some(path); } } + resolve_packaged_links(&mut document, dir); document }; Ok(document) } #[cfg(not(target_arch = "wasm32"))] -fn read_packaged_link(link: &str, dir: Option<&std::path::Path>) -> Option<(String, Vec)> { +const MAX_LINK_PATH_BYTES: usize = 128 * 1024; + +#[cfg(not(target_arch = "wasm32"))] +fn packaged_link_candidates(link: &str, dir: Option<&std::path::Path>) -> Vec { + // Enough for a maximum-length Windows path even with four-byte UTF-8 characters. + // Bound splitting/allocation for link strings supplied by imported documents. + if link.len() > MAX_LINK_PATH_BYTES { + return Vec::new(); + } let mut candidates = vec![std::path::PathBuf::from(link)]; if let Some(dir) = dir { // IDML may retain Windows paths even when the package is opened on another platform. @@ -87,7 +96,29 @@ fn read_packaged_link(link: &str, dir: Option<&std::path::Path>) -> Option<(Stri candidates.push(dir.join("Links").join(name)); } } - for path in candidates { + candidates +} + +/// Embedded images do not call the IDML resource loader. Rebind their missing links +/// beside the opened document too, without replacing their embedded image bytes. +#[cfg(not(target_arch = "wasm32"))] +pub(crate) fn resolve_packaged_links(document: &mut Document, dir: Option<&std::path::Path>) { + let Some(dir) = dir else { return }; + for asset in document.assets.values_mut() { + let Some(link) = asset.link.as_deref() else { continue }; + if link.len() > MAX_LINK_PATH_BYTES || std::path::Path::new(link).is_file() { + continue; + } + let Some(path) = packaged_link_candidates(link, Some(dir)).into_iter().find(|p| p.is_file()) else { continue }; + // Preserve ordinary Windows paths rather than canonicalizing to a verbatim URI. + let path = std::path::absolute(&path).unwrap_or(path); + std::sync::Arc::make_mut(asset).link = Some(path.to_string_lossy().into_owned()); + } +} + +#[cfg(not(target_arch = "wasm32"))] +fn read_packaged_link(link: &str, dir: Option<&std::path::Path>) -> Option<(String, Vec)> { + for path in packaged_link_candidates(link, dir) { if let Ok(bytes) = std::fs::read(&path) { // Keep ordinary Windows drive/UNC syntax for IDML URI export; canonicalize // would introduce a verbatim prefix that is not an IDML file URI. @@ -132,7 +163,7 @@ mod tests { use super::*; - fn linked_idml(link: &str) -> Result<(Vec, Vec)> { + fn linked_idml(link: &str, embed_images: bool) -> Result<(Vec, Vec)> { let png = designcraft_render::Rendered { width: 2, height: 2, pixels: vec![255; 16] }.to_png(); let mut session = Session::new(); session.execute("file.new", &json!({}))?; @@ -142,7 +173,7 @@ mod tests { Arc::make_mut(asset).link = Some(link.into()); Ok(Value::Null) })?; - let bytes = designcraft_idml::export_idml_with(&session.doc()?.doc, &designcraft_idml::ExportOptions { embed_images: false }); + let bytes = designcraft_idml::export_idml_with(&session.doc()?.doc, &designcraft_idml::ExportOptions { embed_images }); Ok((bytes, png)) } @@ -151,7 +182,7 @@ mod tests { let dir = std::env::temp_dir().join(format!("dc-idml-nested-links-{}", std::process::id())); let path = dir.join("Links").join("illustrations").join("logo.png"); std::fs::create_dir_all(path.parent().unwrap()).unwrap(); - let (idml, png) = linked_idml("C:\\original-computer\\project\\Links\\illustrations\\logo.png").unwrap(); + let (idml, png) = linked_idml("C:\\original-computer\\project\\Links\\illustrations\\logo.png", false).unwrap(); std::fs::write(&path, &png).unwrap(); std::fs::write(dir.join("logo.png"), b"a different file with the same basename").unwrap(); @@ -169,7 +200,7 @@ mod tests { let dir = std::env::temp_dir().join(format!("dc-idml-flat-links-{}", std::process::id())); let path = dir.join("Links").join("logo.png"); std::fs::create_dir_all(path.parent().unwrap()).unwrap(); - let (idml, png) = linked_idml("/original-computer/project/logo.png").unwrap(); + let (idml, png) = linked_idml("/original-computer/project/logo.png", false).unwrap(); std::fs::write(&path, &png).unwrap(); let doc = import(&idml, Some(&dir)).unwrap(); @@ -179,4 +210,49 @@ mod tests { assert_eq!(super::super::links::status(asset), "ok"); std::fs::remove_dir_all(dir).unwrap(); } + + #[test] + fn embedded_link_resolution_preserves_originals_and_image_bytes() { + let nonce = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos(); + let dir = std::env::temp_dir().join(format!("dc-embedded-links-{}-{nonce}", std::process::id())); + let original = dir.join("original").join("logo.png"); + let package = dir.join("package"); + let copy = package.join("Links").join("logo.png"); + std::fs::create_dir_all(original.parent().unwrap()).unwrap(); + std::fs::create_dir_all(copy.parent().unwrap()).unwrap(); + let (idml, png) = linked_idml(original.to_str().unwrap(), true).unwrap(); + std::fs::write(&original, b"modified original").unwrap(); + std::fs::write(©, &png).unwrap(); + let doc = import(&idml, Some(&package)).unwrap(); + let asset = doc.assets.values().next().unwrap(); + assert_eq!(std::path::Path::new(asset.link.as_deref().unwrap()), original.as_path(), "an existing original stays authoritative"); + assert_eq!(*asset.data, png); + assert_eq!(super::super::links::status(asset), "modified"); + + std::fs::remove_file(&original).unwrap(); + std::fs::write(©, b"modified packaged copy").unwrap(); + let doc = import(&idml, Some(&package)).unwrap(); + let asset = doc.assets.values().next().unwrap(); + assert_eq!(asset.link.as_deref(), copy.to_str()); + assert_eq!(*asset.data, png, "rebinding must not replace embedded pixels"); + assert_eq!(super::super::links::status(asset), "modified"); + + std::fs::remove_file(©).unwrap(); + for folder in [Some(package.as_path()), None] { + let doc = import(&idml, folder).unwrap(); + let asset = doc.assets.values().next().unwrap(); + assert_eq!(std::path::Path::new(asset.link.as_deref().unwrap()), original.as_path(), "keep a missing link when there is no candidate"); + assert_eq!(*asset.data, png); + assert_eq!(super::super::links::status(asset), "missing"); + } + let oversized = "a/".repeat(MAX_LINK_PATH_BYTES); + assert!(packaged_link_candidates(&oversized, Some(&package)).is_empty()); + let mut doc = import(&idml, None).unwrap(); + Arc::make_mut(doc.assets.values_mut().next().unwrap()).link = Some(oversized.clone()); + resolve_packaged_links(&mut doc, Some(&package)); + let asset = doc.assets.values().next().unwrap(); + assert_eq!(asset.link.as_deref(), Some(oversized.as_str())); + assert_eq!(*asset.data, png); + std::fs::remove_dir_all(dir).unwrap(); + } } diff --git a/crates/engine/src/cmd/package.rs b/crates/engine/src/cmd/package.rs index 5cc4cc860..50556d97f 100644 --- a/crates/engine/src/cmd/package.rs +++ b/crates/engine/src/cmd/package.rs @@ -221,6 +221,31 @@ fn copy_links(s: &mut Session, p: &Value) -> Result { mod tests { use super::*; + #[test] + fn relocated_packages_rebind_embedded_image_links_on_open() { + let nonce = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos(); + let dir = std::env::temp_dir().join(format!("dc-package-relocation-{}-{nonce}", std::process::id())); + let original = dir.join("original"); + let moved = dir.join("moved"); + let png = designcraft_render::Rendered { width: 4, height: 3, pixels: [30, 90, 180, 255].repeat(12) }.to_png(); + let mut s = Session::new(); + s.execute("file.new", &json!({"title": "OwnedPackage", "width": 72, "height": 72})).unwrap(); + s.execute("file.place", &json!({"base64": super::super::base64_encode(&png), "name": "owned.png", "x": 5, "y": 5, "width": 16})).unwrap(); + s.execute("file.package", &json!({"dir": original.to_string_lossy(), "idml": true, "pdf": false})).unwrap(); + std::fs::rename(&original, &moved).unwrap(); + let expected = std::path::absolute(moved.join("Links").join("owned.png")).unwrap(); + for (command, extension) in [("file.open", "designcraft"), ("file.openIdml", "idml")] { + let mut opened = Session::new(); + opened.execute(command, &json!({"path": moved.join(format!("OwnedPackage.{extension}")).to_string_lossy()})).unwrap(); + let asset = opened.doc().unwrap().doc.assets.values().next().unwrap(); + assert_eq!(*asset.data, png, "{extension}: preserve embedded pixels"); + assert_eq!(asset.link.as_deref(), expected.to_str(), "{extension}: use the relocated Links file"); + let links = opened.execute("links.list", &json!({})).unwrap(); + assert_eq!(links[0]["status"], "ok", "{extension}"); + } + std::fs::remove_dir_all(dir).unwrap(); + } + #[test] fn package_writes_document_links_and_report() { let dir = std::env::temp_dir().join(format!("dc-package-{}", std::process::id()));