Merge pull request #15 from storytold/pr/never-crash-importers

Never crash: importers (text, IDML, fonts, format, EPUB, images)

crates/epub/src/lib.rs

@@ -4,6 +4,7 @@
 //! frame); paragraph styles become CSS classes (`p.<slug>`), character styles become `span`
 //! classes, local overrides become inline styles. Placed graphics become `<figure>` elements at
 //! their position in the reading order. Parent-page items (folios, running heads) are skipped.
+#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
 #![forbid(unsafe_code)]
 
 use std::collections::BTreeMap;

crates/fonts/src/fontdb.rs

@@ -304,6 +304,20 @@ fn enumerate_faces(data: &[u8]) -> Vec<Found> {
     out
 }
 
+/// The compiled-in Source Sans 3 Regular, for a database that somehow has no faces at all.
+pub(crate) fn last_resort_face() -> Arc<FontFace> {
+    static FACE: std::sync::OnceLock<Arc<FontFace>> = std::sync::OnceLock::new();
+    FACE.get_or_init(|| {
+        // The font is compiled in (`include_bytes!`), so parsing it can't depend on input; the
+        // `last_resort_face_parses` test proves it on every run.
+        #[allow(clippy::expect_used)]
+        let face = make_face(FontBytes::Static(BUNDLED[0]), 0, FALLBACK_FAMILY.into(), "Regular".into(), Vec::new())
+            .expect("the compiled-in Source Sans 3 Regular parses");
+        Arc::new(face)
+    })
+    .clone()
+}
+
 fn make_face(bytes: FontBytes, index: u32, family: String, style: String, coords: Vec<([u8; 4], f32)>) -> Option<FontFace> {
     let data: &[u8] = match &bytes {
         FontBytes::Static(b) => b,
@@ -550,7 +564,7 @@ impl FontDb {
         self.find(FALLBACK_FAMILY, style)
             .or_else(|| self.find(FALLBACK_FAMILY, "Regular"))
             .or_else(|| self.read_faces().first().cloned())
-            .expect("bundled fonts are always present")
+            .unwrap_or_else(last_resort_face)
     }
 
     /// Is `family` available (loaded)?

crates/fonts/src/lib.rs

@@ -4,6 +4,7 @@
 //! Shaping here is style-agnostic: [`shape`] turns a string in one face into glyph ids, clusters
 //! and advances in font units. `designcraft-compose` applies sizes, tracking, scaling and
 //! justification on top.
+#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
 #![forbid(unsafe_code)]
 
 mod fontdb;
@@ -150,6 +151,13 @@ pub fn first_glyph(face: &FontFace, chars: &[char]) -> u32 {
 mod tests {
     use super::*;
 
+    #[test]
+    fn last_resort_face_parses() {
+        let f = fontdb::last_resort_face();
+        assert_eq!(f.family, fontdb::FALLBACK_FAMILY);
+        assert!(f.upem > 0.0);
+    }
+
     #[test]
     fn bundled_families_load() {
         let db = FontDb::global();

crates/format/src/lib.rs

@@ -6,6 +6,7 @@
 //! - `meta.json`: format version and generator
 //!
 //! Older single-file JSON documents (assets as base64 in `assetData`) still open.
+#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
 #![forbid(unsafe_code)]
 
 use std::io::{Cursor, Read, Write};

crates/idml/src/export.rs

@@ -143,7 +143,7 @@ impl<'a> Ex<'a> {
         let mut ex = Ex {
             d,
             opts,
-            next: d.next_id + 0x100,
+            next: d.next_id.saturating_add(0x100),
             swatch_ids: HashMap::new(),
             fonts: BTreeMap::new(),
             threads: HashMap::new(),
@@ -2008,7 +2008,7 @@ impl<'a> Ex<'a> {
                 if owners[r * t.ncols() + c] != (r, c) {
                     continue;
                 }
-                let cell = t.cell(r, c).expect("in range");
+                let Some(cell) = t.cell(r, c) else { continue };
                 let vj = match cell.vj {
                     designcraft_doc::VerticalJustification::Top => "TopAlign",
                     designcraft_doc::VerticalJustification::Center => "CenterAlign",

crates/idml/src/import.rs

@@ -47,7 +47,8 @@ pub fn import_idml_with(bytes: &[u8], read_link: &dyn Fn(&str) -> Option<Vec<u8>
             continue;
         }
         let name = f.name().to_string();
-        let mut buf = Vec::with_capacity(f.size() as usize);
+        // 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() })?;
         files.insert(name, buf);
     }
@@ -1357,7 +1358,7 @@ impl<'r> Importer<'r> {
         }
         let h = e.num("HeaderRowCount").unwrap_or(0.0) as usize;
         let f = e.num("FooterRowCount").unwrap_or(0.0) as usize;
-        if h + f < nr {
+        if h.saturating_add(f) < nr {
             t.set_header_footer(h, f);
         }
         let stroke = |me: &mut Self, el: &El, prefix: &str, base: &CellStroke| -> CellStroke {

crates/idml/src/lib.rs

@@ -35,6 +35,7 @@
 //!   basic feather, EPS/PDF/AI placed graphics (imported as images when the data is available),
 //!   clipping paths, compound-path fill rules.
 //! - Unknown elements are ignored; nothing is preserved opaquely for round-trip yet.
+#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
 
 mod export;
 mod import;

crates/images/src/lib.rs

@@ -2,6 +2,7 @@
 //! Rasters go through `image` (PNG, JPEG, GIF, WebP, TIFF, BMP) or `psd` (Photoshop's merged
 //! composite); SVG is parsed with usvg (text set in the bundled fonts) and rasterised with resvg
 //! for the screen — PDF export draws the same tree as vectors.
+#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
 
 use std::sync::{Arc, OnceLock};
 

crates/textimport/src/lib.rs

@@ -5,6 +5,7 @@
 //! Word styles are imported by name (their font, size, bold/italic and space before/after become
 //! the style definition, like InDesign's "Use Word styles" import option). RTF keeps bold,
 //! italic, underline, size and paragraph breaks.
+#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
 #![forbid(unsafe_code)]
 
 mod docx;

crates/textimport/src/tagged.rs

@@ -104,7 +104,7 @@ pub fn import(bytes: &[u8]) -> Result<Imported, ImportError> {
     let mut char_styles: Vec<String> = Vec::new();
     let mut warnings = Vec::new();
     let push = |paras: &mut Vec<(String, Vec<(String, CharFormat)>)>, c: char, fmt: &CharFormat| {
-        let runs = &mut paras.last_mut().expect("one paragraph").1;
+        let Some((_, runs)) = paras.last_mut() else { return };
         match runs.last_mut() {
             Some((t, f)) if f == fmt => t.push(c),
             _ => runs.push((c.to_string(), fmt.clone())),
@@ -153,7 +153,9 @@ pub fn import(bytes: &[u8]) -> Result<Imported, ImportError> {
                         if !v.is_empty() && !para_styles.contains(&v) {
                             para_styles.push(v.clone());
                         }
-                        paras.last_mut().expect("one paragraph").0 = v;
+                        if let Some(p) = paras.last_mut() {
+                            p.0 = v;
+                        }
                     }
                     "CharStyle" => {
                         let v = value.trim();

crates/textimport/src/xlsx.rs

@@ -10,7 +10,10 @@ use crate::{ImportError, Imported};
 fn cell_ref(r: &str) -> Option<(usize, usize)> {
     let letters: String = r.chars().take_while(|c| c.is_ascii_alphabetic()).collect();
     let row: usize = r[letters.len()..].parse().ok()?;
-    let col = letters.chars().try_fold(0usize, |acc, c| Some(acc * 26 + (c.to_ascii_uppercase() as u8).checked_sub(b'A')? as usize + 1))?;
+    let col = letters.chars().try_fold(0usize, |acc, c| {
+        let digit = (c.to_ascii_uppercase() as u8).checked_sub(b'A')? as usize + 1;
+        acc.checked_mul(26)?.checked_add(digit)
+    })?;
     Some((row.checked_sub(1)?, col.checked_sub(1)?))
 }
 
@@ -72,21 +75,29 @@ pub fn import(bytes: &[u8]) -> Result<Imported, ImportError> {
     if cells.is_empty() {
         warnings.push("the worksheet is empty".into());
     }
-    // The used range, from the first used row/column.
+    // The used range, from the first used row/column. Cell references come from the file: the
+    // table is capped before anything is allocated for it.
+    const MAX_CELLS: usize = 100_000;
+    const MAX_COLS: usize = 1_000;
     let (r0, c0) = cells.iter().fold((usize::MAX, usize::MAX), |(r, c), ((a, b), _)| (r.min(*a), c.min(*b)));
     let (r1, c1) = cells.iter().fold((0, 0), |(r, c), ((a, b), _)| (r.max(*a), c.max(*b)));
-    let mut rows: Vec<Vec<String>> = if cells.is_empty() { vec![vec![String::new()]] } else { vec![vec![String::new(); c1 - c0 + 1]; r1 - r0 + 1] };
-    for ((r, c), v) in cells {
-        rows[r - r0][c - c0] = v;
+    let (nrows, ncols) = if cells.is_empty() { (1, 1) } else { ((r1 - r0).saturating_add(1), (c1 - c0).saturating_add(1)) };
+    let ncols_kept = ncols.min(MAX_COLS);
+    if ncols_kept < ncols {
+        warnings.push(format!("only the first {ncols_kept} columns were placed"));
     }
-    const MAX_CELLS: usize = 100_000;
-    if rows.len() * rows[0].len() > MAX_CELLS {
-        let keep = (MAX_CELLS / rows[0].len()).max(1);
-        warnings.push(format!("only the first {keep} rows were placed"));
-        rows.truncate(keep);
+    let nrows_kept = nrows.min((MAX_CELLS / ncols_kept).max(1));
+    if nrows_kept < nrows {
+        warnings.push(format!("only the first {nrows_kept} rows were placed"));
+    }
+    let mut rows: Vec<Vec<String>> = vec![vec![String::new(); ncols_kept]; nrows_kept];
+    for ((r, c), v) in cells {
+        if let Some(cell) = rows.get_mut(r - r0).and_then(|row| row.get_mut(c - c0)) {
+            *cell = v;
+        }
     }
     let mut story = Story::new(StoryId(0));
-    let width = 72.0 * rows[0].len() as f64;
+    let width = 72.0 * ncols_kept as f64;
     let t = Table::from_strings(1, &rows, width, &ParaFormat::default(), &CharFormat::default());
     story.insert_table(0, t);
     Ok(Imported { story, para_styles: vec![], char_styles: vec![], warnings })
@@ -99,6 +110,12 @@ mod tests {
     use super::*;
 
     fn workbook() -> Vec<u8> {
+        workbook_with(
+            r#"<worksheet><sheetData><row r="2"><c r="B2" t="s"><v>0</v></c><c r="C2" t="s"><v>1</v></c></row><row r="3"><c r="B3" t="s"><v>2</v></c><c r="C3"><v>3.5</v></c></row><row r="4"><c r="B4" t="inlineStr"><is><t>Cake</t></is></c><c r="C4"><v>12.0</v></c></row></sheetData></worksheet>"#,
+        )
+    }
+
+    fn workbook_with(sheet: &str) -> Vec<u8> {
         let mut buf = std::io::Cursor::new(Vec::new());
         {
             let mut z = zip::ZipWriter::new(&mut buf);
@@ -113,10 +130,7 @@ mod tests {
                     r#"<Relationships xmlns="http://schemas.openxmlformats.org/package/2006/relationships"><Relationship Id="rId1" Type="x" Target="worksheets/sheet1.xml"/></Relationships>"#,
                 ),
                 ("xl/sharedStrings.xml", r#"<sst><si><t>Item</t></si><si><t>Price</t></si><si><r><t>Te</t></r><r><t>a</t></r></si></sst>"#),
-                (
-                    "xl/worksheets/sheet1.xml",
-                    r#"<worksheet><sheetData><row r="2"><c r="B2" t="s"><v>0</v></c><c r="C2" t="s"><v>1</v></c></row><row r="3"><c r="B3" t="s"><v>2</v></c><c r="C3"><v>3.5</v></c></row><row r="4"><c r="B4" t="inlineStr"><is><t>Cake</t></is></c><c r="C4"><v>12.0</v></c></row></sheetData></worksheet>"#,
-                ),
+                ("xl/worksheets/sheet1.xml", sheet),
             ] {
                 z.start_file(name, o).unwrap();
                 z.write_all(body.as_bytes()).unwrap();
@@ -135,4 +149,18 @@ mod tests {
         assert_eq!([cell(0, 0), cell(0, 1), cell(1, 0), cell(1, 1), cell(2, 0), cell(2, 1)], ["Item", "Price", "Tea", "3.5", "Cake", "12"]);
         assert_eq!(cell_ref("AA10"), Some((9, 26)));
     }
+
+    /// A cell far away from the others used to allocate the whole used range (billions of cells)
+    /// before the cell cap applied; a long column name overflowed the column number.
+    #[test]
+    fn far_away_cell_references_are_capped() {
+        let wb = workbook_with(
+            r#"<worksheet><sheetData><row><c r="A1" t="inlineStr"><is><t>a</t></is></c><c r="A999999999" t="inlineStr"><is><t>z</t></is></c><c r="XFDXFDXFDXFDXFDXFD1" t="inlineStr"><is><t>far</t></is></c></row></sheetData></worksheet>"#,
+        );
+        let imp = import(&wb).unwrap();
+        let t = imp.story.tables.values().next().expect("a table");
+        assert!(t.nrows() * t.ncols() <= 100_000);
+        assert!(!imp.warnings.is_empty());
+        assert_eq!(cell_ref("XFDXFDXFDXFDXFDXFD1"), None);
+    }
 }