diff --git a/apps/gridcraft-web/src/web.rs b/apps/gridcraft-web/src/web.rs index 43d9cf50a..479efcfda 100644 --- a/apps/gridcraft-web/src/web.rs +++ b/apps/gridcraft-web/src/web.rs @@ -97,8 +97,10 @@ fn services(inbox: Inbox, ctx: egui::Context) -> Services { let inbox = open_inbox.clone(); let ctx = ctx.clone(); wasm_bindgen_futures::spawn_local(async move { - let Some(file) = - rfd::AsyncFileDialog::new().add_filter("Spreadsheets", &["xlsx", "xlsm", "xlsb", "ods", "csv", "tsv", "txt", "json"]).pick_file().await + let Some(file) = rfd::AsyncFileDialog::new() + .add_filter("Spreadsheets", &["xlsx", "xlsm", "xlsb", "ods", "csv", "tsv", "txt", "json"]) + .pick_file() + .await else { return; }; diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 34c1ad357..d52418b4c 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -47,8 +47,7 @@ fn open(s: &mut Session, p: &Json) -> Result { return ok(); }; let (wb, warnings) = crate::io::open_bytes(&name, &bytes)?; - let imported = - matches!(crate::io::FileKind::from_path(&name), Some(crate::io::FileKind::Ods | crate::io::FileKind::Xlsb)) + let imported = matches!(crate::io::FileKind::from_path(&name), Some(crate::io::FileKind::Ods | crate::io::FileKind::Xlsb)) || matches!(gridcraft_xlsx::sniff(&bytes), gridcraft_xlsx::Format::Ods | gridcraft_xlsx::Format::Xlsb); let title = if imported { format!("{} (imported).xlsx", std::path::Path::new(&name).file_stem().and_then(|n| n.to_str()).unwrap_or("Book")) diff --git a/crates/xlsx/src/lib.rs b/crates/xlsx/src/lib.rs index 7346cf5e9..5245e561d 100644 --- a/crates/xlsx/src/lib.rs +++ b/crates/xlsx/src/lib.rs @@ -34,8 +34,8 @@ use gridcraft_core::DateSystem; use gridcraft_model::Workbook; pub use csv::{read_csv, write_csv}; -pub use xlsb::read_xlsb; pub use ods::read_ods; +pub use xlsb::read_xlsb; /// Errors from reading or writing files. #[derive(Debug, thiserror::Error)] diff --git a/crates/xlsx/src/ods.rs b/crates/xlsx/src/ods.rs index c41bec6d5..4f63fd1f0 100644 --- a/crates/xlsx/src/ods.rs +++ b/crates/xlsx/src/ods.rs @@ -327,7 +327,7 @@ impl Import { } Tag::Row if self.sheet.as_ref().is_some_and(|s| self.stack.get(s.depth + 1..).is_some_and(|p| p.iter().all(|t| *t == Tag::Rows))) => { self.row = - Some(RowDraft { depth, repeat: positive(attr(reader, e, TABLE, b"number-rows-repeated")?, 1, MAX_ROWS)?, col: 0, cells: vec![] }); + Some(RowDraft { depth, repeat: positive(attr(reader, e, TABLE, b"number-rows-repeated")?, 1, u32::MAX)?, col: 0, cells: vec![] }); } Tag::Cell | Tag::Covered if self.row.as_ref().is_some_and(|r| depth == r.depth + 1) => { let kind = if attr(reader, e, CALC_EXT, b"value-type")?.as_deref() == Some("error") { @@ -344,7 +344,7 @@ impl Import { }; self.cell = Some(CellDraft { depth, - repeat: positive(attr(reader, e, TABLE, b"number-columns-repeated")?, 1, MAX_COLS)?, + repeat: positive(attr(reader, e, TABLE, b"number-columns-repeated")?, 1, u32::MAX)?, width: positive(attr(reader, e, TABLE, b"number-columns-spanned")?, 1, MAX_COLS)?, height: positive(attr(reader, e, TABLE, b"number-rows-spanned")?, 1, MAX_ROWS)?, covered: t == Tag::Covered, @@ -460,12 +460,15 @@ impl Import { let cell = self.convert(&draft); let (width, height) = if draft.covered { (1, 1) } else { (draft.width, draft.height) }; let Some(row) = self.row.as_mut() else { return Ok(()) }; - let end = row - .col - .checked_add(draft.repeat) - .filter(|n| *n <= MAX_COLS) - .ok_or_else(|| IoError::TooLarge("ODS row exceeds worksheet width".into()))?; - if !cell.is_blank() || width > 1 || height > 1 { + let kept = !cell.is_blank() || width > 1 || height > 1; + // Writers pad rows with blank repeats up to their own (possibly wider) sheet edge; + // blank cells past ours carry no data, so clip them instead of rejecting the file. + let end = match row.col.checked_add(draft.repeat).filter(|n| *n <= MAX_COLS) { + Some(end) => end, + None if !kept => MAX_COLS, + None => return Err(IoError::TooLarge("ODS row exceeds worksheet width".into())), + }; + if kept { row.cells.push(RepeatedCell { col: row.col, repeat: draft.repeat, width, height, cell }); } row.col = end; @@ -516,14 +519,18 @@ impl Import { }, None => StyleId::DEFAULT, }; - Cell { value, formula: None, style } + Cell { style, ..Cell::value(value) } } fn finish_row(&mut self) -> Result<(), IoError> { let Some(row) = self.row.take() else { return Ok(()) }; let Some(sheet) = self.sheet.as_mut() else { return Ok(()) }; - let end = - sheet.row.checked_add(row.repeat).filter(|n| *n <= MAX_ROWS).ok_or_else(|| IoError::TooLarge("ODS exceeds worksheet height".into()))?; + // Blank trailing row repeats past the sheet edge (e.g. from larger-sheet writers) are clipped. + let end = match sheet.row.checked_add(row.repeat).filter(|n| *n <= MAX_ROWS) { + Some(end) => end, + None if row.cells.is_empty() => MAX_ROWS, + None => return Err(IoError::TooLarge("ODS exceeds worksheet height".into())), + }; for entry in row.cells { let count = u64::from(entry.repeat) * u64::from(row.repeat); if !entry.cell.is_blank() { diff --git a/crates/xlsx/src/tests/ods_tests.rs b/crates/xlsx/src/tests/ods_tests.rs index 69f92c25a..c0f2e0e21 100644 --- a/crates/xlsx/src/tests/ods_tests.rs +++ b/crates/xlsx/src/tests/ods_tests.rs @@ -165,9 +165,11 @@ fn ods_rejects_bad_packages_encryption_and_truncated_xml() { #[test] fn ods_expansion_bounds_stop_small_repeat_bombs() { for cells in [ - r#""#, + r#""#, + r#""#, r#""#, - r#""#, + r#""#, + r#""#, r#""#, r#""#, ] { @@ -216,3 +218,21 @@ fn ods_extension_error_cache_never_imports_its_placeholder_zero() { assert_eq!(wb.sheets[0].value(CellRef::new(0, 0)), text("#DIV/0!")); assert!(report.warnings.iter().any(|w| w.contains("unsupported or missing"))); } + +#[test] +fn ods_blank_repeats_past_the_sheet_edge_are_clipped() { + // Writers with larger sheets pad to their own edge; blank padding past ours carries no data. + let bytes = ods(r#" + + + + "#); + let (wb, _) = read_ods(&bytes).unwrap(); + assert_eq!(wb.sheets[0].cells.len(), 1); + assert_eq!(wb.sheets[0].value(CellRef::new(0, 0)), Value::Number(1.0)); + // Data after the clipped edge is still an error, never silently dropped. + let late = ods( + r#""#, + ); + assert!(matches!(read_ods(&late), Err(IoError::TooLarge(_)))); +}