Repository navigation
Commit fc6ddb8
fix(driver-sqlite-wasm): a text value round-trips byte-for-byte — U+0000 no longer truncates it, a leading U+FEFF is no longer dropped on read (#19998)
Fixes #19978
Clause-②: no
`SqliteWasmDriver` now stores and reads a text value byte-for-byte, as
`SqlDriver` on better-sqlite3 does. Before this change, an embedded
U+0000 cut the stored value short, and a leading U+FEFF was dropped when
the value was read. Neither raised. The fix lives entirely in this
package's sql.js transport (`knex-wasm-dialect.ts` plus a new
`sqljs-exact-text.ts`). sql.js is not patched. No code line in
`SqlDriver` moves, and no public export moves. One doc-comment
parenthetical in `sql-driver.ts` and one sentence of a pending changeset
are corrected, because this fix makes them false (see "Deliberate
correction of a pending changeset" below).
## Where each byte was lost (measured first; the card's readings were a
scratch probe nobody had re-run)
sql.js 1.14.1 is what the lockfile installs (`pnpm-lock.yaml`:
`sql.js@1.14.1`) and what `node_modules/sql.js/package.json` reports.
The control is better-sqlite3 via driver-sql's devDependency. The probes
are scratch files, not committed.
**Raw sql.js, write side.** Bind a JS string, then read `hex(v)`. The
hex is ASCII, so the read decode cannot hide a write-side loss:
| value | its UTF-8 | sql.js stored | better-sqlite3 stored |
|:--|:--|:--|:--|
| `'a'` + U+0000 + `'b'` | `610062` | `61` | `610062` |
| `'ab'` + U+0000 | `616200` | `6162` | `616200` |
| U+FEFF + `'hello'` | `EFBBBF68656C6C6F` | `EFBBBF68656C6C6F` |
`EFBBBF68656C6C6F` |
| `'he'` + U+FEFF + `'llo'` | `6865EFBBBF6C6C6F` | same | same |
**Raw sql.js, read side.** Cells are planted with `cast(x'…' as text)`,
so no bind is involved:
| stored | sql.js `getString` | sql.js `getBlob` on the same cell |
better-sqlite3 |
|:--|:--|:--|:--|
| `610062` | `"a"` | `610062` | `"a\u0000b"` |
| `EFBBBF78` | `"x"` | `EFBBBF78` | `"x"` |
| `EFBBBF` + 33 ASCII bytes | the ASCII only | all bytes | BOM kept |
**H1 holds, and the loss has two separate causes.** The write loses a
U+0000. The read loses both a U+0000 and a leading U+FEFF. A U+FEFF was
always stored intact.
**H2: the exact functions.**
- Write: sql.js `Statement.prototype.bindString` calls
`sqlite3_bind_text(this.stmt, pos, strptr, -1, 0)`
(`dist/sql-wasm-debug.js` lines 684–695). The length `-1` tells SQLite
to read up to the first NUL.
- Read: `Statement.prototype.getString` returns `sqlite3_column_text`,
cwrapped with return type `"string"`, so `UTF8ToString` decodes it. That
function stops at the first NUL (`findStringEnd`) and decodes through a
module-level `new TextDecoder()` whose default `ignoreBOM: false` drops
a leading BOM. In the minified `dist/sql-wasm.js` this is `Za=new
TextDecoder` with
`z=(a,b,c)=>a?Za.decode(C.subarray(a,$a(C,a,b,c))):""`, so the BOM is
dropped at every length. The debug build decodes strings of 16 bytes or
fewer by hand, and those keep it. That is why a long-BOM case is pinned
too.
- Ours: `Client_WasmSqlite._query` in `knex-wasm-dialect.ts` owns both.
It binds through `stmt.bind` / `db.run` and decodes through
`stmt.getAsObject()`. `wasm-connection.ts` owns neither.
**Driver level, base `a7581b326`: `create` → `findOne`, and `find` with
`{ v: value }`.**
```text
== driver-sql on better-sqlite3 (control)
nul_mid stored-hex=610062 read="a\u0000b" faithful=true
nul_trail stored-hex=616200 read="ab\u0000" faithful=true
bom_lead stored-hex=EFBBBF68656C6C6F read="hello" faithful=true
== driver-sqlite-wasm (sql.js), before
nul_mid stored-hex=61 read="a" faithful=false
nul_trail stored-hex=6162 read="ab" faithful=false
bom_lead stored-hex=EFBBBF68656C6C6F read="hello" faithful=false
== driver-sqlite-wasm (sql.js), this head
(identical to the better-sqlite3 block, every line)
```
## The fix (H3: it fits in our adapter)
- **Read.** A cell that `stmt.get()` returns as a string is a TEXT cell.
For each one, `readExactRow` re-reads the stored bytes with
`Statement.getBlob` (`sqlite3_column_bytes` + `sqlite3_column_blob`;
SQLite hands a UTF-8 TEXT value to `column_blob` unconverted) and
decodes them with `TextDecoder('utf-8', { ignoreBOM: true })`. `getBlob`
is not in `@types/sql.js`, but it is the method sql.js's own `get()`
calls for a BLOB column. It keeps its name in all three 1.14.1 builds
this package can load (`sql-wasm.js`, `sql-wasm-browser.js`,
`sql-wasm-debug.js`); `getString` is renamed in the minified two. If
`getBlob` is absent, the read throws and does not fall back to the lossy
decode.
- **Write.** Only a string binding that holds U+0000 is changed; nothing
else can be truncated. It is bound as its UTF-8 bytes (a `Uint8Array`,
which sql.js binds as a BLOB with an explicit length), and every
parameter token that receives it is wrapped as `+CAST(PARAM AS TEXT)`.
The unary plus is load-bearing. `CAST(… AS TEXT)` alone carries TEXT
affinity and changes a comparison. Measured on sql.js: comparing the
integer `5` as less than the text `' x'` answers 1 against a bound text,
0 through `CAST(? AS TEXT)`, and 1 through `+CAST(? AS TEXT)`
(better-sqlite3 bound: 1). A numeric-affinity column stores `'12'` as
integer through either path. A statement with no such binding comes back
as the same string and the same array.
- **Which token receives which binding** follows SQLite's own numbering
rule, which `sqljs-exact-text.ts` applies:
- bare `?` takes the largest index so far + 1;
- `?NNN` takes `NNN`;
- `:name` / `@name` / `#name` / `$name` take the index of their first
occurrence;
- nothing inside quotes, `[…]` or comments is a parameter.
Wrapping adds or removes no parameter token, so no index moves. No
statement form is refused, which is why this is `Clause-②: no` and not a
narrowing.
- **Ruled out:** an explicit-length `sqlite3_bind_text` through the
statement pointer. That pointer is `this.stmt` only in the debug build;
the minified builds rename it (`this.Qa`), so it is not a usable seam.
## H4: why the shared case table is not edited here
`VALUE_ROUNDTRIP_CASES` lives in
`packages/spec/src/data/value-roundtrip-conformance.ts`. The claim's
file surface allowed a shared case file under
`packages/drivers/driver-sql/src/` "that the SQLite family's conformance
suites read". No such file exists, and the real one is in
`packages/spec`, so this is the stop-on-breach case and the table is not
edited. There is a second reason, a substantive one.
`sql-driver-value-roundtrip-conformance.test.ts` runs that table through
the dialect matrix, including the live Postgres CI job. Postgres
documents that its `text` type cannot store the character with code
zero, so a U+0000 row would make that cell fail on Postgres. That is a
platform decision (refuse U+0000 everywhere? answer per dialect?), not a
driver fix. It is NOT MEASURED here, because there is no live Postgres
in this container. A leading-U+FEFF row is a better candidate for the
shared table, but how the MySQL and Postgres drivers decode it is
unmeasured here. Both questions go to the seat in the report. Meanwhile
the answer is pinned in this package, against the written value and its
own UTF-8.
## Tests
New file `sqlite-wasm-text-bytes-roundtrip.test.ts`, 35 cases. Each seam
is pinned on its own, because one can hide the other:
- **Write:** `hex(v)` equals the written string's UTF-8, for U+0000 in
the middle and trailing, U+FEFF leading (short and long) and in the
middle, and a plain control.
- **Read:** cells planted by SQL literal read back exactly (`610062`,
`EFBBBF78`, a long BOM value, a plain control).
- **Round trip:** `create` → `findOne`, and `update` (whose returned row
is also checked).
- **Filters:** equality on each exact value selects that row and no
other. Equality on `'a'` no longer matches the stored `'a'` + U+0000 +
`'b'`; before, the comparand was cut at the same NUL, so it did. `$in`
places each NUL-bearing comparand. `$contains` U+FEFF and `$startsWith`
U+FEFF select the right rows and read them back whole.
- **Placement:**
- the no-affinity pin;
- the statement comes back unchanged when no binding holds U+0000;
- placement past `'it''s ?'`, `` `a?` ``, `[b?]`, `"?"`, both comment
forms and the identifier `c$d`;
- `?NNN` and `:name` through the driver;
- SQLite's numbering on a mixed statement;
- a binding no parameter receives is left for sql.js to answer as
before.
- **Refusal of the fallback:** a statement object without `getBlob`
throws.
**Ablations.** Each ran from the committed state through
`scripts/ablation-replace.mjs`. Every mutation was proved on disk
(anchor 1 → 0, blob changed), and every restore was proved (blob equal
to HEAD, `git diff HEAD` empty). The subject resolves from `src`
(relative imports), so no build was involved:
- **A: read seam back to `stmt.getAsObject()`.** 11 of 35 red: every
read pin, the update pin, `$contains` U+FEFF (its values), and the two
numbered/named pins. Every write-hex pin and every equality filter
stayed green.
- **B: write rewrite disabled** (`truncatable.size === 0` →
`truncatable.size >= 0`). 10 of 35 red: the U+0000 hex pins and reads,
update, the prefix-equality pin, and the placement pins. Every read-seam
pin stayed green. A first attempt used a replacement that was a
substring of its anchor. The tool refused it (replacement count 1 → 1)
and restored, and no test ran; it was repeated with a distinct
replacement.
- **C: the unary plus removed.** 3 red: the affinity pin (`r: 0`, not 1)
and the two literal-placement pins.
- **D: `?NNN` numbered as a bare `?`.** 2 red: the `?2`/`?1` driver pin
and the mixed-statement pin.
Package runs at `58ee6c042`:
- `pnpm --filter @objectstack/driver-sqlite-wasm test`: 30 files, **556
passed**.
- `typecheck` (`tsc --noEmit`): exit 0. Its program includes both new
files, checked with `--listFiles`.
## Deliberate correction of a pending changeset
The dispatch asked whether the "not read back verbatim" sentence in
`.changeset/19912-json-backfill-depth-limit.md` still holds. That
changeset is still pending and belongs to a landed PR. It says the
backfill leaves as stored, "on `SqliteWasmDriver`, a legacy text with a
leading U+FEFF or an embedded NUL (sql.js drops both when it reads the
text)". **After the fix (head `58ee6c042`) that is false.** Measured
with a scratch probe: legacy json TEXT cells were planted, then the
backfill ran via a second `initObjects`.
```text
legacy better-sqlite3 wasm, both seams ablated (dist rebuilt) wasm, this head
bom (U+FEFF + x) EFBBBF78 22EFBBBF7822 EFBBBF78 (left) 22EFBBBF7822
nul (a + U+0000 + b) 610062 22615C75303030306222 610062 (left) 22615C75303030306222
plain control 68656C6C6F 2268656C6C6F22 2268656C6C6F22 2268656C6C6F22
```
`SqliteWasmDriver` now converges these cells exactly as better-sqlite3
does. `ablation-dist-preflight` confirmed the ablation was present in
`dist/` for the control. After the rebuild it was absent again and the
tree was clean. The same parenthetical sits in
`SqlDriver.backfillCanonicalJsonEncoding`'s doc block in
`packages/drivers/driver-sql/src/sql-driver.ts` ("sql.js drops an
embedded NUL and a leading U+FEFF"). The seat ruled that both are
corrected in this PR (claim amendment 5818397702 on #19978). Patch round
1 (`6a195b37c`) made the edits below, and patch round 2 (`98cb90873`)
corrected them after contract review 5819169241 (FAIL). Round 1 had
labelled `TursoDriver`'s local mode as libsql, but every non-remote arm
of `TursoDriver.toKnexConfig` hands Knex `client: 'better-sqlite3'`. The
measurement behind the edits was taken fresh at `58ee6c042` on three
local faces, which run on two engines: `SqlDriver` on better-sqlite3,
`TursoDriver` in local mode (better-sqlite3 through Knex, url
`:memory:`), and `SqliteWasmDriver` (sql.js). All three convert
`EFBBBF78` → `22EFBBBF7822`, `610062` → `22615C75303030306222` and the
control `68656C6C6F` → `2268656C6C6F22`. All three leave invalid UTF-8
(`FF78`, `61C3`) as stored, because each reads it back with U+FFFD in
place of the invalid bytes. `SqlDriver`'s `sqlite3` / `sqlite` clients
are NOT MEASURED, because that client is not installed in the container.
libsql is not a local engine here and is not cited. The reviewer read it
directly (`@libsql/client` 0.17.4, `:memory:`): a stored `FF78` panics
the native binding, and a stored `610062` reads back as `"a"`.
**`.changeset/19912-json-backfill-depth-limit.md`** (pending, landed
with PR #19972; one sentence of the merge base rewritten into three):
- Old: "A cell the engine does not read back verbatim is left as stored
and keeps reading as it did, where the old statement rewrote it: on
`SqliteWasmDriver`, a legacy text with a leading U+FEFF or an embedded
NUL (sql.js drops both when it reads the text); on any engine, text
holding invalid UTF-8."
- New: "A cell the engine does not read back verbatim is left as stored
and keeps reading as it did, where the old statement rewrote it: text
holding invalid UTF-8, measured on better-sqlite3 and sql.js.
`SqlDriver` on better-sqlite3, `TursoDriver` in local mode (which runs
on better-sqlite3 too) and `SqliteWasmDriver` (sql.js) were each
measured to read such a cell back with U+FFFD in place of the invalid
bytes, so the text the rewrite would be decided from is not the stored
text, and the cell is left alone. A legacy text with a leading U+FEFF or
an embedded NUL is read back verbatim on all three faces, and is
rewritten like any other plain string."
**`packages/drivers/driver-sql/src/sql-driver.ts`**, in the
`backfillCanonicalJsonEncoding` doc block. Two comment lines change and
no code line moves. A build of driver-sql from the merge base and one
from the head differ in exactly those two comment lines of
`dist/index.js` / `dist/index.mjs`, re-proved in round 2. The driver-sql
suite reads 2680 passed / 170 skipped on both copies (round 1).
- Old: "(sql.js drops an embedded NUL and a leading U+FEFF; any engine
replaces invalid UTF-8)"
- New: "(better-sqlite3 and sql.js, the engines measured, read invalid
UTF-8 back as U+FFFD)"
**This PR's own changeset** gains one bullet: the local `Field.json`
backfill now converts those legacy cells on this driver too, with the
measured bytes.
`Check Changeset` is red by design on this head.
`check-empty-changeset.mjs` reads the 19912 edit as the DELIBERATE
CORRECTION class ("do NOT restore it -- say so on the PR and get it
confirmed"). The edit is not restored and `skip-changeset` is not
applied. Under ruling 1A (#19940, 5814546887), the confirmation is a
same-head at-tier contract-review PASS that names this note and judges
each rewritten sentence.
## Gates
- **Derived gate list.** `node scripts/pm/dispatch-gates.mjs --commands
--repo objectstack-ai/objectstack` on this head gives 59 commands. All
59 exit 0 at `58ee6c042`. `--ran` reconciliation: 59 derived, 59 run, 0
NOT-MEASURED (derived from recorded exit codes).
- `check:dual-build-cjs-loads`, `check:lean-entry-closure` and
`check:type-check-debt` first answered PREREQUISITE NOT MET (exit 3).
They were re-run after `turbo run build --filter='./packages/*'
--filter='./packages/*/*'` and answered 0.
- `check:query-options-erasure` first went red: the test surface grew
from 236 to 242, from this test's `as any` query casts. The queries are
now typed as `DriverQuery`, and the gate is green.
- **Driver conformance census, before and after.** `pnpm
check:driver-conformance` reads `OK — 50 covered cell(s), 0 in the DEBT
ledger, 0 exempt` both on base `a7581b326` and at `58ee6c042`. No cell
was added or removed.
- **`node scripts/check-issue-citations.mjs --base origin/main`:** exit
0.
- **Patch round 1 at `6a195b37c`.** `dispatch-gates` re-derives 62
commands: the 59 above, plus `check:dispatcher-error-vocabulary`,
`check:object-def-param-keys` and `check:tenant-chokepoint`, which
`sql-driver.ts` brings in. All 62 ran after a `./packages/*` build. 61
exit 0, and `check-empty-changeset` exits 1 by design (the correction
above). `check-issue-citations --base origin/main` exits 2 at this head,
but only because `origin/main` moved past the merge base: #19988 removed
three `#16608` citations from a file this diff does not touch. Against
the merge base `67ebc84a7` it exits 0. The driver-sqlite-wasm suite
reads 30 files / 556 passed, and typecheck exits 0 for
driver-sqlite-wasm and driver-sql. `check:driver-conformance` is
unchanged: 50 covered, 0 in debt.
- **Patch round 2 at `98cb90873`** (text only, the same two files).
- `check-changeset-no-major`, `check-adr-0087-registration`,
`check-issue-citations --base 67ebc84`, `check:doc-authoring`,
`check:nul-bytes` and driver-sql typecheck all exit 0.
- `check-empty-changeset` exits 1 by design: one `::error` annotation,
on the 19912 note.
- `dispatch-gates` re-derives the same 62 families.
- The two changesets this PR touches name libsql 0 times and "any
engine" 0 times.
- **Lint, narrowed and proven.** `pnpm exec eslint --no-inline-config
--format json` over the three changed TypeScript files gives 3 files, 0
errors, 0 warnings.
- None of the three is ignored: an explicitly passed ignored file
reports a warning, and there were none.
- `eslint.config.mjs` sets no `parserOptions.project` (0 hits for
`project:` / `projectService`), so linting is not type-aware and this
diff cannot move any untouched file's verdict.
- The repo-wide `pnpm lint` is CI's.
## Acceptance notes
- **Found here, out of scope:** a `$contains` or `$startsWith` whose
comparand holds U+0000 is matched on both SQLite faces, better-sqlite3
included, with the GLOB pattern cut at that U+0000 (wildcards after it
included) against each value cut at its own first U+0000. So `$contains
'a'`+U+0000 answers only the row stored as `'a'`+U+0000+`'b'` (measured
by the second contract review). `glob()` cuts both the pattern and the
value at their first U+0000. So a comparand that starts with U+0000
makes `$contains` and `$endsWith` match every row, and makes
`$startsWith` answer only the rows that are empty before their first
U+0000 (re-measured in patch round 2 on better-sqlite3 and sql.js, which
answer identically). The SQLite text predicate is `GLOB`, and `glob()`
reads its pattern as a C string. Measured on both faces with the probe
above. That is a filter that silently gives the wrong answer,
reproducible today. It is not fixed here; the seat filed it as #19999.
- `WasmSqliteConnection`'s `defaultLocateFile()` calls
`require.resolve('sql.js/package.json')`. That throws
`ERR_PACKAGE_PATH_NOT_EXPORTED` on sql.js 1.14.1, whose `exports` map
has no `./package.json`. So it always returns `undefined`, and sql.js
then locates its own `.wasm`, which works. Behaviour is unaffected; only
the docblock's claim is dead. Noted, not filed. Carrier: none.
- `origin/main` (`67ebc84a7`) was merged in before opening. It shares no
path with this diff, and the lockfile did not move. The dependency
closure was rebuilt and the package suite re-run on the merged head.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 14add48 commit fc6ddb8
6 files changed
Lines changed: 568 additions & 8 deletions
File tree
- .changeset
- packages/drivers
- driver-sqlite-wasm/src
- driver-sql/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
| 10 | + | |
11 | 11 | | |
12 | 12 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11308 | 11308 | | |
11309 | 11309 | | |
11310 | 11310 | | |
11311 | | - | |
11312 | | - | |
| 11311 | + | |
| 11312 | + | |
11313 | 11313 | | |
11314 | 11314 | | |
11315 | 11315 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
26 | 26 | | |
27 | 27 | | |
28 | 28 | | |
| 29 | + | |
29 | 30 | | |
30 | 31 | | |
31 | 32 | | |
| |||
229 | 230 | | |
230 | 231 | | |
231 | 232 | | |
232 | | - | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
233 | 238 | | |
234 | 239 | | |
235 | 240 | | |
| |||
244 | 249 | | |
245 | 250 | | |
246 | 251 | | |
247 | | - | |
| 252 | + | |
248 | 253 | | |
249 | 254 | | |
250 | 255 | | |
251 | 256 | | |
252 | 257 | | |
253 | 258 | | |
254 | 259 | | |
255 | | - | |
| 260 | + | |
256 | 261 | | |
257 | 262 | | |
258 | 263 | | |
| 264 | + | |
259 | 265 | | |
260 | | - | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
261 | 270 | | |
262 | 271 | | |
263 | 272 | | |
| |||
266 | 275 | | |
267 | 276 | | |
268 | 277 | | |
269 | | - | |
| 278 | + | |
270 | 279 | | |
271 | 280 | | |
272 | 281 | | |
| |||
0 commit comments