Merge pull request #2240 from dotCooCoo/fix/pdf-unparseable-color-values

PDF/.ai import: dictionary values the reader can't parse no longer crash the ink and font scans

crates/pdf/src/import_color.rs

@@ -309,7 +309,7 @@ impl<'p> Colors<'p> {
                 }
             }
             Object::Dict(d) => {
-                for (_, v) in d.entries() {
+                for v in values(&d) {
                     if let Some(v) = direct(v) {
                         self.visit(v, depth + 1);
                     }
@@ -386,6 +386,13 @@ pub(crate) fn direct(m: MaybeRef<Object<'_>>) -> Option<Object<'_>> {
     }
 }
 
+/// The values of `d` that the reader can parse, sorted by key as `Dict::entries` sorts them.
+pub(crate) fn values<'a>(d: &craft_pdf::syntax::object::Dict<'a>) -> impl Iterator<Item = MaybeRef<Object<'a>>> {
+    let mut keys: Vec<Name<'a>> = d.keys().collect();
+    keys.sort_by(|a, b| a.as_ref().cmp(b.as_ref()));
+    keys.into_iter().filter_map(|k| d.get_raw::<Object<'a>>(k.as_ref()))
+}
+
 fn alt_model(o: &Object<'_>) -> Option<Alt> {
     match o {
         Object::Name(n) => match n.as_str() {

crates/pdf/src/import_scan.rs

@@ -415,7 +415,7 @@ impl Walker<'_, '_> {
     }
 
     fn note_fonts(&mut self, res: &Resources<'_>) {
-        for (key, _) in res.fonts.entries() {
+        for key in res.fonts.keys() {
             if let Some(f) = res.fonts.get::<Dict<'_>>(key.as_ref())
                 && let Some(name) = f.get::<Name<'_>>(b"BaseFont")
             {

crates/pdf/src/pages.rs

@@ -10,7 +10,7 @@ use craft_pdf::syntax::{DecryptionError, LoadPdfError, Pdf};
 use kurbo::{Affine, Rect};
 use vectorcraft_doc::setup::MAX_BLEED;
 
-use crate::import_color::direct;
+use crate::import_color::{direct, values};
 use crate::{Choice, CropTo, ImportOptions, PdfError};
 
 /// What `info` tells about one page.
@@ -46,11 +46,6 @@ const MAX_LINKS: usize = 16;
 /// The application's calculator-procedure depth policy, layered over the shared reader's limits.
 const MAX_PROCEDURES: usize = 64;
 
-/// The values of `d` that the reader can parse (`Dict::entries` panics on one it can't).
-fn values<'a>(d: &Dict<'a>) -> impl Iterator<Item = MaybeRef<Object<'a>>> {
-    d.keys().filter_map(|k| d.get_raw::<Object<'a>>(k.as_ref()))
-}
-
 /// What the function or colour space `o` is built on: the functions a stitching function joins;
 /// the base, alternate and tint of a colour space; the alternate of a colour profile. Nothing for
 /// any other object.

crates/pdf/src/tests_import_color.rs

@@ -216,6 +216,27 @@ fn colour_spaces_are_read_from_the_interpreters_debug_form() {
     assert_eq!(crate::import_color::split_color("Color { color_space: X, components: [NaN], opacity: 1.0 }"), None);
 }
 
+/// A dictionary value the reader can't parse (a name with a broken `#` escape, a number with a
+/// stray `-`) is left out when the file's ink spaces are looked for, and the inks after it are found.
+#[test]
+fn inks_are_found_past_dictionary_values_the_reader_cannot_parse() {
+    use craft_pdf::interpret::Function;
+    use craft_pdf::syntax::object::{FromBytes, Object};
+    let tint = "<< /FunctionType 2 /Domain [0 1] /C0 [0 0 0 0] /C1 [0 0 1 0] /N 1 >>";
+    let key = format!("{:?}", Function::new(&Object::from_bytes(tint.as_bytes()).unwrap()).unwrap());
+    for junk in ["/#zz", "12-.5"] {
+        let page = page_with(&format!("/A {junk} /CS0 [/Separation /Gold /DeviceCMYK {tint}]"), &[("CS0", "1")]);
+        // Opened by the reader alone, without the checks of `pages::open`.
+        let pdf = craft_pdf::syntax::Pdf::new(pdf(&[page], None)).unwrap();
+        let mut c = Colors::new(&pdf, vec![]);
+        let space = c.space_of(&format!("ColorSpace(Separation(Separation {{ tint_transform: {key} }}))"), 1);
+        assert_eq!(space, Space::Inks(0), "{junk}");
+        let gold = c.native(space, &[1.0]).unwrap();
+        assert_eq!(gold.link, Some(("Gold".to_string(), 1.0)), "{junk}");
+        assert!(near(gold.color, Color::cmyk(0.0, 0.0, 1.0, 0.0)), "{junk}: {:?}", gold.color);
+    }
+}
+
 /// Spot colours defined in Lab can have an ICC Lab profile as their alternate space, `[/ICCBased
 /// profile]` with 3 components: the tint transform gives Lab values, which are not RGB (#1032). The
 /// profile is told by its `Alternate` (`[/Lab …]`), else by its header's data colour space.

crates/pdf/src/tests_import_fidelity.rs

@@ -321,6 +321,23 @@ fn font_names_resolve_to_available_families() {
     assert_eq!((family.as_str(), style.as_str(), found), ("Missing Sans Pro", "Bold Italic", false));
 }
 
+/// A value of the page's font dictionary that the reader can't parse (a name with a broken `#`
+/// escape, a number with a stray `-`) is left out when the page's fonts are noted, and the fonts
+/// after it are noted.
+#[test]
+fn fonts_are_noted_past_font_resources_the_reader_cannot_parse() {
+    for junk in ["/#zz", "12-.5"] {
+        let resources = format!("/Font << /A {junk} /F1 << /Type /Font /Subtype /Type1 /BaseFont /Helvetica >> >>");
+        let bytes = one_page("BT /F1 24 Tf 10 30 Td (Hello) Tj ET", &resources, &[]);
+        // Opened by the reader alone, without the checks of `pages::open`.
+        let pdf = craft_pdf::syntax::Pdf::new(bytes).unwrap();
+        let mut fonts = crate::import_scan::FontNames::new();
+        let mut ocgs = crate::import_scan::Ocgs::read(&pdf);
+        crate::import_scan::scan_page(&pdf.pages()[0], &mut ocgs, false, &mut fonts, pdf.decode_limits().max_decoded_stream()).unwrap();
+        assert_eq!(fonts.into_values().collect::<Vec<_>>(), [("Helvetica".to_string(), false)], "{junk}");
+    }
+}
+
 // ---------- meshes, stops and extend ----------
 
 fn hex(bytes: &[u8]) -> String {