From d84107aacabd1154850b27b02bad226053882bee Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 11:39:37 -0400 Subject: [PATCH] Never crash: importers (text, IDML, fonts, format, EPUB, images) An .xlsx cell reference far from the others no longer allocates the whole range before the cell cap (billions of cells, out of memory), and long column names can't overflow. IDML import doesn't trust zip part sizes for up-front allocation and adds header/footer counts saturating. Tagged Text and IDML export use let-else; the font fallback has a compiled-in last resort. These crates deny the panicking lints. Co-Authored-By: Claude Opus 5.5 --- clippy.toml | 4 +++ crates/epub/src/lib.rs | 1 + crates/fonts/src/fontdb.rs | 16 ++++++++- crates/fonts/src/lib.rs | 8 +++++ crates/format/src/lib.rs | 1 + crates/idml/src/export.rs | 4 +-- crates/idml/src/import.rs | 5 +-- crates/idml/src/lib.rs | 1 + crates/images/src/lib.rs | 1 + crates/textimport/src/lib.rs | 1 + crates/textimport/src/tagged.rs | 6 ++-- crates/textimport/src/xlsx.rs | 58 ++++++++++++++++++++++++--------- 12 files changed, 84 insertions(+), 22 deletions(-) create mode 100644 clippy.toml diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 0000000..4b6c79f --- /dev/null +++ b/clippy.toml @@ -0,0 +1,4 @@ +allow-unwrap-in-tests = true +allow-expect-in-tests = true +allow-panic-in-tests = true +allow-indexing-slicing-in-tests = true diff --git a/crates/epub/src/lib.rs b/crates/epub/src/lib.rs index 62715c3..3a61b50 100644 --- a/crates/epub/src/lib.rs +++ b/crates/epub/src/lib.rs @@ -4,6 +4,7 @@ //! frame); paragraph styles become CSS classes (`p.`), character styles become `span` //! classes, local overrides become inline styles. Placed graphics become `
` 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; diff --git a/crates/fonts/src/fontdb.rs b/crates/fonts/src/fontdb.rs index eb44b88..c4b37e8 100644 --- a/crates/fonts/src/fontdb.rs +++ b/crates/fonts/src/fontdb.rs @@ -304,6 +304,20 @@ fn enumerate_faces(data: &[u8]) -> Vec { 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 { + static FACE: std::sync::OnceLock> = 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 { 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)? diff --git a/crates/fonts/src/lib.rs b/crates/fonts/src/lib.rs index 3193517..2d65ac0 100644 --- a/crates/fonts/src/lib.rs +++ b/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(); diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 22814b0..5b4896c 100644 --- a/crates/format/src/lib.rs +++ b/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}; diff --git a/crates/idml/src/export.rs b/crates/idml/src/export.rs index 0600520..6e999d5 100644 --- a/crates/idml/src/export.rs +++ b/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", diff --git a/crates/idml/src/import.rs b/crates/idml/src/import.rs index a3a97b9..8cf17fe 100644 --- a/crates/idml/src/import.rs +++ b/crates/idml/src/import.rs @@ -47,7 +47,8 @@ pub fn import_idml_with(bytes: &[u8], read_link: &dyn Fn(&str) -> Option 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 { diff --git a/crates/idml/src/lib.rs b/crates/idml/src/lib.rs index ba86d74..25ae9ee 100644 --- a/crates/idml/src/lib.rs +++ b/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; diff --git a/crates/images/src/lib.rs b/crates/images/src/lib.rs index eec7a4a..d2fd3ee 100644 --- a/crates/images/src/lib.rs +++ b/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}; diff --git a/crates/textimport/src/lib.rs b/crates/textimport/src/lib.rs index 6523d13..fd328a8 100644 --- a/crates/textimport/src/lib.rs +++ b/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; diff --git a/crates/textimport/src/tagged.rs b/crates/textimport/src/tagged.rs index e3ae05a..ca78bc4 100644 --- a/crates/textimport/src/tagged.rs +++ b/crates/textimport/src/tagged.rs @@ -104,7 +104,7 @@ pub fn import(bytes: &[u8]) -> Result { let mut char_styles: Vec = 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 { 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(); diff --git a/crates/textimport/src/xlsx.rs b/crates/textimport/src/xlsx.rs index f6d0c80..b131698 100644 --- a/crates/textimport/src/xlsx.rs +++ b/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 { 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> = 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![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 { + workbook_with( + r#"0123.5Cake12.0"#, + ) + } + + fn workbook_with(sheet: &str) -> Vec { let mut buf = std::io::Cursor::new(Vec::new()); { let mut z = zip::ZipWriter::new(&mut buf); @@ -113,10 +130,7 @@ mod tests { r#""#, ), ("xl/sharedStrings.xml", r#"ItemPriceTea"#), - ( - "xl/worksheets/sheet1.xml", - r#"0123.5Cake12.0"#, - ), + ("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#"azfar"#, + ); + 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); + } }