From ec4fbb270fe7555a55c06141a8139fb89fae81bf Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 17 Oct 2026 06:20:21 +0200 Subject: [PATCH] Ignore NaN and infinite numbers when importing IDML Found while testing crates/idml/src/import.rs:2748 (stroke_from: StrokeWeight, MiterLimit), also El::num in crates/idml/src/xml.rs:96 and the plain `.parse().ok()` calls around import.rs:1833, 2823 and 2832 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/tests_idml.rs | 97 +++++++++++++++++++++++++++++++++ crates/idml/src/import.rs | 35 ++++++++---- crates/idml/src/names.rs | 2 +- crates/idml/src/xml.rs | 11 +++- 4 files changed, 133 insertions(+), 12 deletions(-) diff --git a/crates/engine/src/tests_idml.rs b/crates/engine/src/tests_idml.rs index 0701052d..3a49168b 100644 --- a/crates/engine/src/tests_idml.rs +++ b/crates/engine/src/tests_idml.rs @@ -575,3 +575,100 @@ fn table_of_contents_character_styles_and_sort_round_trip_through_idml() { let e = &after.entries[1]; assert_eq!((e.page_number_style.as_deref(), e.separator_style.as_deref(), e.sort), (None, None, false)); } + +/// `text` with the value after every `open` (up to the next `close`) set to `value`, and how +/// many values were replaced. +fn replace_values(text: &str, open: &str, close: char, value: &str) -> (String, usize) { + let mut rest = text; + let mut new = String::new(); + let mut count = 0; + while let Some(at) = rest.find(open) { + let start = at + open.len(); + new.push_str(&rest[..start]); + new.push_str(value); + let end = rest[start..].find(close).map_or(rest.len(), |e| start + e); + rest = &rest[end..]; + count += 1; + } + new.push_str(rest); + (new, count) +} + +/// `package` with every `{name}="…"` attribute and `<{name}>…` / `<{name} type="…">…` +/// property element in its XML parts set to `value`, and how many values were replaced. +fn with_attr_set(package: &[u8], name: &str, value: &str) -> (Vec, usize) { + let mut zip = zip::ZipArchive::new(std::io::Cursor::new(package)).unwrap(); + let mut out = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); + let mut count = 0; + for i in 0..zip.len() { + let mut f = zip.by_index(i).unwrap(); + let mut body = Vec::new(); + std::io::Read::read_to_end(&mut f, &mut body).unwrap(); + if f.name().ends_with(".xml") + && let Ok(text) = String::from_utf8(body.clone()) + { + let (text, a) = replace_values(&text, &format!(" {name}=\""), '"', value); + let (text, b) = replace_values(&text, &format!("<{name}>"), '<', value); + let (text, c) = replace_values(&text, &format!("<{name} type=\"unit\">"), '<', value); + let (text, d) = replace_values(&text, &format!("<{name} type=\"double\">"), '<', value); + count += a + b + c + d; + body = text.into_bytes(); + } + out.start_file(f.name(), zip::write::SimpleFileOptions::default()).unwrap(); + std::io::Write::write_all(&mut out, &body).unwrap(); + } + (out.finish().unwrap().into_inner(), count) +} + +/// Non-finite numbers in an IDML package never reach the model: the imported document saves +/// and reopens. Every listed name occurs in the exported sample, so each case really rewrites +/// values; names the sample lacks (feather `Width`, gradient start/length) go through `El::num`, +/// covered in `xml.rs`. +#[test] +fn idml_non_finite_numbers_import_to_a_document_that_reopens() { + let package = designcraft_idml::export_idml(&crate::sample::magazine()); + let attrs = [ + "StrokeWeight", + "MiterLimit", + "Angle", + "FirstBaselineOffset", + "MinimumFirstBaselineOffset", + "SpaceBefore", + "SpaceAfter", + "LeftIndent", + "RightIndent", + "FirstLineIndent", + "PointSize", + "Tracking", + "Leading", + "FillTint", + "StrokeTint", + "GapTint", + "Position", + "BaselineShift", + "Skew", + ]; + let mut failures = Vec::new(); + for attr in attrs { + for value in ["NaN", "inf", "-inf"] { + let (bytes, replaced) = with_attr_set(&package, attr, value); + if replaced == 0 { + failures.push(format!("{attr}: not present in the exported package")); + continue; + } + let doc = match designcraft_idml::import_idml(&bytes) { + Ok(doc) => doc, + Err(e) => { + failures.push(format!("{attr}={value}: import failed: {e}")); + continue; + } + }; + match designcraft_format::save(&doc).map(|saved| designcraft_format::load(&saved)) { + Ok(Ok(_)) => {} + Ok(Err(e)) => failures.push(format!("{attr}={value}: saved but load failed: {e}")), + Err(e) => failures.push(format!("{attr}={value}: save failed: {e}")), + } + } + } + assert!(failures.is_empty(), "{failures:#?}"); +} diff --git a/crates/idml/src/import.rs b/crates/idml/src/import.rs index 5cd3746a..ae4d9f89 100644 --- a/crates/idml/src/import.rs +++ b/crates/idml/src/import.rs @@ -391,6 +391,12 @@ fn nums(s: &str) -> Vec { s.split_whitespace().filter_map(|v| v.parse().ok()).collect() } +/// Like [`nums`], but empty when any number is NaN or infinite. +fn finite_nums(s: &str) -> Vec { + let v = nums(s); + if v.iter().all(|x| x.is_finite()) { v } else { Vec::new() } +} + /// The viewBox size of an SVG whose root has a viewBox but no width/height (percentages count as /// none): InDesign places such a file at one viewBox unit to the point. fn svg_viewbox_size(data: &[u8]) -> Option<(f64, f64)> { @@ -936,12 +942,16 @@ impl<'r> Importer<'r> { // Custom stroke styles. for e in top.iter().filter(|e| matches!(e.local(), "StripedStrokeStyle" | "DashedStrokeStyle" | "DottedStrokeStyle")) { let (Some(id), Some(name)) = (e.get("Self"), e.get("Name")) else { continue }; - let nums = |k: &str| e.get(k).unwrap_or("").split_whitespace().filter_map(|x| x.parse::().ok()).collect::>(); let kind = match e.local() { "StripedStrokeStyle" => designcraft_doc::StrokeType::Stripes { - bands: nums("StripeArray").as_chunks::<2>().0.iter().map(|c| (c[0] / 100.0, (c[1] - c[0]) / 100.0)).collect(), + bands: finite_nums(e.get("StripeArray").unwrap_or("")) + .as_chunks::<2>() + .0 + .iter() + .map(|c| (c[0] / 100.0, (c[1] - c[0]) / 100.0)) + .collect(), }, - "DashedStrokeStyle" => designcraft_doc::StrokeType::Dashed { pattern: nums("DashArray") }, + "DashedStrokeStyle" => designcraft_doc::StrokeType::Dashed { pattern: finite_nums(e.get("DashArray").unwrap_or("")) }, _ => designcraft_doc::StrokeType::Dotted, }; let _ = id; @@ -1830,7 +1840,7 @@ impl<'r> Importer<'r> { for li in tl.find_all("ListItem") { let get = |k: &str| li.find(k).map(|x| x.text_content()); tabs.push(TabStop { - position: get("Position").and_then(|v| v.trim().parse().ok()).unwrap_or(0.0), + position: get("Position").and_then(|v| v.trim().parse::().ok()).filter(|v| v.is_finite()).unwrap_or(0.0), align: names::tab_align_in(get("Alignment").unwrap_or_default().trim()), leader: get("Leader").unwrap_or_default(), align_on: get("AlignmentCharacter").filter(|c| c != ".").unwrap_or_default(), @@ -2189,8 +2199,8 @@ impl<'r> Importer<'r> { if r >= nr || c >= nc { continue; } - let rs = (ce.num("RowSpan").unwrap_or(1.0).max(1.0) as usize).min(nr - r); - let cs = (ce.num("ColumnSpan").unwrap_or(1.0).max(1.0) as usize).min(nc - c); + let rs = (span(ce, "RowSpan").max(1.0) as usize).min(nr - r); + let cs = (span(ce, "ColumnSpan").max(1.0) as usize).min(nc - c); let mut cell = t.cell(r, c).cloned().unwrap_or_default(); let cell_style = ce.get("AppliedCellStyle").map(|r| self.cell_style_ref(r)).unwrap_or_else(|| designcraft_doc::NO_CELL_STYLE.into()); if cell_style != designcraft_doc::NO_CELL_STYLE { @@ -2745,7 +2755,7 @@ impl<'r> Importer<'r> { if let Some(c) = color { s.swatch = self.swatch_ref(&c); } - if let Some(w) = g(self, "StrokeWeight").and_then(|v| v.parse().ok()) { + if let Some(w) = g(self, "StrokeWeight").and_then(|v| v.parse::().ok()).filter(|v| v.is_finite()) { s.weight = w; } if let Some(t) = g(self, "StrokeTint").and_then(|v| v.parse().ok()) { @@ -2763,7 +2773,7 @@ impl<'r> Importer<'r> { if let Some(v) = g(self, "EndJoin") { s.join = names::join_in(&v); } - if let Some(v) = g(self, "MiterLimit").and_then(|v| v.parse().ok()) { + if let Some(v) = g(self, "MiterLimit").and_then(|v| v.parse::().ok()).filter(|v| v.is_finite()) { s.miter_limit = v; } if let Some(v) = g(self, "LeftLineEnd") { @@ -2829,7 +2839,7 @@ impl<'r> Importer<'r> { { it.fill.gradient_angle = Some(a); } - let start: Option> = e.prop("GradientFillStart").map(|s| s.split_whitespace().filter_map(|v| v.parse().ok()).collect()); + let start: Option> = e.prop("GradientFillStart").map(|s| finite_nums(&s)); if let (Some([x, y]), Some(len)) = (start.as_deref().and_then(|v| <[f64; 2]>::try_from(v).ok()), e.num("GradientFillLength")) && len > 0.0 { @@ -2847,7 +2857,7 @@ impl<'r> Importer<'r> { { it.stroke.gradient_angle = Some(a); } - let start: Option> = e.prop("GradientStrokeStart").map(|s| s.split_whitespace().filter_map(|v| v.parse().ok()).collect()); + let start: Option> = e.prop("GradientStrokeStart").map(|s| finite_nums(&s)); if let (Some([x, y]), Some(len)) = (start.as_deref().and_then(|v| <[f64; 2]>::try_from(v).ok()), e.num("GradientStrokeLength")) && len > 0.0 { @@ -3301,6 +3311,11 @@ fn overlay(base: &mut El, top: &El, depth: usize) { } } +/// A cell span count; an infinite span is kept so it clamps to the table edge. +fn span(e: &El, k: &str) -> f64 { + e.prop(k).and_then(|v| v.trim().parse::().ok()).unwrap_or(1.0) +} + fn finite(v: Option) -> Option { v.filter(|v| v.is_finite()) } diff --git a/crates/idml/src/names.rs b/crates/idml/src/names.rs index 70d02e7c..56924447 100644 --- a/crates/idml/src/names.rs +++ b/crates/idml/src/names.rs @@ -263,7 +263,7 @@ pub fn leading_out(l: Leading) -> (String, &'static str) { } } pub fn leading_in(s: &str) -> Option { - if s == "Auto" { Some(Leading::Auto) } else { s.trim().parse().ok().map(Leading::Points) } + if s == "Auto" { Some(Leading::Auto) } else { s.trim().parse::().ok().filter(|v| v.is_finite()).map(Leading::Points) } } pub fn stroke_align_out(a: StrokeAlign) -> &'static str { diff --git a/crates/idml/src/xml.rs b/crates/idml/src/xml.rs index b3f1ef0d..374e70c7 100644 --- a/crates/idml/src/xml.rs +++ b/crates/idml/src/xml.rs @@ -93,8 +93,9 @@ impl El { pub fn prop_el(&self, k: &str) -> Option<&El> { self.find("Properties").and_then(|p| p.find(k)) } + /// A finite numeric property; `NaN` and infinities count as absent. pub fn num(&self, k: &str) -> Option { - self.prop(k).and_then(|v| v.trim().parse().ok()) + self.prop(k).and_then(|v| v.trim().parse::().ok()).filter(|v| v.is_finite()) } pub fn boolean(&self, k: &str) -> Option { match self.prop(k)?.trim() { @@ -337,4 +338,12 @@ mod tests { assert_eq!(e.num("Leading"), Some(14.0)); assert_eq!(e.prop("Missing"), None); } + + /// `NaN` and infinities parse as `f64` but are not usable numbers. + #[test] + fn non_finite_numbers_count_as_absent() { + let x = br#"-inf2.5"#; + let e = parse(x).unwrap(); + assert_eq!((e.num("W"), e.num("X"), e.num("Y"), e.num("Z")), (None, None, None, Some(2.5))); + } }