Merge PR #1102: CSV export: check the size cap before each field, not after the row (#1023)

crates/xlsx/src/csv.rs

@@ -193,9 +193,9 @@ pub fn read_csv(bytes: &[u8], opts: &CsvOptions) -> Result<Workbook, IoError> {
 ///
 /// 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<Vec<u8>, 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);

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);