Merge pull request #1193 from Silvertree2010/fix/h2418-idml-import-accepts-nan-inf-in-s

Ignore NaN and infinite numbers when importing IDML

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}>` / `<{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<u8>, 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:#?}");
+}

crates/idml/src/import.rs

@@ -391,6 +391,12 @@ fn nums(s: &str) -> Vec<f64> {
     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<f64> {
+    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::<f64>().ok()).collect::<Vec<_>>();
             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;
@@ -1832,7 +1842,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::<f64>().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(),
@@ -2191,8 +2201,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 rs = (span(ce, "RowSpan").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 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 {
@@ -2753,7 +2763,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::<f64>().ok()).filter(|v| v.is_finite()) {
             s.weight = w;
         }
         if let Some(t) = g(self, "StrokeTint").and_then(|v| v.parse().ok()) {
@@ -2771,7 +2781,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::<f64>().ok()).filter(|v| v.is_finite()) {
             s.miter_limit = v;
         }
         if let Some(v) = g(self, "LeftLineEnd") {
@@ -2837,7 +2847,7 @@ impl<'r> Importer<'r> {
             {
                 it.fill.gradient_angle = Some(a);
             }
-            let start: Option<Vec<f64>> = e.prop("GradientFillStart").map(|s| s.split_whitespace().filter_map(|v| v.parse().ok()).collect());
+            let start: Option<Vec<f64>> = 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
             {
@@ -2855,7 +2865,7 @@ impl<'r> Importer<'r> {
             {
                 it.stroke.gradient_angle = Some(a);
             }
-            let start: Option<Vec<f64>> = e.prop("GradientStrokeStart").map(|s| s.split_whitespace().filter_map(|v| v.parse().ok()).collect());
+            let start: Option<Vec<f64>> = 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
             {
@@ -3309,6 +3319,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::<f64>().ok()).unwrap_or(1.0)
+}
+
 fn finite(v: Option<f64>) -> Option<f64> {
     v.filter(|v| v.is_finite())
 }

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<Leading> {
-    if s == "Auto" { Some(Leading::Auto) } else { s.trim().parse().ok().map(Leading::Points) }
+    if s == "Auto" { Some(Leading::Auto) } else { s.trim().parse::<f64>().ok().filter(|v| v.is_finite()).map(Leading::Points) }
 }
 
 pub fn stroke_align_out(a: StrokeAlign) -> &'static str {

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<f64> {
-        self.prop(k).and_then(|v| v.trim().parse().ok())
+        self.prop(k).and_then(|v| v.trim().parse::<f64>().ok()).filter(|v| v.is_finite())
     }
     pub fn boolean(&self, k: &str) -> Option<bool> {
         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#"<A W="NaN" X="inf"><Properties><Y type="unit">-inf</Y><Z>2.5</Z></Properties></A>"#;
+        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)));
+    }
 }