From 8037ef4c4f3610d0c2cac40223f73685c214de2e Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 02:39:32 +0000 Subject: [PATCH] address review: enforce the CSV import cell limit while parsing --- crates/engine/src/cmd/extra/data.rs | 8 ++++++-- crates/xlsx/src/csv.rs | 15 +++++++++++++++ crates/xlsx/src/lib.rs | 5 ++++- crates/xlsx/src/tests/csv_tests.rs | 15 +++++++++++++++ 4 files changed, 40 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/extra/data.rs b/crates/engine/src/cmd/extra/data.rs index a99d87d31..5af7df0a8 100644 --- a/crates/engine/src/cmd/extra/data.rs +++ b/crates/engine/src/cmd/extra/data.rs @@ -96,8 +96,12 @@ pub(super) fn from_csv(s: &mut Session, p: &Json) -> Result { let delim = str_param(p, "delimiter") .and_then(|d| d.chars().next()) .unwrap_or(if text.lines().next().is_some_and(|l| l.contains('\t')) { '\t' } else { ',' }); - let opts = gridcraft_xlsx::CsvOptions { delimiter: delim as u8, ..Default::default() }; - let wb = gridcraft_xlsx::read_csv(&bytes, &opts).map_err(|e| EngineError::Other(e.to_string()))?; + let opts = gridcraft_xlsx::CsvOptions { delimiter: delim as u8, max_used_cells: Some(MAX_IMPORT_CELLS), ..Default::default() }; + // The cell limit is enforced while parsing, before an oversized file is stored. + let wb = gridcraft_xlsx::read_csv(&bytes, &opts).map_err(|e| match e { + gridcraft_xlsx::IoError::TooLarge(_) => too_many_cells(), + e => EngineError::Other(e.to_string()), + })?; let sh = wb.sheet(0).ok_or(EngineError::NoDocument)?; let Some(u) = sh.used_range() else { return Err(EngineError::Other("The file is empty.".into())) }; if u.count() > MAX_IMPORT_CELLS { diff --git a/crates/xlsx/src/csv.rs b/crates/xlsx/src/csv.rs index a2e5493f1..254e903f0 100644 --- a/crates/xlsx/src/csv.rs +++ b/crates/xlsx/src/csv.rs @@ -496,6 +496,9 @@ pub fn read_csv_with_report(bytes: &[u8], opts: &CsvOptions) -> Result<(Workbook let mut styles = std::mem::take(&mut wb.styles); let mut fmt_cache: Vec<(&'static str, StyleId)> = vec![]; let mut row: u32 = 0; + // The bounding box of the non-empty cells, which is the sheet's used range. + let mut used: Option<(u32, u32, u32, u32)> = None; + let mut too_many = false; parse_records(&text, delim, quote, |rec| { if row >= MAX_ROWS { return false; @@ -509,6 +512,8 @@ pub fn read_csv_with_report(bytes: &[u8], opts: &CsvOptions) -> Result<(Workbook continue; } let pos = CellRef::new(row, col as u32); + let (r0, c0, c1) = used.map_or((row, col as u32, col as u32), |(r0, c0, _, c1)| (r0, c0.min(col as u32), c1.max(col as u32))); + used = Some((r0, c0, row, c1)); let cell = if opts.parse_values { let rule = date_columns.get(col).map_or(DateRule::NoDates, |(rule, _)| *rule); let p = read_field(&f, opts.date_system, rule); @@ -529,9 +534,19 @@ pub fn read_csv_with_report(bytes: &[u8], opts: &CsvOptions) -> Result<(Workbook }; sheet.cells.set(pos, cell); } + // Stop at the first record that takes the used range past the limit, before more is stored. + if let (Some(max), Some((r0, c0, r1, c1))) = (opts.max_used_cells, used) + && u64::from(r1 - r0 + 1) * u64::from(c1 - c0 + 1) > max + { + too_many = true; + return false; + } row += 1; true }); + if too_many { + return Err(IoError::TooLarge(format!("the CSV's used range is over the limit of {} cells", opts.max_used_cells.unwrap_or(0)))); + } wb.styles = styles; wb.sheets = vec![Arc::new(sheet)]; Ok((wb, report)) diff --git a/crates/xlsx/src/lib.rs b/crates/xlsx/src/lib.rs index 5b94531c4..815d816cc 100644 --- a/crates/xlsx/src/lib.rs +++ b/crates/xlsx/src/lib.rs @@ -177,11 +177,14 @@ pub struct CsvOptions { pub date_system: DateSystem, /// Interpret fields like typed input (numbers, dates, booleans…); otherwise keep text. pub parse_values: bool, + /// Refuse (with [`IoError::TooLarge`], while reading) a file whose used range, from its first to + /// its last non-empty cell, would pass this many cells; `None` for no limit. + pub max_used_cells: Option, } impl Default for CsvOptions { fn default() -> Self { - CsvOptions { delimiter: b',', quote: b'"', date_system: DateSystem::D1900, parse_values: true } + CsvOptions { delimiter: b',', quote: b'"', date_system: DateSystem::D1900, parse_values: true, max_used_cells: None } } } diff --git a/crates/xlsx/src/tests/csv_tests.rs b/crates/xlsx/src/tests/csv_tests.rs index 585fb680f..ec4405f1b 100644 --- a/crates/xlsx/src/tests/csv_tests.rs +++ b/crates/xlsx/src/tests/csv_tests.rs @@ -406,3 +406,18 @@ fn parser_stops_collecting_fields_at_max_cols() { }); assert_eq!(lens, vec![MAX_COLS as usize]); } + +#[test] +fn csv_used_range_limit_is_enforced_while_reading() { + let opts = |max| CsvOptions { max_used_cells: Some(max), ..Default::default() }; + // 3 rows x 3 columns: exactly at the limit reads, one cell over is refused. + let dense = b"1,2,3\n4,5,6\n7,8,9\n"; + assert!(read_csv(dense, &opts(9)).is_ok()); + assert!(matches!(read_csv(dense, &opts(8)), Err(IoError::TooLarge(_)))); + // The limit is on the used range: empty leading rows, columns and trailing lines are free. + assert!(read_csv(b"\n\n,,x,y\n,,z,w\n\n\n", &opts(4)).is_ok()); + // A wide sparse row against a tall one: refused once the box passes the limit. + assert!(matches!(read_csv(b"a\nb\nc\n,,,,d\n", &opts(10)), Err(IoError::TooLarge(_)))); + // No limit by default. + assert!(read_csv(dense, &CsvOptions::default()).is_ok()); +}