From 5ce63e5fc24120c94aa14e5148c240128d81c750 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 03:48:32 -0500 Subject: [PATCH] Get Data: refuse an import whose dense rows would be too large (#3193) * Get Data: refuse an import whose dense rows would be too large data.getData and data.fromTextCsv built every row densely before writing it: JSON records were padded to the union of all keys (found with a quadratic Vec::contains scan), and CSV cells were expanded over the whole used rectangle. A few sparse records or one far-away CSV cell therefore allocated billions of slots. The importer now collects JSON keys with a HashSet, refuses more keys than a sheet has columns, and checks the projected rows x columns against MAX_IMPORT_CELLS (10,000,000) before building any row. * address review: stop collecting JSON keys at the column limit and reject oversized row counts before building rows * address review: enforce the CSV import cell limit while parsing --- crates/engine/src/cmd/extra/data.rs | 66 +++++++++++++++++++++++++++-- crates/engine/src/tests.rs | 12 ++++++ crates/xlsx/src/csv.rs | 15 +++++++ crates/xlsx/src/lib.rs | 5 ++- crates/xlsx/src/tests/csv_tests.rs | 15 +++++++ 5 files changed, 109 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/extra/data.rs b/crates/engine/src/cmd/extra/data.rs index 9a1ae7417..5af7df0a8 100644 --- a/crates/engine/src/cmd/extra/data.rs +++ b/crates/engine/src/cmd/extra/data.rs @@ -42,6 +42,15 @@ pub(super) fn table_resize(s: &mut Session, p: &Json) -> Result { }) } +/// Most cells (rows × columns, blanks included) Get Data imports. The rows are built densely +/// before they are written, so a few sparse records or one far-away CSV cell must not expand +/// into a rectangle of billions of slots. +const MAX_IMPORT_CELLS: u64 = 10_000_000; + +fn too_many_cells() -> EngineError { + EngineError::Other(format!("The data is too large to import ({MAX_IMPORT_CELLS} cells at most).")) +} + pub(super) fn from_csv(s: &mut Session, p: &Json) -> Result { let bytes = if let Some(t) = str_param(p, "text") { t.as_bytes().to_vec() @@ -57,15 +66,27 @@ pub(super) fn from_csv(s: &mut Session, p: &Json) -> Result { if text.trim_start().starts_with('[') { let arr: Vec = serde_json::from_str(&text).map_err(|e| bad("data.getData", e.to_string()))?; let mut keys: Vec = Vec::new(); + let mut seen: std::collections::HashSet<&str> = std::collections::HashSet::new(); for o in &arr { if let Some(m) = o.as_object() { for k in m.keys() { - if !keys.contains(k) { + if !seen.contains(k.as_str()) { + // Stop collecting at the column limit. + if keys.len() >= gridcraft_core::MAX_COLS as usize { + return Err(EngineError::Other("The data has more fields than a sheet has columns.".into())); + } + seen.insert(k.as_str()); keys.push(k.clone()); } } } } + if arr.len() as u64 + 1 > u64::from(gridcraft_core::MAX_ROWS) { + return Err(EngineError::Other("The data has more rows than a sheet has.".into())); + } + if (arr.len() as u64 + 1).saturating_mul(keys.len().max(1) as u64) > MAX_IMPORT_CELLS { + return Err(too_many_cells()); + } let mut out = vec![keys.iter().map(|k| json!(k)).collect::>()]; for o in &arr { out.push(keys.iter().map(|k| o.get(k).cloned().unwrap_or(Json::Null)).collect()); @@ -75,10 +96,17 @@ 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 { + return Err(too_many_cells()); + } (u.start.row..=u.end.row) .map(|r| { (u.start.col..=u.end.col) @@ -129,3 +157,35 @@ pub(super) fn queries(s: &mut Session, _: &Json) -> Result { .collect(), )) } + +#[cfg(test)] +mod tests { + use serde_json::json; + + use crate::Session; + + fn s() -> Session { + let mut s = Session::new(); + s.new_workbook(); + s + } + + #[test] + fn sparse_imports_are_refused_before_their_rows_are_built() { + let mut s = s(); + let sheets = s.doc().unwrap().wb.sheets.len(); + // 5,000 records with one field each: 5,001 × 5,000 slots once every record gets every field. + let records: Vec<_> = (0..5_000).map(|i| json!({ format!("k{i}"): i })).collect(); + let text = serde_json::to_string(&records).unwrap(); + let err = s.execute("data.getData", json!({"text": text})).expect_err("too many cells"); + assert!(err.to_string().contains("too large"), "{err}"); + // A CSV whose last line reaches 16,001 columns, 1,000 lines down. + let text = format!("1{}{}x", "\n".repeat(999), ",".repeat(16_000)); + let err = s.execute("data.fromTextCsv", json!({"text": text})).expect_err("too many cells"); + assert!(err.to_string().contains("too large"), "{err}"); + assert_eq!(s.doc().unwrap().wb.sheets.len(), sheets); + // Small imports still work. + let out = s.execute("data.getData", json!({"text": r#"[{"a": 1}, {"b": 2}]"#})).unwrap(); + assert_eq!((out["rows"].as_u64(), out["cols"].as_u64()), (Some(3), Some(2)), "{out}"); + } +} diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 08fc9119f..ed5e33945 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1350,6 +1350,18 @@ fn scripts_leave_file_access_working_after_they_run() { let _ = std::fs::remove_dir_all(&dir); } +#[test] +fn get_data_refuses_json_with_too_many_fields_or_rows_before_building_rows() { + let mut s = s(); + let wide = format!("[{}]", (0..=gridcraft_core::MAX_COLS).map(|i| format!("{{\"k{i}\":1}}")).collect::>().join(",")); + let err = s.execute("data.fromTextCsv", json!({"text": wide})).unwrap_err().to_string(); + assert!(err.contains("more fields"), "{err}"); + // MAX_ROWS objects plus the header row are one too many. + let tall = format!("[{}]", vec!["{}"; gridcraft_core::MAX_ROWS as usize].join(",")); + let err = s.execute("data.fromTextCsv", json!({"text": tall})).unwrap_err().to_string(); + assert!(err.contains("more rows"), "{err}"); +} + #[test] fn scripts_cannot_reach_outside_the_workbook_even_when_trusted() { let dir = std::env::temp_dir().join(format!("gridcraft-script-deny-{}", std::process::id())); 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 773e2895f..67694e72d 100644 --- a/crates/xlsx/src/lib.rs +++ b/crates/xlsx/src/lib.rs @@ -231,11 +231,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()); +}