address review: refuse recipient edits over the cap and report dropped records on import

Every record is padded to every field and the list is kept twice (as read,
and filtered and sorted), so a short CSV with a 1,000-field header and
100,000 one-field rows held 2 x 100 million empty cells, several gigabytes.

A list now keeps at most 4 million cells: wide lists keep fewer records (with
a warning), narrow ones keep the usual 100,000.

crates/engine/src/cmd/mailings.rs

@@ -25,8 +25,8 @@ use merge::{address_map, finish, go_to, merge_fields, step};
 pub use merge::{merge_range, refresh};
 #[cfg(test)]
 use merge::{mergefield_name, placeholder};
-use recipients::recipients;
 pub use recipients::{RECIPIENT_EXTENSIONS, is_spreadsheet_name, parse_csv};
+use recipients::{recipients, recipients_strict};
 
 /// The mail-merge data source and preview state (lives in the session, not the document).
 #[derive(Clone, Debug, Default)]
@@ -79,7 +79,8 @@ pub fn specs() -> Vec<CommandSpec> {
                 // Edited records keep the list's filter and sort (rules on removed fields drop).
                 // Copies, so a list that fails to load leaves the filter and sort as they were.
                 let (filter, sort) = (s.merge.filter.clone(), s.merge.sort.clone());
-                let r = recipients(s, &json!({"rows": v.get("rows"), "fields": v.get("fields")}))?;
+                // An edit that would lose records is refused, not truncated.
+                let r = recipients_strict(s, &json!({"rows": v.get("rows"), "fields": v.get("fields")}))?;
                 s.merge.filter = filter;
                 s.merge.sort = sort;
                 apply_filter(s);

crates/engine/src/cmd/mailings/recipients.rs

@@ -72,6 +72,36 @@ const MAX_RECORDS: usize = 100_000;
 
 const MAX_FIELDS: usize = 1_000;
 
+/// Most cells (records × fields) a recipient list keeps. Every record is padded to every field
+/// and the list is kept twice (as read, and filtered and sorted), so a short file with a wide
+/// header and many one-field rows would otherwise claim gigabytes of empty cells.
+const MAX_CELLS: usize = 4_000_000;
+
+/// Most records a list with `fields` fields keeps: [`MAX_RECORDS`], fewer for a wide list.
+fn max_records(fields: usize) -> usize {
+    MAX_RECORDS.min(MAX_CELLS / fields.max(1))
+}
+
+/// Cut `rows` to what a list with `fields` fields keeps, saying so when it drops any. Returns how
+/// many records it dropped.
+fn cap_records<T>(rows: &mut Vec<T>, fields: usize) -> usize {
+    let keep = max_records(fields);
+    let dropped = rows.len().saturating_sub(keep);
+    if dropped > 0 {
+        log::warn!("the recipient list keeps at most {keep} records with {fields} fields; the rest are left out");
+        rows.truncate(keep);
+    }
+    dropped
+}
+
+/// The error for a list that would lose records: `keep` is what `fields` fields allow.
+fn over_limit(dropped: usize, fields: usize) -> CmdError {
+    CmdError::Params(format!(
+        "the recipient list has {dropped} more records than the {} that fit with {fields} fields; remove records or fields",
+        max_records(fields)
+    ))
+}
+
 pub(super) fn cell(x: &Value) -> String {
     match x {
         Value::Null => String::new(),
@@ -82,7 +112,7 @@ pub(super) fn cell(x: &Value) -> String {
 
 /// A recipient table from `rows` (objects keyed by field, or arrays in `fields` order). Without
 /// `fields`, the columns are the objects' keys.
-fn table_from_rows(fields: Option<&Vec<Value>>, arr: &[Value]) -> Result<(Vec<String>, Vec<Vec<String>>), CmdError> {
+fn table_from_rows(fields: Option<&Vec<Value>>, arr: &[Value]) -> Result<(Vec<String>, Vec<Vec<String>>, usize), CmdError> {
     let headers: Vec<String> = match fields {
         Some(f) => f.iter().take(MAX_FIELDS).map(|h| cell(h).trim().to_string()).collect(),
         None => {
@@ -102,19 +132,30 @@ fn table_from_rows(fields: Option<&Vec<Value>>, arr: &[Value]) -> Result<(Vec<St
     };
     let rows = arr
         .iter()
-        .take(MAX_RECORDS)
+        .take(max_records(headers.len()))
         .filter_map(|r| match r {
             Value::Array(cells) => Some((0..headers.len()).map(|i| cells.get(i).map(cell).unwrap_or_default()).collect()),
             Value::Object(o) => Some(headers.iter().map(|h| o.get(h).map(cell).unwrap_or_default()).collect()),
             _ => None,
         })
         .collect();
-    Ok((headers, rows))
+    let dropped = arr.len().saturating_sub(max_records(headers.len()));
+    Ok((headers, rows, dropped))
 }
 
+/// Loads a recipient list; records past the cap are left out and reported as `dropped`.
 pub(super) fn recipients(s: &mut Session, v: &Value) -> CmdResult {
+    load_recipients(s, v, false)
+}
+
+/// Like [`recipients`], but a list that would lose records is refused and the current list kept.
+pub(super) fn recipients_strict(s: &mut Session, v: &Value) -> CmdResult {
+    load_recipients(s, v, true)
+}
+
+fn load_recipients(s: &mut Session, v: &Value, strict: bool) -> CmdResult {
     let mut sheet = Value::Null;
-    let (headers, rows) = if let Some(arr) = v.get("rows").and_then(Value::as_array) {
+    let (headers, rows, dropped) = if let Some(arr) = v.get("rows").and_then(Value::as_array) {
         table_from_rows(v.get("fields").and_then(Value::as_array), arr)?
     } else {
         let all = if let Some(c) = p::str(v, "csv") {
@@ -180,6 +221,9 @@ pub(super) fn recipients(s: &mut Session, v: &Value) -> CmdResult {
         };
         header_split(all, p::bool(v, "headers").unwrap_or(true))?
     };
+    if strict && dropped > 0 {
+        return Err(over_limit(dropped, headers.len()));
+    }
     s.merge.headers = headers;
     s.merge.source = rows.clone();
     s.merge.rows = rows;
@@ -188,6 +232,9 @@ pub(super) fn recipients(s: &mut Session, v: &Value) -> CmdResult {
     s.merge.record = 0;
     refresh_shown(s);
     let mut r = json!({"fields": s.merge.headers, "records": s.merge.rows.len()});
+    if dropped > 0 {
+        r["dropped"] = json!(dropped);
+    }
     if !sheet.is_null() {
         r["sheet"] = sheet;
     }
@@ -223,8 +270,9 @@ fn read_file(path: &str) -> Result<Vec<u8>, CmdError> {
 
 /// Field names and records from a table's rows: with `headers`, the first row names the fields
 /// (blank names become `Column N`, repeats are numbered); otherwise every row is a record and
-/// the fields are `Column 1`…`Column N`. Records are padded or cut to the fields, and capped.
-fn header_split(mut all: Vec<Vec<String>>, headers: bool) -> Result<(Vec<String>, Vec<Vec<String>>), CmdError> {
+/// the fields are `Column 1`…`Column N`. Records are padded or cut to the fields, and capped
+/// ([`max_records`]).
+fn header_split(mut all: Vec<Vec<String>>, headers: bool) -> Result<(Vec<String>, Vec<Vec<String>>, usize), CmdError> {
     if all.is_empty() {
         return Err(CmdError::Failed("the recipient list is empty".into()));
     }
@@ -235,9 +283,9 @@ fn header_split(mut all: Vec<Vec<String>>, headers: bool) -> Result<(Vec<String>
         let width = all.iter().map(Vec::len).max().unwrap_or(0).min(MAX_FIELDS);
         (1..=width).map(|i| format!("Column {i}")).collect()
     };
-    all.truncate(MAX_RECORDS);
+    let dropped = cap_records(&mut all, fields.len());
     for r in &mut all {
         r.resize(fields.len(), String::new());
     }
-    Ok((fields, all))
+    Ok((fields, all, dropped))
 }

crates/engine/src/cmd/mailings/tests.rs

@@ -695,3 +695,40 @@ fn merge_uses_only_filtered_records_in_sort_order() {
     assert!(s.run("mailings.finish", &json!({})).is_err());
     assert!(s.run("mailings.editRecipients", &json!({})).is_ok());
 }
+
+/// A wide header and many one-field rows: every record is padded to every field (and the list
+/// kept twice), so the cells the list keeps are capped, not just its records and fields.
+#[test]
+fn a_wide_sparse_recipient_list_is_capped_by_cells() {
+    let header = (1..=1_000).map(|i| format!("F{i}")).collect::<Vec<_>>().join(",");
+    let csv = format!("{header}\n{}", "x\n".repeat(20_000));
+    let mut s = Session::new(Document::from_text("Hello"));
+    let r = s.run("mailings.recipients", &json!({"csv": csv})).unwrap();
+    assert_eq!(r["records"], 4_000);
+    let rows: Vec<Value> = (0..20_000).map(|_| json!({})).collect();
+    let fields: Vec<String> = (1..=1_000).map(|i| format!("F{i}")).collect();
+    let r = s.run("mailings.recipients", &json!({"rows": rows, "fields": fields})).unwrap();
+    assert_eq!(r["records"], 4_000);
+    // A narrow list keeps its usual number of records.
+    let r = s.run("mailings.recipients", &json!({"csv": format!("Name\n{}", "Ann\n".repeat(20_000))})).unwrap();
+    assert_eq!(r["records"], 20_000);
+}
+
+/// An import says how many records the cap left out; an edit that would lose records is refused
+/// and the list stays as it was.
+#[test]
+fn recipient_cap_is_reported_and_edits_that_exceed_it_are_refused() {
+    let mut s = Session::new(Document::from_text("Hello"));
+    let fields: Vec<String> = (1..=80).map(|i| format!("F{i}")).collect();
+    let rows: Vec<Value> = (0..50_000).map(|_| json!({})).collect();
+    let r = s.run("mailings.recipients", &json!({"rows": rows, "fields": fields})).unwrap();
+    assert_eq!((r["records"].clone(), r.get("dropped")), (json!(50_000), None));
+    let fields: Vec<String> = (1..=81).map(|i| format!("F{i}")).collect();
+    let rows: Vec<Value> = (0..50_000).map(|_| json!({})).collect();
+    let e = s.run("mailings.editRecipients", &json!({"rows": rows, "fields": fields.clone()}));
+    assert!(matches!(e, Err(CmdError::Params(_))), "{e:?}");
+    assert_eq!((s.merge.headers.len(), s.merge.rows.len()), (80, 50_000));
+    let rows: Vec<Value> = (0..50_000).map(|_| json!({})).collect();
+    let r = s.run("mailings.recipients", &json!({"rows": rows, "fields": fields})).unwrap();
+    assert_eq!((r["records"].clone(), r["dropped"].clone()), (json!(49_382), json!(618)));
+}