aleleba/gridcraft
1
0
Fork 0
mirror of https://github.com/storytold/gridcraft.git synced 2026-10-31 22:26:57 -06:00
Code Issues Packages Projects Releases Wiki Activity

Review fix-up: clip blank ODS repeats past the sheet edge, struct-update Cell literal

Browse Source 
Blank cell and row repeats that run past 16384 columns / 1048576 rows
(padding from writers with larger sheets) are clipped instead of
rejecting the file; any data or merge past the edge is still an error.
convert() builds its Cell with ..Cell::value(value) so new Cell fields
take their defaults.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Brandon Thomas
2026-10-10 12:22:51 -04:00
co-authored by Claude Opus 5.5
parent 26b3db78db
commit 20e39fa3cd
5 changed files with 46 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/gridcraft-web/src/web.rs
+4 -2
View File
@@ -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;
};
crates/engine/src/cmd/file.rs
+1 -2
View File
@@ -47,8 +47,7 @@ fn open(s: &mut Session, p: &Json) -> Result<Json> {
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"))
crates/xlsx/src/lib.rs
+1 -1
View File
@@ -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)]
crates/xlsx/src/ods.rs
+18 -11
View File
@@ -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() {
crates/xlsx/src/tests/ods_tests.rs
+22 -2
View File
@@ -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#"<table:table-cell table:number-columns-repeated="4294967295"/>"#,
r#"<table:table-cell table:number-columns-repeated="4294967296"/>"#,
r#"<table:table-cell table:number-columns-repeated="4294967295" office:value-type="float" office:value="1"/>"#,
r#"<table:table-cell table:number-columns-repeated="0"/>"#,
r#"<table:table-cell table:number-columns-repeated="16384"/><table:table-cell/>"#,
r#"<table:table-cell table:number-columns-repeated="16384"/><table:table-cell office:value-type="float" office:value="1"/>"#,
r#"<table:table-cell table:number-columns-repeated="16384"/><table:table-cell table:number-columns-spanned="2"/>"#,
r#"<table:table-cell table:number-columns-repeated="16384" table:number-columns-spanned="2" office:value-type="float" office:value="1"/>"#,
r#"<table:table-cell office:value-type="string"><text:p><text:s text:c="4294967295"/></text:p></table:table-cell>"#,
] {
@@ -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#"<table:table table:name="Data">
<table:table-row><table:table-cell office:value-type="float" office:value="1"/><table:table-cell table:number-columns-repeated="4294967295"/><table:table-cell/></table:table-row>
<table:table-row table:number-rows-repeated="16777215"><table:table-cell table:number-columns-repeated="16777216"/></table:table-row>
<table:table-row table:number-rows-repeated="5"><table:table-cell/></table:table-row>
</table:table>"#);
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#"<table:table table:name="Data"><table:table-row table:number-rows-repeated="1048576"><table:table-cell/></table:table-row><table:table-row><table:table-cell office:value-type="float" office:value="1"/></table:table-row></table:table>"#,
);
assert!(matches!(read_ods(&late), Err(IoError::TooLarge(_))));
}