Variables: dataset CSV keeps line breaks in quoted cells and reads a file only up to its cap (#1929)

crates/engine/src/cmd/variablecmds.rs

@@ -13,7 +13,7 @@ use serde_json::{Value, json};
 use vectorcraft_doc::{DataSet, DataValue, NodeId, NodeKind, Variable, VariableKind, Variables};
 
 use super::edit::selected_roots;
-use super::fileio::{absolute_path, read_file, write_file};
+use super::fileio::{absolute_path, read_file_head, write_file};
 use super::textedit::set_plain_text;
 use super::*;
 
@@ -502,7 +502,8 @@ fn dataset_select(s: &mut Session, p: &Value) -> Result<Value> {
 /// Bytes of a datasets CSV from `path`, `data`, `dataBase64` or `csv`.
 fn datasets_csv_bytes(p: &Value, cmd: &str) -> Result<Vec<u8>> {
     let bytes = match (str_param(p, "path"), str_param(p, "data"), str_param(p, "dataBase64"), str_param(p, "csv")) {
-        (Some(path), ..) => read_file(&absolute_path(path))?,
+        // One byte past the cap is enough to refuse the file below.
+        (Some(path), ..) => read_file_head(&absolute_path(path), MAX_CSV_BYTES + 1)?,
         (None, Some(text), ..) => text.as_bytes().to_vec(),
         (None, None, Some(b64), _) => vectorcraft_format::base64_decode(b64).ok_or_else(|| bad(cmd, "bad dataBase64"))?,
         (None, None, None, Some(csv)) => csv.as_bytes().to_vec(),
@@ -514,55 +515,53 @@ fn datasets_csv_bytes(p: &Value, cmd: &str) -> Result<Vec<u8>> {
     Ok(bytes)
 }
 
-/// Split one CSV line into cells (commas; `"` quotes, doubled inside).
-fn split_csv_line(line: &str) -> std::result::Result<Vec<String>, String> {
-    let mut cells = Vec::new();
-    let mut cur = String::new();
-    let mut chars = line.chars().peekable();
-    let mut in_quotes = false;
-    while let Some(c) = chars.next() {
-        match c {
-            '"' if in_quotes => match chars.peek() {
-                Some('"') => {
-                    cur.push('"');
-                    chars.next();
-                }
-                _ => in_quotes = false,
-            },
-            '"' if !in_quotes && cur.is_empty() => in_quotes = true,
-            ',' if !in_quotes => {
-                cells.push(std::mem::take(&mut cur));
-            }
-            _ => cur.push(c),
-        }
-    }
-    if in_quotes {
-        return Err("unclosed quote".into());
-    }
-    cells.push(cur);
-    if cells.len() > MAX_CSV_COLS {
-        return Err(format!("more than {MAX_CSV_COLS} columns"));
-    }
-    Ok(cells)
-}
-
-/// Parse a datasets CSV into rows of cells (no header interpretation yet).
+/// Parse a datasets CSV into rows of cells (no header interpretation yet): commas between cells,
+/// `"` quotes a cell (doubled inside), which may then hold commas and line breaks, as
+/// [`csv_escape`] writes them. Blank lines are skipped.
 fn parse_datasets_csv(text: &str, cmd: &str) -> Result<Vec<Vec<String>>> {
     let text = text.strip_prefix('\u{feff}').unwrap_or(text);
-    let mut rows = Vec::new();
-    for (i, raw) in text.lines().enumerate() {
-        let line = raw.trim_end_matches('\r');
-        if line.trim().is_empty() {
-            continue;
+    let (mut rows, mut row, mut cur) = (Vec::new(), Vec::<String>::new(), String::new());
+    let (mut in_quotes, mut line) = (false, 1usize);
+    let mut chars = text.chars().peekable();
+    loop {
+        let c = chars.next();
+        match c {
+            Some('"') if in_quotes => {
+                if chars.peek() == Some(&'"') {
+                    cur.push('"');
+                    chars.next();
+                } else {
+                    in_quotes = false;
+                }
+            }
+            Some('"') if cur.is_empty() => in_quotes = true,
+            Some(',') if !in_quotes => row.push(std::mem::take(&mut cur)),
+            Some('\r') if !in_quotes => {}
+            Some('\n') | None if !in_quotes => {
+                row.push(std::mem::take(&mut cur));
+                if row.iter().any(|c| !c.trim().is_empty()) {
+                    rows.push(std::mem::take(&mut row));
+                    // +1 for the header.
+                    if rows.len() > MAX_CSV_ROWS + 1 {
+                        return Err(bad(cmd, format!("more than {MAX_CSV_ROWS} data rows")));
+                    }
+                }
+                row.clear();
+                if c.is_none() {
+                    return Ok(rows);
+                }
+                line += 1;
+            }
+            None => return Err(bad(cmd, format!("line {line}: unclosed quote"))),
+            Some(c) => {
+                line += usize::from(c == '\n');
+                cur.push(c);
+            }
         }
-        let cells = split_csv_line(line).map_err(|e| bad(cmd, format!("line {}: {e}", i + 1)))?;
-        rows.push(cells);
-        if rows.len() > MAX_CSV_ROWS + 1 {
-            // +1 for the header.
-            return Err(bad(cmd, format!("more than {MAX_CSV_ROWS} data rows")));
+        if row.len() > MAX_CSV_COLS {
+            return Err(bad(cmd, format!("line {line}: more than {MAX_CSV_COLS} columns")));
         }
     }
-    Ok(rows)
 }
 
 fn csv_escape(s: &str) -> String {

crates/engine/src/tests_variables.rs

@@ -878,6 +878,24 @@ fn image_datasets_swap_pixels_and_keep_bounds() {
     assert!(s.execute("variable.makeImageDynamic", &json!({})).is_err(), "type is not an image");
 }
 
+/// A text value with commas, quotes and line breaks comes back from the exported CSV as it went out.
+#[test]
+fn datasets_csv_keeps_line_breaks_in_quoted_cells() {
+    let mut s = session();
+    let t = text(&mut s, "One");
+    s.execute("variable.define", &json!({"name": "Name", "kind": "text"})).unwrap();
+    s.execute("variable.bind", &json!({"variable": "Name", "ids": [t.0]})).unwrap();
+    let value = "Line one, \"quoted\"\nLine two";
+    s.execute("dataset.new", &json!({"name": "A", "values": {"Name": value}})).unwrap();
+    let csv = s.execute("dataset.export", &json!({})).unwrap()["csv"].as_str().unwrap().to_string();
+    s.execute("dataset.delete", &json!({"names": ["A"]})).unwrap();
+    s.execute("dataset.import", &json!({"csv": csv})).unwrap();
+    let sets = &s.doc().unwrap().doc.variables.datasets;
+    assert_eq!(sets.len(), 1);
+    assert_eq!(sets[0].values.get("Name"), Some(&vectorcraft_doc::DataValue::Text(value.into())));
+    assert!(s.execute("dataset.import", &json!({"csv": "name,Name\nX,\"open"})).is_err(), "unclosed quote");
+}
+
 /// Import / export datasets as CSV: header `name` then variables; round-trip keeps values.
 #[test]
 fn datasets_csv_round_trips_and_rejects_bad_input() {