diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 30b8ed28..1d055423 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -128,6 +128,11 @@ several lines, and **anything longer than 60 characters is truncated with `…`** so one column cannot set the width of every row. `--format json` returns the values unaltered. +A text value that is not valid UTF-8 (`CAST(x'ff' AS TEXT)`, `char(55296)`) +fails the query in every format, with an error naming its column, rather +than printing replacement characters. Wrap it in `hex()` or `CAST(… AS BLOB)` +to read its bytes. + Laying the table out to the terminal's width, and paging a long result, are both out of scope; pipe to `less` for the latter. diff --git a/packages/python/e2e-attestations/bot-1593-non-utf8-text-1104.json b/packages/python/e2e-attestations/bot-1593-non-utf8-text-1104.json new file mode 100644 index 00000000..999afdd3 --- /dev/null +++ b/packages/python/e2e-attestations/bot-1593-non-utf8-text-1104.json @@ -0,0 +1,7 @@ +{ + "command": "uv run python -m pytest tests/e2e/query_contract_test.py tests/e2e/on_file_query_test.py tests/e2e/cli_version_test.py -x -q", + "ran_at": 1793814772, + "exit_code": 0, + "commit": "43d8e7c1ebd157e80090a466348da1b9d5aafcda", + "branch": "bot/1593-non-utf8-text-1104" +} diff --git a/packages/python/tests/integration/binding/text_not_utf8_test.py b/packages/python/tests/integration/binding/text_not_utf8_test.py new file mode 100644 index 00000000..3546b662 --- /dev/null +++ b/packages/python/tests/integration/binding/text_not_utf8_test.py @@ -0,0 +1,33 @@ +"""Binding-tier tests for text SQLite holds as invalid UTF-8. + +The query fails with an ordinary exception naming the column, and the +instance stays usable afterwards. +""" + +import pytest + +from dirsql import DirSQL + +MESSAGE = 'column "v" holds text that is not valid UTF-8' + + +@pytest.fixture +def root(tmp_path): + (tmp_path / "a.json").write_text("{}", encoding="utf-8") + return str(tmp_path) + + +def describe_text_not_utf8(): + @pytest.mark.asyncio + async def it_raises_an_exception_naming_the_column(root): + db = DirSQL(root) + with pytest.raises(Exception, match=MESSAGE): + await db.query("SELECT CAST(x'ff41' AS TEXT) AS v") + + @pytest.mark.asyncio + async def it_leaves_the_instance_usable(root): + db = DirSQL(root) + with pytest.raises(Exception, match=MESSAGE): + await db.query("SELECT CAST(x'ff' AS TEXT) AS v FROM './a.json'") + rows = await db.query("SELECT basename FROM './a.json'") + assert rows == [{"basename": "a.json"}] diff --git a/packages/rust/changelog.d/2026-11-04-text-not-utf8-errors.md b/packages/rust/changelog.d/2026-11-04-text-not-utf8-errors.md new file mode 100644 index 00000000..e3efc4ed --- /dev/null +++ b/packages/rust/changelog.d/2026-11-04-text-not-utf8-errors.md @@ -0,0 +1,7 @@ +**Fixed** A query returning a text value that is not valid UTF-8 (for example +`CAST(x'ff' AS TEXT)` or `char(55296)`) now fails with +`column "" holds text that is not valid UTF-8`, in every CLI format and +SDK. It used to crash `--format table` and every SDK `query`, and the crash +left a Python or TypeScript `DirSQL` instance failing every later query with +a poisoned lock; the instance now stays usable. `--format json`, which printed +such text with replacement characters, fails the same way. diff --git a/packages/rust/migrations.d/2026-11-04-text-not-utf8-errors.md b/packages/rust/migrations.d/2026-11-04-text-not-utf8-errors.md new file mode 100644 index 00000000..3a70cc54 --- /dev/null +++ b/packages/rust/migrations.d/2026-11-04-text-not-utf8-errors.md @@ -0,0 +1,25 @@ +### Text that is not valid UTF-8 fails the query + +#### Summary + +A query returning a text value that is not valid UTF-8 now fails with `column "" holds text that is not valid UTF-8` (the new `DbError::NotUtf8`) in every CLI format and SDK, instead of crashing. `--format json`, which printed such text with replacement characters, now fails the same way. Queries over valid text are unchanged. + +#### Required changes + +To keep reading such a value, select its bytes instead of its text: + +| Before | After | +|---|---| +| `SELECT v FROM t` | `SELECT hex(v) AS v FROM t` or `SELECT CAST(v AS BLOB) AS v FROM t` | + +#### Deprecations removed + +_None._ + +#### Behavior changes without code changes + +`dirsql "SELECT CAST(x'ff41' AS TEXT) AS t" --format json` printed `[{"t":"�A"}]` (a replacement character) and exited 0; it now exits non-zero with the error above. A Rust caller matching on `DbError` exhaustively needs an arm for `NotUtf8`. + +#### Verification + +`dirsql "SELECT CAST(x'ff' AS TEXT) AS t" --format json` exits non-zero and prints `column "t" holds text that is not valid UTF-8` on stderr. diff --git a/packages/rust/src/db.rs b/packages/rust/src/db.rs index bc3a511e..83007aff 100644 --- a/packages/rust/src/db.rs +++ b/packages/rust/src/db.rs @@ -49,6 +49,9 @@ pub enum DbError { #[error("{0}")] Params(String), + + #[error("column {0:?} holds text that is not valid UTF-8")] + NotUtf8(String), } /// The reserved namespace for dirsql's internal bookkeeping tables. Every @@ -1565,8 +1568,8 @@ impl Db { let mut spare: Vec> = Vec::with_capacity(column_names.len()); while let Some(row) = rows.next()? { let mut cells = recycled(spare); - for i in 0..column_names.len() { - cells.push(row.get_ref(i)?); + for (i, column) in column_names.iter().enumerate() { + cells.push(utf8_checked(row.get_ref(i)?, column)?); } on_row(&mut state, &cells); spare = recycled(cells); @@ -1584,7 +1587,7 @@ impl Db { while let Some(row) = rows.next()? { let mut map = HashMap::new(); for (i, name) in column_names.iter().enumerate() { - let val: rusqlite::types::Value = row.get(i)?; + let val = rusqlite::types::Value::from(utf8_checked(row.get_ref(i)?, name)?); map.insert(name.clone(), Value::from(val)); } results.push(map); @@ -1900,6 +1903,17 @@ fn fill_cell(cell: &mut Value, source: ValueRef<'_>) { } } +/// `cell`, unless it is text that is not valid UTF-8: rusqlite panics +/// converting such text to an owned value. +fn utf8_checked<'a>(cell: ValueRef<'a>, column: &str) -> Result> { + match cell { + ValueRef::Text(bytes) if std::str::from_utf8(bytes).is_err() => { + Err(DbError::NotUtf8(column.to_string())) + } + _ => Ok(cell), + } +} + fn value_ref(value: &Value) -> ValueRef<'_> { match value { Value::Null => ValueRef::Null, @@ -2322,12 +2336,31 @@ mod tests { } #[test] - fn query_each_reads_text_that_is_not_utf8_lossily() { + fn query_each_fails_on_text_that_is_not_utf8() { let db = titled_docs(&["a"]); - let (_, rows) = folded(&db, "SELECT CAST(x'ff41' AS TEXT) AS t"); + let error = db + .query_each("SELECT CAST(x'ff41' AS TEXT) AS t", |_| (), |_, _| {}) + .unwrap_err(); - assert_eq!(rows, [[Value::Text("\u{FFFD}A".into())]]); + assert_eq!( + error.to_string(), + "column \"t\" holds text that is not valid UTF-8" + ); + } + + #[test] + fn query_ordered_fails_on_text_that_is_not_utf8() { + let db = titled_docs(&["a"]); + + let error = db + .query_ordered("SELECT CAST(x'ff41' AS TEXT) AS t") + .unwrap_err(); + + assert_eq!( + error.to_string(), + "column \"t\" holds text that is not valid UTF-8" + ); } fn owned(cell: &ValueRef<'_>) -> Value { @@ -2376,7 +2409,7 @@ mod tests { let held = db .query_each_ref( - "SELECT CAST(x'ff41' AS TEXT) AS t", + "SELECT CAST(x'c3a941' AS TEXT) AS t", &Params::default(), |_| Vec::new(), |held: &mut Vec>, cells| match cells { @@ -2386,7 +2419,7 @@ mod tests { ) .unwrap(); - assert_eq!(held, [b"\xffA".to_vec()]); + assert_eq!(held, [b"\xc3\xa9A".to_vec()]); } #[test] diff --git a/packages/rust/tests/json_text_e2e.rs b/packages/rust/tests/json_text_e2e.rs index 4603ad4a..c519743d 100644 --- a/packages/rust/tests/json_text_e2e.rs +++ b/packages/rust/tests/json_text_e2e.rs @@ -1,7 +1,6 @@ //! End-to-end tests for text cells in the CLI's JSON output: the real //! `dirsql` binary must print each text cell as the JSON string -//! `serde_json` writes for it, reading text SQLite holds as invalid UTF-8 -//! with replacement characters. +//! `serde_json` writes for it. //! //! Nothing is mocked (real process, real filesystem, real SQLite). //! @@ -30,14 +29,6 @@ fn json(sql: &str) -> String { String::from_utf8(out.stdout).expect("stdout must be UTF-8") } -#[test] -fn text_that_is_not_utf8_prints_with_replacement_characters() { - assert_eq!( - json("SELECT CAST(x'ff41' AS TEXT) AS t"), - "[{\"t\":\"\u{FFFD}A\"}]\n" - ); -} - #[test] fn text_needing_escapes_prints_with_json_escapes() { assert_eq!( diff --git a/packages/rust/tests/text_not_utf8_e2e.rs b/packages/rust/tests/text_not_utf8_e2e.rs new file mode 100644 index 00000000..56a9f6a4 --- /dev/null +++ b/packages/rust/tests/text_not_utf8_e2e.rs @@ -0,0 +1,51 @@ +//! End-to-end tests for text SQLite holds as invalid UTF-8: the real +//! `dirsql` binary must fail the query with an error naming the column, in +//! every format, never crash or print replacement characters. +//! +//! Nothing is mocked (real process, real filesystem, real SQLite). +//! +//! Gated behind `--features cli`: the `dirsql` bin target is +//! `required-features = ["cli"]`. + +#![cfg(feature = "cli")] + +use assert_cmd::prelude::*; +use tempfile::TempDir; + +fn failing(sql: &str, format: &str) -> String { + let root = TempDir::new().unwrap(); + let out = std::process::Command::cargo_bin("dirsql") + .expect("binary must exist") + .arg(sql) + .args(["--format", format]) + .current_dir(root.path()) + .output() + .expect("spawning `dirsql` failed"); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(!out.status.success(), "expected failure, stdout: {stdout}"); + assert!(!stdout.contains('\u{FFFD}'), "stdout: {stdout}"); + String::from_utf8(out.stderr).expect("stderr must be UTF-8") +} + +fn assert_names_the_column(stderr: &str) { + assert!( + stderr.contains("column \"t\" holds text that is not valid UTF-8"), + "stderr: {stderr}" + ); + assert!(!stderr.contains("panicked"), "stderr: {stderr}"); +} + +#[test] +fn table_format_fails_on_text_that_is_not_utf8() { + assert_names_the_column(&failing("SELECT CAST(x'ff41' AS TEXT) AS t", "table")); +} + +#[test] +fn json_format_fails_on_text_that_is_not_utf8() { + assert_names_the_column(&failing("SELECT CAST(x'ff41' AS TEXT) AS t", "json")); +} + +#[test] +fn a_lone_surrogate_fails_the_query() { + assert_names_the_column(&failing("SELECT char(55296) AS t", "table")); +} diff --git a/packages/ts/e2e-attestations/bot-1593-non-utf8-text-1104.json b/packages/ts/e2e-attestations/bot-1593-non-utf8-text-1104.json new file mode 100644 index 00000000..e49937de --- /dev/null +++ b/packages/ts/e2e-attestations/bot-1593-non-utf8-text-1104.json @@ -0,0 +1,7 @@ +{ + "command": "pnpm exec vitest run --dir tests/e2e query-contract query-params", + "ran_at": 1793814874, + "exit_code": 0, + "commit": "69038037f80feaf986806360f2547a05cc46d8dd", + "branch": "bot/1593-non-utf8-text-1104" +} diff --git a/packages/ts/tests/integration/binding/text-not-utf8.test.ts b/packages/ts/tests/integration/binding/text-not-utf8.test.ts new file mode 100644 index 00000000..a6952203 --- /dev/null +++ b/packages/ts/tests/integration/binding/text-not-utf8.test.ts @@ -0,0 +1,39 @@ +// Binding-tier tests for text SQLite holds as invalid UTF-8: the query +// rejects with an error naming the column, and the instance stays usable +// afterwards. +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { DirSQL } from "dirsql"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +const MESSAGE = 'column "v" holds text that is not valid UTF-8'; + +let dir: string; + +beforeEach(async () => { + dir = await mkdtemp(join(tmpdir(), "dirsql-not-utf8-")); + await writeFile(join(dir, "a.json"), "{}"); +}); + +afterEach(async () => { + await rm(dir, { recursive: true, force: true }); +}); + +describe("DirSQL text that is not UTF-8", () => { + it("rejects with an error naming the column", async () => { + const db = new DirSQL({ root: dir }); + await expect(db.query("SELECT CAST(x'ff41' AS TEXT) AS v")).rejects.toThrow( + MESSAGE, + ); + }); + + it("leaves the instance usable", async () => { + const db = new DirSQL({ root: dir }); + await expect( + db.query("SELECT CAST(x'ff' AS TEXT) AS v FROM './a.json'"), + ).rejects.toThrow(MESSAGE); + const rows = await db.query("SELECT basename FROM './a.json'"); + expect(rows).toEqual([{ basename: "a.json" }]); + }); +});