From a091fd9b509e317308a12c84bce0a50acd3af61f Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:04:20 +0200 Subject: [PATCH] CSV export: check the size cap before each field, not after the row (#1023) --- crates/xlsx/src/csv.rs | 22 +++++++++++++++++----- crates/xlsx/src/tests/csv_tests.rs | 19 +++++++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/crates/xlsx/src/csv.rs b/crates/xlsx/src/csv.rs index 47486bfa2..485de7a37 100644 --- a/crates/xlsx/src/csv.rs +++ b/crates/xlsx/src/csv.rs @@ -193,9 +193,9 @@ pub fn read_csv(bytes: &[u8], opts: &CsvOptions) -> Result { /// /// The output is capped at [`MAX_CSV_BYTES`]: two cells far apart make the used range, and so the /// file, as big as the whole rectangle between them (`A1` and `XFD1048576` are 16 GiB of commas). -/// An export that would pass the cap is refused before anything is allocated, or as soon as the -/// text written so far passes it, with [`IoError::TooLarge`]; accepted exports keep every field in -/// its place. +/// An export that would pass the cap is refused before anything is allocated, or at the first +/// field or line end that takes it past the cap (a field is measured before it is written), with +/// [`IoError::TooLarge`]; accepted exports keep every field in its place. pub fn write_csv(sheet: &Sheet, delimiter: u8, cell_text: impl FnMut(CellRef) -> String) -> Result, IoError> { write_csv_limited(sheet, delimiter, MAX_CSV_BYTES, cell_text) } @@ -221,9 +221,21 @@ pub fn write_csv_limited(sheet: &Sheet, delimiter: u8, limit: usize, mut cell_te out.push(delim); } let t = cell_text(CellRef::new(row, col)); - if t.contains(delim) || t.contains('"') || t.contains('\n') || t.contains('\r') { + let quoted = t.contains(delim) || t.contains('"') || t.contains('\n') || t.contains('\r'); + // The exact size of the field as written (the delimiter was pushed above), so a row + // never grows past the cap unseen and no doubled-quote copy of the text is built. + let field_len = if quoted { t.len() + t.matches('"').count() + 2 } else { t.len() }; + if out.len().saturating_add(field_len) > limit { + return Err(too_large(format!("more than {} MiB (stopped at row {})", limit >> 20, row + 1))); + } + if quoted { out.push('"'); - out.push_str(&t.replace('"', "\"\"")); + for ch in t.chars() { + if ch == '"' { + out.push('"'); + } + out.push(ch); + } out.push('"'); } else { out.push_str(&t); diff --git a/crates/xlsx/src/tests/csv_tests.rs b/crates/xlsx/src/tests/csv_tests.rs index cc52072e7..3716a162d 100644 --- a/crates/xlsx/src/tests/csv_tests.rs +++ b/crates/xlsx/src/tests/csv_tests.rs @@ -186,6 +186,25 @@ fn the_cap_counts_the_bytes_the_cells_hold_and_accepted_exports_keep_every_posit let err = write_csv_limited(&long, b',', 1000, text_of(&long)).unwrap_err(); assert!(err.to_string().contains("stopped at row"), "{err}"); assert!(write_csv_limited(&long, b',', 50 * 102, text_of(&long)).is_ok()); + // Within one row the cap is checked before each field is written: a wide row of long and + // quote-heavy fields is refused at the field that crosses it, without asking for the rest. + let mut wide = Sheet::new("W"); + for c in 0..10u32 { + wide.set_value(CellRef::new(0, c), Value::text("\"".repeat(100))); + } + let mut asked = 0; + let err = write_csv_limited(&wide, b',', 500, |c| { + asked += 1; + text_of(&wide)(c) + }) + .unwrap_err(); + assert!(err.to_string().contains("stopped at row 1"), "{err}"); + assert_eq!(asked, 3, "each field is 202 bytes; the third passes 500"); + // Exact boundary: a quoted field of 202 bytes plus CRLF fits in 204 and not in 203. + let mut one = Sheet::new("O"); + one.set_value(CellRef::new(0, 0), Value::text("\"".repeat(100))); + assert_eq!(write_csv_limited(&one, b',', 204, text_of(&one)).unwrap().len(), 204); + assert!(write_csv_limited(&one, b',', 203, text_of(&one)).is_err()); // A tab is one byte, like a comma. let tabs = write_csv_limited(&s, b'\t', 14, text_of(&s)).unwrap(); assert_eq!(tabs.len(), 14);