From 899a4c2bcbbff6db603df105c78b706ad76fe586 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Mon, 2 Nov 2026 11:20:11 +0700 Subject: [PATCH] Variables: Import Data Sets reads CSV files that are not UTF-8 as Windows-1252 Squashed from #5116 onto main. --- crates/engine/src/cmd/place.rs | 2 +- crates/engine/src/cmd/place/text.rs | 2 +- crates/engine/src/cmd/variablecmds.rs | 2 +- crates/engine/src/cmd/variablecmds/csv.rs | 49 +++++++++++++++----- crates/engine/src/tests/variables.rs | 56 +++++++++++++++++++++++ docs/mcp.md | 3 ++ 6 files changed, 100 insertions(+), 14 deletions(-) diff --git a/crates/engine/src/cmd/place.rs b/crates/engine/src/cmd/place.rs index 48d43d8eb..cc9d9def0 100644 --- a/crates/engine/src/cmd/place.rs +++ b/crates/engine/src/cmd/place.rs @@ -36,7 +36,7 @@ mod doc; pub(crate) mod document; pub(crate) mod docx; mod rtf; -mod text; +pub(super) mod text; use std::sync::Arc; diff --git a/crates/engine/src/cmd/place/text.rs b/crates/engine/src/cmd/place/text.rs index 15c5f2575..ab5562b5f 100644 --- a/crates/engine/src/cmd/place/text.rs +++ b/crates/engine/src/cmd/place/text.rs @@ -96,7 +96,7 @@ fn decode(bytes: &[u8], o: TextOptions) -> String { } /// Byte `b` of Windows-1252. -pub(super) fn windows_1252(b: u8) -> char { +pub(in crate::cmd) fn windows_1252(b: u8) -> char { const HIGH: [char; 32] = [ '€', '\u{81}', '‚', 'ƒ', '„', '…', '†', '‡', 'ˆ', '‰', 'Š', '‹', 'Œ', '\u{8D}', 'Ž', '\u{8F}', // '\u{90}', '‘', '’', '“', '”', '•', '–', '—', '˜', '™', 'š', '›', 'œ', '\u{9D}', 'ž', 'Ÿ', diff --git a/crates/engine/src/cmd/variablecmds.rs b/crates/engine/src/cmd/variablecmds.rs index 96b3c2a7a..f31477a85 100644 --- a/crates/engine/src/cmd/variablecmds.rs +++ b/crates/engine/src/cmd/variablecmds.rs @@ -799,7 +799,7 @@ pub fn specs() -> Vec { "Import Data Sets…", ["Window", "Variables"], None, - "{path? | data? | dataBase64? | csv?} load datasets from CSV, comma- or tab-separated (header: name, then variable names; one row per dataset; without the name column every header is a variable and the rows take the first free names Data Set 1, 2…). A header that names no variable defines one, unbound: `#Name` defines a visibility variable, `@Name` defines an image variable, and a header without a prefix defines a text variable. A prefixed header whose name is a variable of another kind, and a `%` header that isn't a variable's exact name (graph data files), are refused, and so are two headers that name one variable and a header that would define a variable while the variables are locked. Rows whose name already exists are replaced → {imported, updated, names, defined}", + "{path? | data? | dataBase64? | csv?} load datasets from CSV, comma- or tab-separated, in UTF-16 with a byte-order mark, UTF-8 or else Windows-1252 (header: name, then variable names; one row per dataset; without the name column every header is a variable and the rows take the first free names Data Set 1, 2…). A header that names no variable defines one, unbound: `#Name` defines a visibility variable, `@Name` defines an image variable, and a header without a prefix defines a text variable. A prefixed header whose name is a variable of another kind, and a `%` header that isn't a variable's exact name (graph data files), are refused, and so are two headers that name one variable and a header that would define a variable while the variables are locked. Rows whose name already exists are replaced → {imported, updated, names, defined}", has_doc, dataset_import ), diff --git a/crates/engine/src/cmd/variablecmds/csv.rs b/crates/engine/src/cmd/variablecmds/csv.rs index 507c153f8..fe405500b 100644 --- a/crates/engine/src/cmd/variablecmds/csv.rs +++ b/crates/engine/src/cmd/variablecmds/csv.rs @@ -2,6 +2,7 @@ use super::{ BTreeMap, BTreeSet, DataSet, DataValue, EngineError, MAX_NAME, Result, Session, Value, Variable, VariableKind, Variables, absolute_path, bad, json, read_file_head, str_param, variables_unlocked, write_file, }; +use crate::cmd::place::text::windows_1252; /// Cap on a graph variable's CSV (MCP and files are untrusted). pub(super) const MAX_GRAPH_CSV: usize = 256 * 1024; @@ -13,20 +14,45 @@ const MAX_CSV_ROWS: usize = 1_000; const MAX_CSV_COLS: usize = 256; -/// Bytes of a datasets CSV from `path`, `data`, `dataBase64` or `csv`. -fn datasets_csv_bytes(p: &Value, cmd: &str) -> Result> { - let bytes = match (str_param(p, "path"), str_param(p, "data"), str_param(p, "dataBase64"), str_param(p, "csv")) { +/// Bytes of a datasets CSV from `path`, `data`, `dataBase64` or `csv`, and whether they came +/// as text (`data` or `csv`). +fn datasets_csv_bytes(p: &Value, cmd: &str) -> Result<(Vec, bool)> { + let (bytes, text) = match (str_param(p, "path"), str_param(p, "data"), str_param(p, "dataBase64"), str_param(p, "csv")) { // 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(), + (Some(path), ..) => (read_file_head(&absolute_path(path), MAX_CSV_BYTES + 1)?, false), + (None, Some(text), ..) => (text.as_bytes().to_vec(), true), + (None, None, Some(b64), _) => (vectorcraft_format::base64_decode(b64).ok_or_else(|| bad(cmd, "bad dataBase64"))?, false), + (None, None, None, Some(csv)) => (csv.as_bytes().to_vec(), true), _ => return Err(bad(cmd, "give `path`, `data`, `dataBase64` or `csv`")), }; if bytes.len() > MAX_CSV_BYTES { return Err(bad(cmd, format!("CSV is larger than {MAX_CSV_BYTES} bytes"))); } - Ok(bytes) + Ok((bytes, text)) +} + +/// The text of a datasets CSV given as a file or as `dataBase64`: UTF-16 after a UTF-16 +/// byte-order mark, else UTF-8, with or without a byte-order mark, else Windows-1252. A file +/// that starts with a byte-order mark must be valid in its encoding, and a file without one must +/// not hold NUL bytes (UTF-16 without a byte-order mark). +fn datasets_csv_text(bytes: &[u8], cmd: &str) -> Result { + let utf16 = |rest: &[u8], le: bool| { + let (units, odd) = rest.as_chunks::<2>(); + let units = units.iter().map(|&c| if le { u16::from_le_bytes(c) } else { u16::from_be_bytes(c) }); + let text = char::decode_utf16(units).collect::>(); + text.ok().filter(|_| odd.is_empty()).ok_or_else(|| bad(cmd, "CSV is not UTF-16")) + }; + match bytes { + [0xFF, 0xFE, rest @ ..] => return utf16(rest, true), + [0xFE, 0xFF, rest @ ..] => return utf16(rest, false), + _ if bytes.contains(&0) => return Err(bad(cmd, "CSV holds NUL bytes: save it as UTF-8, or as UTF-16 with a byte-order mark")), + _ => {} + } + match std::str::from_utf8(bytes) { + Ok(text) => Ok(text.to_string()), + Err(_) if bytes.starts_with(b"\xEF\xBB\xBF") => Err(bad(cmd, "CSV is not UTF-8")), + Err(_) => Ok(bytes.iter().map(|&b| windows_1252(b)).collect()), + } } /// A row of a datasets CSV: the file line it starts on, its cells, and for each cell whether it was quoted. @@ -310,9 +336,10 @@ fn header_variable( /// name are replaced; new names are appended. One undo step. pub(super) fn dataset_import(s: &mut Session, p: &Value) -> Result { const C: &str = "dataset.import"; - let bytes = datasets_csv_bytes(p, C)?; - let text = std::str::from_utf8(&bytes).map_err(|_| bad(C, "CSV is not UTF-8"))?; - let rows = parse_datasets_csv(text, C)?; + let (bytes, given_as_text) = datasets_csv_bytes(p, C)?; + // `data` and `csv` are already text, NUL characters included (`dataset.export` writes them). + let text = if given_as_text { String::from_utf8(bytes).map_err(|_| bad(C, "CSV is not UTF-8"))? } else { datasets_csv_text(&bytes, C)? }; + let rows = parse_datasets_csv(&text, C)?; let Some((_, header, _)) = rows.first() else { return Err(bad(C, "CSV is empty")); }; diff --git a/crates/engine/src/tests/variables.rs b/crates/engine/src/tests/variables.rs index c54350cc9..a7dcd9dec 100644 --- a/crates/engine/src/tests/variables.rs +++ b/crates/engine/src/tests/variables.rs @@ -1729,6 +1729,62 @@ fn datasets_csv_round_trips_and_rejects_bad_input() { s.execute("edit.undo", &json!({})).unwrap(); } +/// A CSV that isn't UTF-8 is read as Windows-1252. UTF-8 files, with or without a byte-order mark, +/// read as before, and a file that starts with the UTF-8 byte-order mark but isn't UTF-8 is refused. +#[test] +fn datasets_csv_reads_windows_1252_files() { + let mut s = session(); + s.execute("variable.define", &json!({"name": "City", "kind": "text"})).unwrap(); + let import = |s: &mut Session, bytes: &[u8]| s.execute("dataset.import", &json!({"dataBase64": vectorcraft_format::base64_encode(bytes)})); + let value = |s: &Session, name: &str| s.doc().unwrap().doc.variables.dataset(name).and_then(|d| d.values.get("City")).cloned(); + let text_value = |v: &str| Some(vectorcraft_doc::DataValue::Text(v.into())); + let imp = import(&mut s, b"name,City\r\nA,Jos\xe9\r\nB,\x80 5 \x96 \x93Caf\xe9\x94\r\n").unwrap(); + assert_eq!(imp["names"], json!(["A", "B"])); + assert_eq!(value(&s, "A"), text_value("José")); + assert_eq!(value(&s, "B"), text_value("€ 5 – “Café”")); + import(&mut s, "name,City\nC,José\n".as_bytes()).unwrap(); + assert_eq!(value(&s, "C"), text_value("José")); + import(&mut s, "\u{feff}name,City\nD,Zoë\n".as_bytes()).unwrap(); + assert_eq!(value(&s, "D"), text_value("Zoë")); + let e = import(&mut s, b"\xef\xbb\xbfname,City\nE,Jos\xe9\n").unwrap_err().to_string(); + assert!(e.contains("CSV is not UTF-8"), "{e}"); + assert_eq!(value(&s, "E"), None); +} + +/// A CSV that starts with a UTF-16 byte-order mark is read as UTF-16, and +/// one that isn't valid UTF-16 is refused. A file with NUL bytes and no byte-order mark is refused. +/// A text value holding NUL imports back from the `csv` that `dataset.export` returns. +#[test] +fn datasets_csv_reads_utf16_files() { + let mut s = session(); + s.execute("variable.define", &json!({"name": "City", "kind": "text"})).unwrap(); + let import = |s: &mut Session, bytes: &[u8]| s.execute("dataset.import", &json!({"dataBase64": vectorcraft_format::base64_encode(bytes)})); + let value = |s: &Session, name: &str| s.doc().unwrap().doc.variables.dataset(name).and_then(|d| d.values.get("City")).cloned(); + let text_value = |v: &str| Some(vectorcraft_doc::DataValue::Text(v.into())); + let utf16 = |bom: &[u8], text: &str, le: bool| -> Vec { + let units = text.encode_utf16().flat_map(|u| if le { u.to_le_bytes() } else { u.to_be_bytes() }); + bom.iter().copied().chain(units).collect() + }; + import(&mut s, &utf16(b"\xff\xfe", "name\tCity\r\nA\tZoë 😀\r\n", true)).unwrap(); + assert_eq!(value(&s, "A"), text_value("Zoë 😀")); + import(&mut s, &utf16(b"\xfe\xff", "name,City\nB,José\n", false)).unwrap(); + assert_eq!(value(&s, "B"), text_value("José")); + let mut odd = utf16(b"\xff\xfe", "name,City\nC,x\n", true); + odd.push(b'y'); + let e = import(&mut s, &odd).unwrap_err().to_string(); + assert!(e.contains("CSV is not UTF-16"), "{e}"); + let e = import(&mut s, &[b"\xff\xfe".as_slice(), "name,City\nD,".as_bytes(), b"\x00\xd8"].concat()).unwrap_err().to_string(); + assert!(e.contains("CSV is not UTF-16"), "{e}"); + let e = import(&mut s, &utf16(b"", "name,City\nE,x\n", true)).unwrap_err().to_string(); + assert!(e.contains("CSV holds NUL bytes"), "{e}"); + assert_eq!((value(&s, "C"), value(&s, "D"), value(&s, "E")), (None, None, None)); + s.execute("dataset.new", &json!({"name": "F", "values": {"City": "a\u{0}b"}})).unwrap(); + let csv = s.execute("dataset.export", &json!({})).unwrap()["csv"].as_str().unwrap().to_string(); + s.execute("dataset.delete", &json!({"names": ["F"]})).unwrap(); + s.execute("dataset.import", &json!({"csv": csv})).unwrap(); + assert_eq!(value(&s, "F"), text_value("a\u{0}b")); +} + /// A data set of [`library`]: its name, and each variable's name with its paragraphs. type LibrarySet<'a> = (&'a str, &'a [(&'a str, &'a [&'a str])]); diff --git a/docs/mcp.md b/docs/mcp.md index bc762c434..a69fe660b 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1658,6 +1658,9 @@ already walks Image Trace. Short agent entry points: data files) is refused, and so are two cells that name one variable (`City` twice, or `Shown` and `#Shown`). The reply's `defined` lists the variables the import defined. While the variables are locked, an import whose header would define a variable fails with "variables are locked". + A `dataset.import` file that isn't UTF-8 is read as Windows-1252, unless it starts with the UTF-8 byte-order mark. + A file that starts with a UTF-16 byte-order mark is read as UTF-16, and a file without one that holds NUL bytes is + refused. `csv` and `data` are read as the text they hold. - Variable libraries: `variable.loadLibrary {path|xml|dataBase64}` (the Variables panel menu's Load Variable Library…) replaces the document's variables and data sets with a variable library's in one undo step. A load, like a `dataset.import`, adds no undo step when it leaves the variables and data sets as they were. The library