fix(idml): report missing parts and show import warnings

Match IDML part names leniently, report missing parts, and show import warnings when opening a file. Cover the malformed URI crash with a regression test.

Validation: all six cargo xtask ci gates passed on the current main merge candidate; 1,176 native tests passed and all 15 WASM packages checked.
Hosted FreeBSD tests and Windows ARM64 packaging and install/run checks passed.

Task: #356

Co-authored-by: skylartaylor <26588250+skylartaylor@users.noreply.github.com>
This commit is contained in:
aiongg
2026-10-11 19:39:34 -06:00
committed by GitHub
co-authored by skylartaylor
parent 95c8c9cc5b
commit f70f93de44
8 changed files with 253 additions and 32 deletions
Generated
+1
View File
@@ -1160,6 +1160,7 @@ dependencies = [
"unicode-bidi",
"unicode-bidi-mirroring",
"unicode-segmentation",
"zip",
]
[[package]]
+1 -1
View File
@@ -18,7 +18,7 @@ pub fn specs() -> Vec<CommandSpec> {
cmd!(noundo "file.newSample", "Sample Document", ["Help"], None, "{} — a multi-page magazine sample", always, file_sample),
cmd!(query "file.presets", "Document Presets", [], None, "{}", always, |_, _| Ok(serde_json::to_value(PRESETS).unwrap_or_default())),
cmd!(noundo "file.open", "Open…", ["File"], Some("Cmd+O"),
"{path} — .designcraft or .idml; the fonts in a `Document Fonts` folder beside it load first → {index, documentFonts: faces loaded, warnings: font files skipped}",
"{path} — .designcraft or .idml; the fonts in a `Document Fonts` folder beside it load first → {index, documentFonts: faces loaded, warnings: font files skipped (and, for .idml, package parts missing)}",
always, file_open),
cmd!(noundo "file.openBytes", "Open Bytes", [], None, "{name, base64} — DesignCraft JSON or an IDML package", always, file_open_bytes),
cmd!(noundo "file.save", "Save", ["File"], Some("Cmd+S"), "{path?}", has_doc, file_save),
+12 -5
View File
@@ -13,7 +13,7 @@ pub fn specs() -> Vec<CommandSpec> {
"{path?, embedImages?: true} — writes an IDML package to `path`, or returns {base64} without a path",
has_doc, export_idml),
cmd!(noundo "file.openIdml", "Open IDML", [], None,
"{path | base64, name?} — opens an IDML package as a new document (linked images are read next to the file or from its Links/ folder; the fonts in a `Document Fonts` folder beside it load first) → {index, documentFonts, warnings}",
"{path | base64, name?} — opens an IDML package as a new document (linked images are read next to the file or from its Links/ folder; the fonts in a `Document Fonts` folder beside it load first) → {index, documentFonts, warnings: package parts missing, font files skipped}",
always, open_idml),
]
}
@@ -34,6 +34,11 @@ fn export_idml(s: &mut Session, p: &Value) -> Result<Value> {
/// Import IDML bytes; `dir` is the folder the package came from (for relative link lookup).
pub fn import(bytes: &[u8], dir: Option<&std::path::Path>) -> Result<Document> {
import_report(bytes, dir).map(|(document, _)| document)
}
/// [`import`], with the problems that did not stop it (parts missing from the package, …).
pub fn import_report(bytes: &[u8], dir: Option<&std::path::Path>) -> Result<(Document, Vec<String>)> {
#[cfg(not(target_arch = "wasm32"))]
let resolved = std::cell::RefCell::new(std::collections::HashMap::new());
let read = |link: &str| -> Option<Vec<u8>> {
@@ -49,7 +54,8 @@ pub fn import(bytes: &[u8], dir: Option<&std::path::Path>) -> Result<Document> {
None
}
};
let document = designcraft_idml::import_idml_with(bytes, &read).map_err(|e| EngineError::Other(e.to_string()))?;
let designcraft_idml::Imported { document, warnings } =
designcraft_idml::import_idml_report(bytes, &read).map_err(|e| EngineError::Other(e.to_string()))?;
#[cfg(not(target_arch = "wasm32"))]
let document = {
let mut document = document;
@@ -62,7 +68,7 @@ pub fn import(bytes: &[u8], dir: Option<&std::path::Path>) -> Result<Document> {
resolve_packaged_links(&mut document, dir);
document
};
Ok(document)
Ok((document, warnings))
}
#[cfg(not(target_arch = "wasm32"))]
@@ -142,14 +148,15 @@ pub(crate) fn open_idml(s: &mut Session, p: &Value) -> Result<Value> {
} else {
return Err(bad("file.openIdml", "missing `path` or `base64`"));
};
let mut d = import(&bytes, dir.as_deref())?;
let (mut d, mut warnings) = import_report(&bytes, dir.as_deref())?;
if let Some(n) = name {
d.title = n;
}
let (fonts, faces, warnings) = match str_param(p, "path").filter(|_| str_param(p, "base64").is_none()) {
let (fonts, faces, font_warnings) = match str_param(p, "path").filter(|_| str_param(p, "base64").is_none()) {
Some(path) => super::file::load_document_fonts(&mut d, path),
None => (None, 0, Vec::new()),
};
warnings.extend(font_warnings);
// Never save over the .idml with the native format: the document starts unsaved.
let mut st = DocState::new(d, None);
st.fonts = fonts;
+75 -25
View File
@@ -48,8 +48,24 @@ fn default_reader(path: &str) -> Option<Vec<u8>> {
/// Import with a custom reader for linked files (`path` → bytes).
pub fn import_idml_with(bytes: &[u8], read_link: &dyn Fn(&str) -> Option<Vec<u8>>) -> Result<Document> {
import_idml_report(bytes, read_link).map(|r| r.document)
}
/// An imported IDML document and what the import could not bring in.
#[derive(Debug)]
pub struct Imported {
pub document: Document,
/// One line per problem that did not stop the import (a part listed in `designmap.xml` but
/// absent from the package, …).
pub warnings: Vec<String>,
}
/// Import with a custom reader for linked files, reporting the problems that did not stop it.
pub fn import_idml_report(bytes: &[u8], read_link: &dyn Fn(&str) -> Option<Vec<u8>>) -> Result<Imported> {
let mut zip = zip::ZipArchive::new(std::io::Cursor::new(bytes)).map_err(|e| IdmlError::NotIdml(e.to_string()))?;
let mut files: HashMap<String, Vec<u8>> = HashMap::new();
// Normalised part name → entry name, first entry wins (zip order, so the choice is stable).
let mut lenient: HashMap<String, String> = HashMap::new();
for i in 0..zip.len() {
let mut f = zip.by_index(i).map_err(|e| IdmlError::NotIdml(e.to_string()))?;
if f.is_dir() {
@@ -59,35 +75,81 @@ pub fn import_idml_with(bytes: &[u8], read_link: &dyn Fn(&str) -> Option<Vec<u8>
// The size is what the archive claims: don't reserve more than a sane part up front.
let mut buf = Vec::with_capacity((f.size() as usize).min(1 << 24));
f.read_to_end(&mut buf).map_err(|e| IdmlError::Part { part: name.clone(), msg: e.to_string() })?;
lenient.entry(normalize_part_name(&name)).or_insert_with(|| name.clone());
files.insert(name, buf);
}
if let Some(m) = files.get("mimetype")
let part = |name: &str| files.get(name).or_else(|| lenient.get(&normalize_part_name(name)).and_then(|n| files.get(n)));
if let Some(m) = part("mimetype")
&& String::from_utf8_lossy(m).trim() != MIMETYPE
{
return Err(IdmlError::NotIdml(format!("unexpected mimetype `{}`", String::from_utf8_lossy(m).trim())));
}
let dm = files.get("designmap.xml").ok_or_else(|| IdmlError::NotIdml("missing designmap.xml".into()))?;
let dm = part("designmap.xml").ok_or_else(|| IdmlError::NotIdml("missing designmap.xml".into()))?;
let root = parse(dm).map_err(|msg| IdmlError::Part { part: "designmap.xml".into(), msg })?;
if root.local() != "Document" {
return Err(IdmlError::NotIdml(format!("designmap root is <{}>", root.name)));
}
// Flatten includes: every `idPkg:*` child is replaced by the children of its part's root.
let mut top: Vec<El> = Vec::new();
let mut warnings = Vec::new();
for c in root.elements() {
if c.name.starts_with("idPkg:") {
let Some(src) = c.get("src") else { continue };
let Some(data) = files.get(src) else { continue };
let part = parse(data).map_err(|msg| IdmlError::Part { part: src.into(), msg })?;
top.extend(part.elements().cloned());
let Some(data) = part(src) else {
warnings.push(format!("{src}: listed in designmap.xml but missing from the package; its contents were not imported"));
continue;
};
let el = parse(data).map_err(|msg| IdmlError::Part { part: src.into(), msg })?;
top.extend(el.elements().cloned());
} else {
top.push(c.clone());
}
}
let mut im = Importer::new(read_link);
im.run(&root, &top)?;
let d = im.finish(&root)?;
d.check().map_err(|e| IdmlError::Invalid(e.to_string()))?;
Ok(d)
let document = im.finish(&root).map_err(|e| match e {
// Without its spread parts there is nothing to open: say which parts were missing.
IdmlError::Invalid(msg) if !warnings.is_empty() => IdmlError::Invalid(format!("{msg} ({})", warnings.join("; "))),
e => e,
})?;
document.check().map_err(|e| IdmlError::Invalid(e.to_string()))?;
Ok(Imported { document, warnings })
}
/// A package part name in the form the lookup compares: InDesign and other IDML writers differ
/// in percent-encoding, case, path separators and a leading `./` or `/`.
fn normalize_part_name(name: &str) -> String {
let decoded = percent_decode(&name.replace('\\', "/"));
let mut rest = decoded.as_str();
loop {
if let Some(r) = rest.strip_prefix("./") {
rest = r;
} else if let Some(r) = rest.strip_prefix('/') {
rest = r;
} else {
break;
}
}
rest.to_lowercase()
}
/// Decode `%XX` escapes; anything else (including a malformed escape) is kept as written.
pub(crate) fn percent_decode(s: &str) -> String {
let b = s.as_bytes();
let mut out = Vec::with_capacity(b.len());
let mut i = 0;
while let Some(&c) = b.get(i) {
if c == b'%'
&& let (Some(h), Some(l)) = (b.get(i + 1).and_then(|h| (*h as char).to_digit(16)), b.get(i + 2).and_then(|l| (*l as char).to_digit(16)))
{
out.push((h * 16 + l) as u8);
i += 3;
continue;
}
out.push(c);
i += 1;
}
String::from_utf8_lossy(&out).into_owned()
}
struct ItemCtx {
@@ -2560,21 +2622,9 @@ fn path_of(pg: &El) -> PathData {
pub(crate) fn uri_to_path(uri: &str) -> String {
let rest = uri.strip_prefix("file://").or_else(|| uri.strip_prefix("file:")).unwrap_or(uri);
// `file:///C:/…` → `/C:/…` → `C:/…`
let rest = if rest.len() > 3 && rest.starts_with('/') && rest.as_bytes()[2] == b':' { &rest[1..] } else { rest };
let mut out = Vec::with_capacity(rest.len());
let b = rest.as_bytes();
let mut i = 0;
while i < b.len() {
if b[i] == b'%'
&& i + 2 < b.len()
&& let Ok(v) = u8::from_str_radix(&rest[i + 1..i + 3], 16)
{
out.push(v);
i += 3;
continue;
}
out.push(b[i]);
i += 1;
}
String::from_utf8_lossy(&out).to_string()
let rest = match rest.as_bytes() {
[b'/', _, b':', _, ..] => rest.get(1..).unwrap_or(rest),
_ => rest,
};
percent_decode(rest)
}
+1 -1
View File
@@ -49,7 +49,7 @@ mod xml;
mod tests;
pub use export::{ExportOptions, export_idml, export_idml_with};
pub use import::{import_idml, import_idml_with};
pub use import::{Imported, import_idml, import_idml_report, import_idml_with};
/// The IDML package mimetype (content of the first, stored `mimetype` entry).
pub const MIMETYPE: &str = "application/vnd.adobe.indesign-idml-package";
+54
View File
@@ -428,6 +428,60 @@ fn imports_scalar_insets_in_object_style_without_replacing_frame_override() {
assert_eq!(frame_inset(&d), [3.0; 4]);
}
/// A one-page spread part `Self` with one rectangle.
fn one_frame_spread(id: &str) -> String {
format!(
r#"<idPkg:Spread xmlns:idPkg="http://ns.adobe.com/AdobeInDesign/idml/1.0/packaging">
<Spread Self="{id}">
<Page Self="{id}p" GeometricBounds="0 0 100 100" ItemTransform="1 0 0 1 0 0"/>
<Rectangle Self="{id}r" ItemTransform="1 0 0 1 0 0">
<Properties><PathGeometry><GeometryPathType PathOpen="false"><PathPointArray>
<PathPointType Anchor="10 10"/><PathPointType Anchor="10 50"/>
<PathPointType Anchor="50 50"/><PathPointType Anchor="50 10"/>
</PathPointArray></GeometryPathType></PathGeometry></Properties>
</Rectangle>
</Spread>
</idPkg:Spread>"#
)
}
/// Writers differ in how a designmap `src` spells its part: percent-encoded, another case,
/// backslashes, a leading `./`. Those resolve; a part absent from the package is a warning that
/// names it, not silently dropped objects.
#[test]
fn designmap_parts_match_leniently_and_missing_ones_are_reported() {
let designmap = r#"<Document xmlns:idPkg="http://ns.adobe.com/AdobeInDesign/idml/1.0/packaging" Self="d">
<idPkg:Spread src="Spreads/Spread_u1a%20b.xml"/>
<idPkg:Spread src="./spreads\SPREAD_u2.xml"/>
<idPkg:Spread src="Spreads/Spread_gone.xml"/>
</Document>"#;
let zip = zip_files(&[
("designmap.xml", designmap),
("Spreads/Spread_u1a b.xml", &one_frame_spread("u1")),
("Spreads/Spread_u2.xml", &one_frame_spread("u2")),
]);
let r = import_idml_report(&zip, &|_| None).unwrap();
assert_eq!(r.document.spreads.len(), 2, "both lenient spellings resolve");
assert!(r.document.spreads.iter().all(|sp| sp.items.len() == 1), "the spreads keep their frames");
assert_eq!(r.warnings.len(), 1, "{:?}", r.warnings);
assert!(r.warnings[0].contains("Spreads/Spread_gone.xml"), "{:?}", r.warnings);
// With every spread part missing the import fails, and the error names the part.
let only_missing = r#"<Document xmlns:idPkg="http://ns.adobe.com/AdobeInDesign/idml/1.0/packaging" Self="d">
<idPkg:Spread src="Spreads/Spread_gone.xml"/>
</Document>"#;
let err = import_idml(&zip_files(&[("designmap.xml", only_missing)])).unwrap_err();
assert!(err.to_string().contains("Spreads/Spread_gone.xml"), "{err}");
}
#[test]
fn percent_decode_keeps_malformed_escapes() {
assert_eq!(import::percent_decode("a%20b%2"), "a b%2");
assert_eq!(import::percent_decode("%é%zz"), "%é%zz");
assert_eq!(import::uri_to_path("file:///C:/a%20b"), "C:/a b");
assert_eq!(import::uri_to_path("file:/a%bé"), "/a%bé");
}
#[test]
fn rejects_non_idml() {
assert!(import_idml(b"not a zip").is_err());
+1
View File
@@ -30,6 +30,7 @@ log = { workspace = true }
[dev-dependencies]
designcraft-idml = { workspace = true }
zip = { workspace = true }
# Headless (offscreen wgpu) rendering of the whole window for UI screenshots: `examples/ui_shot.rs`.
egui_kittest = { workspace = true, features = ["wgpu", "snapshot"] }
designcraft-fonts = { workspace = true, features = ["testing"] }
+108
View File
@@ -812,6 +812,7 @@ impl DesignApp {
/// Open (`file.open`, `file.openBytes`) or place (`file.place`) a file the user chose. A file
/// that can't be opened or placed gets an alert with the reason, as well as the status line.
/// A successful import with warnings keeps the document and shows the warnings in an alert.
pub fn open_file(&mut self, cmd: &str, params: Value) -> Result<Value, String> {
let file = match params.get("path").and_then(Value::as_str) {
Some(path) => std::path::Path::new(path).file_name().map_or_else(|| path.to_string(), |n| n.to_string_lossy().to_string()),
@@ -821,6 +822,20 @@ impl DesignApp {
if let Err(e) = &r {
let title = if cmd == "file.place" { "Can't Place the File" } else { "Can't Open the File" };
self.ui.dialog = Some(dialogs::Dialog::new("alert", json!({"title": title, "file": file, "message": e})));
} else if let Ok(result) = &r {
let warnings: Vec<&str> = result
.get("warnings")
.and_then(Value::as_array)
.into_iter()
.flatten()
.filter_map(Value::as_str)
.filter(|warning| !warning.is_empty())
.collect();
if !warnings.is_empty() {
self.status(format!("{file}: {}", warnings.join("; ")));
self.ui.dialog =
Some(dialogs::Dialog::new("alert", json!({"title": "Import Warnings", "file": file, "message": warnings.join("\n")})));
}
}
r
}
@@ -1319,6 +1334,99 @@ mod tests {
}
}
#[cfg(test)]
mod import_warning_tests {
use super::*;
use std::io::Write;
fn idml(missing_parts: bool) -> Vec<u8> {
let missing = if missing_parts { r#"<idPkg:Spread src="Spreads/missing.xml"/><idPkg:Story src="Stories/missing.xml"/>"# } else { "" };
let map = format!(
r#"<Document xmlns:idPkg="http://ns.adobe.com/AdobeInDesign/idml/1.0/packaging" Self="d"><idPkg:Spread src="Spreads/one.xml"/>{missing}</Document>"#
);
let spread = r#"<idPkg:Spread xmlns:idPkg="http://ns.adobe.com/AdobeInDesign/idml/1.0/packaging"><Spread Self="s"><Page Self="p" GeometricBounds="0 0 100 100" ItemTransform="1 0 0 1 0 0"/><Rectangle Self="r" ItemTransform="1 0 0 1 0 0"/></Spread></idPkg:Spread>"#;
let mut zip = zip::ZipWriter::new(std::io::Cursor::new(Vec::new()));
let options = zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Stored);
for (name, text) in [("mimetype", designcraft_idml::MIMETYPE), ("designmap.xml", &map), ("Spreads/one.xml", spread)] {
zip.start_file(name, options).unwrap();
zip.write_all(text.as_bytes()).unwrap();
}
zip.finish().unwrap().into_inner()
}
fn app() -> DesignApp {
let mut app = DesignApp::new(Session::new(), Services { inbox: Some(Inbox::default()), ..Default::default() });
app.run("file.new", json!({"title": "Existing work"})).unwrap();
app.run("frame.create", json!({"rect": [10, 10, 50, 50]})).unwrap();
app
}
fn assert_partial_import_warning(app: &mut DesignApp, file: &str) {
let dialog = app.ui.dialog.as_ref().expect("partial import must show an alert");
assert_eq!(dialog.id, "alert");
assert_eq!(dialog.fields["title"], "Import Warnings");
assert_eq!(dialog.fields["file"], file);
for part in ["Spreads/missing.xml", "Stories/missing.xml"] {
assert!(dialog.fields["message"].as_str().unwrap().contains(part), "every missing part is named");
assert!(app.ui.status.contains(part), "the status line retains the warning");
}
assert_eq!(app.session.documents().len(), 2);
assert_eq!(app.session.documents()[0].doc.title, "Existing work");
assert!(app.session.documents()[0].is_dirty(), "existing unsaved work stays intact");
assert_eq!(app.session.documents()[0].doc.spreads[0].items.len(), 1);
assert_eq!(app.session.doc().unwrap().doc.spreads.len(), 1);
assert_eq!(app.session.doc().unwrap().doc.spreads[0].items.len(), 1, "available document content is kept");
dialogs::confirm(app).unwrap();
assert!(app.ui.dialog.is_none(), "OK dismisses the warning");
assert_eq!(app.session.documents().len(), 2, "dismissing the warning keeps the imported document");
}
#[test]
fn byte_open_reports_partial_import_warnings_without_losing_documents() {
let mut app = app();
let result =
app.open_file("file.openBytes", json!({"name": "partial.idml", "base64": designcraft_engine::cmd::base64_encode(&idml(true))})).unwrap();
assert_eq!(result["warnings"].as_array().unwrap().len(), 2);
assert_partial_import_warning(&mut app, "partial.idml");
}
#[test]
fn native_open_reports_partial_import_warnings_without_losing_documents() {
let path = std::env::temp_dir().join(format!("designcraft-partial-import-{}", std::process::id())).join("partial.idml");
std::fs::create_dir_all(path.parent().unwrap()).unwrap();
std::fs::write(&path, idml(true)).unwrap();
let picked = path.to_string_lossy().to_string();
let mut app = app();
app.services.pick_open = Some(Box::new(move |_| Some(picked.clone())));
let result = app.run("app.openDialog", json!({}));
std::fs::remove_file(&path).unwrap();
std::fs::remove_dir(path.parent().unwrap()).unwrap();
assert_eq!(result.unwrap()["warnings"].as_array().unwrap().len(), 2);
assert_partial_import_warning(&mut app, "partial.idml");
}
#[test]
fn inbox_open_reports_partial_import_warnings_without_losing_documents() {
let mut app = app();
let request = app.import_request("open");
app.services.inbox.as_ref().unwrap().lock().unwrap().push(ImportedFile { request, name: "partial.idml".into(), bytes: idml(true) });
app.drain_inbox();
assert_partial_import_warning(&mut app, "partial");
}
#[test]
fn complete_import_does_not_show_a_warning() {
let mut app = app();
let result = app
.open_file("file.openBytes", json!({"name": "complete.idml", "base64": designcraft_engine::cmd::base64_encode(&idml(false))}))
.unwrap();
assert_eq!(result["warnings"], json!([]));
assert!(app.ui.dialog.is_none());
assert!(app.ui.status.is_empty());
assert_eq!(app.session.documents().len(), 2);
}
}
/// The whole window in a headless test harness, at a chosen window size.
#[cfg(test)]
pub(crate) mod test_window {