Variables: Import Data Sets reads UTF-16 CSV files without a byte-order mark

A CSV file that holds NUL bytes and has no byte-order mark was refused.
It is now read as UTF-16 when at least half of its two-byte units hold
a NUL byte, in the byte order that reads a line break. A file with fewer
NUL bytes (UTF-8 or Windows-1252 text with stray NUL bytes, or UTF-16
text made mostly of characters above U+00FF) is still refused, and so is
a file that reads a line break in both byte orders, has an odd length,
isn't valid UTF-16 in that order or holds a NUL character. Refused files
get the same message as before.

crates/engine/src/cmd/variablecmds.rs

@@ -867,7 +867,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Import Data Sets…",
             ["Window", "Variables"],
             None,
-            "{path? | data? | dataBase64? | csv?} replace every dataset with the rows of a CSV, comma- or tab-separated, in UTF-16 with a byte-order mark, UTF-8 or else Windows-1252 (header: one variable name per column, whatever its case; one row per dataset, named Data Set 1, 2… in file order). The document's variables and their bindings stay. 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. An import that leaves the variables and data sets as they were adds no undo step → {imported, names, defined, deletedDataSets}",
+            "{path? | data? | dataBase64? | csv?} replace every dataset with the rows of a CSV, comma- or tab-separated, in UTF-16 (with a byte-order mark, or without one when at least half of its two-byte units hold a NUL byte and only one byte order reads a line break; any other file with NUL bytes is refused), UTF-8 or else Windows-1252 (header: one variable name per column, whatever its case; one row per dataset, named Data Set 1, 2… in file order). The document's variables and their bindings stay. 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. An import that leaves the variables and data sets as they were adds no undo step → {imported, names, defined, deletedDataSets}",
             has_doc,
             dataset_import
         ),

crates/engine/src/cmd/variablecmds/csv.rs

@@ -32,20 +32,17 @@ fn datasets_csv_bytes(p: &Value, cmd: &str) -> Result<(Vec<u8>, bool)> {
 }
 
 /// 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).
+/// byte-order mark, else UTF-16 without one when the file holds NUL bytes (see
+/// [`utf16_without_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 with NUL
+/// bytes and no byte-order mark must read as UTF-16 by [`utf16_without_mark`]'s rule.
 fn datasets_csv_text(bytes: &[u8], cmd: &str) -> Result<String> {
-    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::<std::result::Result<String, _>>();
-        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")),
+        [0xFF, 0xFE, rest @ ..] => return utf16(rest, true).ok_or_else(|| bad(cmd, "CSV is not UTF-16")),
+        [0xFE, 0xFF, rest @ ..] => return utf16(rest, false).ok_or_else(|| bad(cmd, "CSV is not UTF-16")),
+        _ if bytes.contains(&0) => {
+            return utf16_without_mark(bytes).ok_or_else(|| 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) {
@@ -55,6 +52,34 @@ fn datasets_csv_text(bytes: &[u8], cmd: &str) -> Result<String> {
     }
 }
 
+/// `bytes` as UTF-16, little-endian when `le` is set, or `None` when they aren't valid UTF-16.
+fn utf16(bytes: &[u8], le: bool) -> Option<String> {
+    let (units, odd) = bytes.as_chunks::<2>();
+    let units = units.iter().map(|&c| if le { u16::from_le_bytes(c) } else { u16::from_be_bytes(c) });
+    char::decode_utf16(units).collect::<std::result::Result<String, _>>().ok().filter(|_| odd.is_empty())
+}
+
+/// `bytes`, which hold NUL bytes and no byte-order mark, as UTF-16, or `None`. At least half of
+/// the two-byte units must hold a NUL byte, as characters from U+0001 to U+00FF do; a file with
+/// fewer, such as UTF-8 or Windows-1252 text with stray NUL bytes, is refused. The byte order is
+/// the one that reads a line break (`\n` or `\r`). The other order reads one only from U+0A00 or
+/// U+0D00, so a file that reads one in both orders is refused. A file that reads none in either
+/// order has no data rows, and is read little-endian so that the import reports that. The text
+/// must be valid UTF-16 in that order and hold no NUL character.
+fn utf16_without_mark(bytes: &[u8]) -> Option<String> {
+    let (units, odd) = bytes.as_chunks::<2>();
+    if !odd.is_empty() || units.iter().filter(|c| c.contains(&0)).count() * 2 < units.len() {
+        return None;
+    }
+    let is_line_break = |u: u16| matches!(u, 0x0A | 0x0D);
+    let lines_le = units.iter().any(|&c| is_line_break(u16::from_le_bytes(c)));
+    let lines_be = units.iter().any(|&c| is_line_break(u16::from_be_bytes(c)));
+    if lines_le && lines_be {
+        return None;
+    }
+    utf16(bytes, !lines_be).filter(|text| !text.contains('\0'))
+}
+
 /// A row of a datasets CSV: the file line it starts on, its cells, and for each cell whether it was quoted.
 type CsvRow = (usize, Vec<String>, Vec<bool>);
 

crates/engine/src/tests/variables.rs

@@ -2137,8 +2137,9 @@ fn datasets_csv_reads_windows_1252_files() {
 }
 
 /// 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.
+/// one that isn't valid UTF-16 is refused. A file with NUL bytes and no byte-order mark that isn't
+/// UTF-16 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();
@@ -2160,7 +2161,7 @@ fn datasets_csv_reads_utf16_files() {
     assert!(e.contains("CSV is not UTF-16"), "{e}");
     let e = import(&mut s, &[b"\xff\xfe".as_slice(), "City\nAB,".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"", "City\nx\n", true)).unwrap_err().to_string();
+    let e = import(&mut s, b"City\nx\0\n").unwrap_err().to_string();
     assert!(e.contains("CSV holds NUL bytes"), "{e}");
     assert_eq!(value(&s, "Data Set 1"), text_value("José"), "a refused file changes nothing");
     s.execute("dataset.set", &json!({"name": "Data Set 1", "values": {"City": "a\u{0}b"}})).unwrap();
@@ -2170,6 +2171,63 @@ fn datasets_csv_reads_utf16_files() {
     assert_eq!(value(&s, "Data Set 1"), text_value("a\u{0}b"));
 }
 
+/// A CSV in UTF-16 without a byte-order mark, with a NUL byte in at least half of its units, is
+/// read in the byte order that reads a line break: in either order, with characters whose bytes
+/// read as a line break, a quote or NUL, and when its bytes are also valid UTF-8. Files with fewer
+/// NUL bytes, that read a line break in both orders, that aren't valid UTF-16 in it, that hold a
+/// NUL character or that have an odd length are refused.
+#[test]
+fn datasets_csv_reads_utf16_files_without_a_byte_order_mark() {
+    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 = |text: &str, le: bool| -> Vec<u8> { text.encode_utf16().flat_map(|u| if le { u.to_le_bytes() } else { u.to_be_bytes() }).collect() };
+    import(&mut s, &utf16("City\r\nParis\r\n", true)).unwrap();
+    assert_eq!(value(&s, "Data Set 1"), text_value("Paris"));
+    import(&mut s, &utf16("City\nZoë 😀\n", false)).unwrap();
+    assert_eq!(value(&s, "Data Set 1"), text_value("Zoë 😀"));
+    // U+4E00's low byte is NUL, U+010A holds a `\n` byte and U+2200 a `"` byte; all three files
+    // are valid UTF-8 too.
+    import(&mut s, &utf16("City\n一\n", true)).unwrap();
+    assert_eq!(value(&s, "Data Set 1"), text_value("一"));
+    import(&mut s, &utf16("City\nĊ\n", true)).unwrap();
+    assert_eq!(value(&s, "Data Set 1"), text_value("Ċ"));
+    import(&mut s, &utf16("City\n∀∀∀\n", true)).unwrap();
+    assert_eq!(value(&s, "Data Set 1"), text_value("∀∀∀"));
+    // A file with no line break has no data rows in either byte order.
+    for bytes in [utf16("City", true), utf16("City", false), b"\0aa\0".to_vec()] {
+        let e = import(&mut s, &bytes).unwrap_err().to_string();
+        assert!(e.contains("CSV has a header but no data rows"), "{e}");
+    }
+    import(&mut s, &utf16("City\tName\n一二三丁目\tKai\n", true)).unwrap();
+    assert_eq!(value(&s, "Data Set 1"), text_value("一二三丁目"));
+    // Most units hold their NUL byte where a big-endian file would (U+4E00); the line breaks
+    // give the byte order.
+    import(&mut s, &utf16("City\n一一一一一一一一一一二\n", true)).unwrap();
+    assert_eq!(value(&s, "Data Set 1"), text_value("一一一一一一一一一一二"));
+    // Windows-1252 and UTF-8 text with stray NUL bytes (the first with `\n\0` units), UTF-16 with
+    // fewer than half of its units holding a NUL byte, and, last, two files that read a line break
+    // in both byte orders (U+0D00 reads as `\r` and U+0A00 as `\n` big-endian).
+    let refused: [&[u8]; 9] = [
+        b"Caf\xe9\n\0Lyon\n\0",
+        b"Cit\0\n",
+        b"City\nx\0\n",
+        &utf16("City\n丁丁丁丁丁丁丁丁丁丁\n", true),
+        &utf16("City\na\u{0}\n", true),
+        &[utf16("City\n", true), vec![0x00, 0xD8], utf16("\n", true)].concat(),
+        &[utf16("City\n", false), vec![0xDC, 0x00], utf16("\n", false)].concat(),
+        &utf16("City\n\u{0D00}\u{0D00}\u{0D00}\n", true),
+        &utf16("City\u{0A00}\nParis\n", true),
+    ];
+    for bytes in refused {
+        let e = import(&mut s, bytes).unwrap_err().to_string();
+        assert!(e.contains("CSV holds NUL bytes"), "{bytes:?}: {e}");
+    }
+    assert_eq!(value(&s, "Data Set 1"), text_value("一一一一一一一一一一二"), "a refused file changes nothing");
+}
+
 /// 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])]);