## Summary
- track sessions created by `DatabaseSync`
- close and invalidate active sessions before closing the database
connection
- keep sessions from an earlier connection closed after the database is
reopened
## Motivation
Session objects checked whether the database's shared connection state
was populated. Reopening the database populated that state again, so
sessions created for the previous connection could be treated as open.
Tying each session handle to the database's active-session list keeps
its lifecycle associated with the connection that created it.
## Testing
- `cargo check -p deno --lib`
- `cargo test unit_node::sqlite_test`
- `./tools/format.js ext/node_sqlite/database.rs
ext/node_sqlite/session.rs tests/unit_node/sqlite_test.ts`
- `./tools/lint.js`
This refreshes the vendored Node.js compatibility test suite so we
exercise Deno against a more recent set of upstream tests. The node_test
submodule is bumped from Node.js 26.3.0 to 26.5.1 (denoland/node_test@f287abd).
`parallel/test-http-rawheaders-limit.js` was removed upstream, so its
config.jsonc entry is dropped, it would otherwise silently never run.
The refreshed suite adds assertions that Deno did not satisfy. Rather
than marking those tests ignored, this implements what they check. Where Node
implements the same behaviour in JS, the implementation here follows
theirs:
- zlib rejects inputs whose `length`/`byteLength` are shadowed by
accessors, and an out-of-range `_outOffset`, with ERR_OUT_OF_RANGE. Node enforces
this in `node_zlib.cc` against the real backing store; Deno's ops already
bounds-check the same way, so this is the error shape the tests want
rather than a memory-safety fix. It does, however, close a panic: the Brotli
encoder ops indexed with the caller's offsets unchecked, so this also
adds the bounds checks the zlib and zstd ops already have, for callers that
drive `_handle.writeSync()` directly and bypass the polyfill.
- zlib honours `rejectGarbageAfterEnd` in the sync paths, and validates
that the option is a boolean. This also defines
ERR_TRAILING_JUNK_AFTER_STREAM_END, which was referenced but never
declared, so constructing it threw "is not a constructor", the existing
DecompressionStream tests passed only because that happens to be a
TypeError too.
- node:sqlite invalidates a TagStore iterator once the cached statement
is rebound by get/all/run/iterate, mirroring StatementSync's
reset_generation. A stale iterator could otherwise resume over another caller's
parameters. Generations come from a store-wide sequence rather than counting up per
cache entry, so an entry evicted from the LRU and re-prepared under the
same SQL cannot hand a stale iterator a generation it already recorded.
- net rejects `options.host` containing a null byte with
ERR_INVALID_ARG_VALUE, via a `validateStringWithoutNullBytes` validator
as upstream does.
- tls IDNA-normalizes the hostname in `checkServerIdentity` and matches
on that, so a fullwidth full stop cannot stand in for a label separator
while a legitimate U-label host still matches a wildcard that covers it.
- http sends the tunnel target, not the proxy, in a CONNECT Host header.
- https includes pfx buffers and passphrases in Agent#getName, via a
port of upstream's `getPfxAgentKey`.
- crypto accepts the historical dss1 alias for sha1 in createHmac.
- timers rejects a foreign `this` in scheduler.yield/wait with
ERR_INVALID_THIS.
- streams flushes a partial character to U+FFFD when setEncoding() is
called after EOF.
- URLSearchParams lets `null` reach the USVString overload per WebIDL
union resolution, as Node now does. `undefined` still means "not passed". This
is the one web-visible change here rather than a node-only one.
- require(esm) top-level-await errors carry a non-enumerable
`requireStack` and render a "Require stack:" list.
One test stays ignored: `test-require-module-tla-execution.js` asserts
the error hint names `--experimental-print-required-tla`, and Deno
deliberately points at `--stack-trace-limit` since it does not implement that flag.
The `process.version` bump that accompanies this Node.js release is left
to a separate PR, as with the 26.3.0 bump.
This prevents `DatabaseSync.deserialize()` from replacing database
contents while a user-defined SQLite callback is active.
`deserialize()` finalizes existing statements, so calling it reentrantly
could invalidate the statement SQLite is currently executing. The method
now uses the existing callback-depth guard and returns
`ERR_INVALID_STATE`, matching `DatabaseSync.close()` behavior.
Validation:
- `./tools/format.js`
- `cargo build --bin deno`
- focused `DatabaseSync.deserialize` callback regression
- full `tests/unit_node/sqlite_test.ts` suite (78 tests)
This makes `node:sqlite` treat JavaScript conversion failures as normal,
catchable errors.
V8 property and index access can execute JavaScript and fail, while C
string conversion rejects embedded null bytes. The affected statement
binding, tag store, function and aggregate option, iterator setup, and
session option paths now propagate those failures instead of assuming
conversion always succeeds. Result and error properties are also defined
directly to avoid invoking inherited setters.
After a failed conversion, the database remains usable by subsequent
operations.
Two hot-path fixes for `node:sqlite`, from profiling the insert/select
benchmark (was ~2× slower than Node).
**`all()`** rebuilt the column-name V8 strings *per row* (`read_row`
recreates them every row). For a 500-row × 4-col result that's ~2000
redundant string allocations + `sqlite3_column_name` FFI calls. Hoisted
the names out of the row loop — built once, reused for every row, which
also gives every row object the same shape.
**`run()`** built the `{ lastInsertRowid, changes }` result with
`Object::set`, which goes through V8's generic `[[Set]]` runtime path.
Switched to `create_data_property` (defines own data properties
directly). Smaller effect — the result object turned out to be a minor
part of insert cost, not the dominant one.
Throughput (1 MiB in-memory, aarch64, interleaved best-of-N):
| | before | after | Node |
|---|---:|---:|---:|
| `select.all` (500 rows) | ~5.0k | ~9.4k (**+85%**) | ~8.6k |
| `insert` (1000-row tx) | ~2.0k | ~2.1k (+6%) | ~3.9k |
Select now matches/beats Node. Insert's remaining gap is diffuse
per-call overhead (slow-op dispatch + `bind_params` + `step`), not any
single hot spot. Result objects verified unchanged: `Object.prototype`,
same keys/values. `tests/unit_node/sqlite_test.ts`: 77/77 pass.
`node:sqlite` disables `ATTACH DATABASE` when a process does not have
full permissions for the database path, because an attached database can
reach file paths outside the original database path. It does this by capping
`SQLITE_LIMIT_ATTACHED` at 0 for those connections.
The `DatabaseSync` limits support could raise that cap back up, which
undoes the protection. `apply_initial_limits` ran after the cap was set, so a
`new DatabaseSync(path, { limits: { attach: 1 } })` option re-enabled
attach, and the `db.limits.attach` setter wrote the limit directly with no
guard. In both cases a process with only scoped read/write access to one directory
could then `ATTACH DATABASE` a file in a directory it was explicitly denied.
This treats the `SQLITE_LIMIT_ATTACHED = 0` cap as authoritative for
those connections rather than user-tunable. Both surfaces now reject raising
the cap the same way: the `limits.attach` constructor option and the
`db.limits.attach` setter throw `ERR_ACCESS_DENIED` when asked to raise the limit above 0
while the cap is in force (a value of 0, keeping it disabled, is still accepted).
The setter error is a plain `Error`, not a `TypeError`, since an
access-denied condition is not a type error. When the process does have full
permissions nothing changes: the constructor option and the setter raise the limit
as before.
Added a spec test that runs with scoped `--allow-read`/`--allow-write`
on one directory and explicit `--deny-read`/`--deny-write` on another, and
checks that a direct read, a direct open, a default `ATTACH`, the `limits.attach`
constructor option, and the `db.limits.attach` setter are all unable to
reach the denied database, with the last two rejected by `ERR_ACCESS_DENIED`.
A companion `--allow-all` step is the positive regression guard: it
verifies that with full permissions both the option and the setter raise the limit and
`ATTACH` then succeeds, so a future change that over-tightens the cap
would fail CI.
Fixes sqlite iterator callback lifetime handling for StatementSync and
SQLTagStore when next/return are detached from the iterator object.
The iterator callback data now owns a strong reference to the parent
wrapper and releases the native context after both generated callbacks
are collected. The iterator __statement_ref property is also defined as
non-writable, non-configurable, and non-enumerable defense-in-depth.
Adds a regression test that exercises detached callbacks after forced GC
for both StatementSync.iterate() and SQLTagStore.iterate().
Tests:
- cargo check -p deno_node_sqlite
- cargo build --bin deno
- ./target/debug/deno run --quiet --v8-flags=--expose-gc
tests/unit_node/testdata/sqlite_iterate_detached_gc.ts
- cargo build -p test_server
- cargo test -p unit_node_tests --test unit_node -- sqlite_test
Closesdenoland/divybot#516
Co-authored-by: divybot <divybot@users.noreply.github.com>
Co-authored-by: Divy Srivastava <me@littledivy.com>
Summary:
- defer freeing node:sqlite authorizer data while the SQLite callback is
still on the stack
- keep the existing authorizer registered when replacement validation
fails
- mark sqlite exec and prepare reentrant because authorizers can call
back into node:sqlite
Tests:
- `BINDGEN_EXTRA_CLANG_ARGS="-I/usr/lib/gcc/x86_64-linux-gnu/13/include"
cargo check -p deno_node_sqlite`
- `BINDGEN_EXTRA_CLANG_ARGS="-I/usr/lib/gcc/x86_64-linux-gnu/13/include"
cargo build -p deno -p test_server`
- `BINDGEN_EXTRA_CLANG_ARGS="-I/usr/lib/gcc/x86_64-linux-gnu/13/include"
cargo test -p unit_node_tests --test unit_node -- sqlite_test
--nocapture`
Closesdenoland/divybot#515
Co-authored-by: divybot <divybot@users.noreply.github.com>
Co-authored-by: Divy Srivastava <me@littledivy.com>
## Summary
`DatabaseSync.close()` finalized all prepared statements and dropped the
SQLite
connection without checking whether one of our user-defined callbacks
(`db.function`, `db.aggregate` step/inverse/value/final, authorizer,
`applyChangeset` filter/onConflict) was currently on the SQLite VDBE
stack.
Calling `db.close()` from inside an aggregate `step` callback freed the
in-flight statement and the connection while SQLite was still executing
— when
control returned to SQLite (and to our aggregate trampoline
`custom_aggregate_step_base`), it dereferenced the freed objects: a
native
**use-after-free**, reproducible with no permissions and no
`--unstable`.
This change:
- Adds a `callback_depth: Rc<Cell<usize>>` to `DatabaseSync` and an RAII
`CallbackDepthGuard` that bumps the counter at the top of every callback
trampoline (scalar function, aggregate step/value, authorizer) and
across
`sqlite3changeset_apply()`.
- Makes `DatabaseSync::close()` return a new `ERR_INVALID_STATE` SQLite
error
(`"cannot close database while a user-defined callback is running"`)
when
`callback_depth > 0`, instead of finalizing the in-flight statement.
- Marks `StatementSync#run`, `#all`, and `#iterate` as `#[reentrant]` so
the
new error thrown from inside a callback can propagate out through SQLite
rather than tripping the op-call reentrancy assertion.
The connection stays valid after a refused close.
## Reproducer
The original PoC now exits cleanly instead of crashing (SIGSEGV/SIGBUS
were
the typical pre-fix outcomes):
```js
import { DatabaseSync } from "node:sqlite";
const db = new DatabaseSync(":memory:");
db.exec("CREATE TABLE t(x); INSERT INTO t VALUES (1),(2),(3),(4),(5)");
let n = 0;
db.aggregate("a", {
start: 0,
step(acc, x) { if (++n === 2) db.close(); return acc; },
});
db.prepare("SELECT a(x) FROM t").get();
```
```
$ deno run poc.mjs
error: SqliteError: cannot close database while a user-defined callback is running
```
## Test plan
- [x] `cargo test -p unit_node_tests --test unit_node -- sqlite_test`
passes
with two new regression tests (aggregate step + scalar function).
- [x] PoC from the issue (`deno run poc.mjs`) throws `ERR_INVALID_STATE`
instead of crashing; verified across multiple runs (no flaky SIGSEGV).
- [x] `tools/lint.js` and `tools/format.js` clean.
Closesdenoland/orchid#330
Co-authored-by: divybot <divybot@users.noreply.github.com>
Co-authored-by: Divy Srivastava <me@littledivy.com>
Node.js 22.12+ added [tagged template literal support for
SQLite](https://nodejs.org/api/sqlite.html#class-databasesyncsqltagstore)
via `db.createTagStore(capacity)`, returning a `SQLTagStore` with
`.run`, `.get`, `.all`, `.iterate`, and `.clear` methods that build SQL
strings from template parts and bind interpolated values as parameters.
Prepared statements are cached by SQL text in an LRU.
The Rust-side `SQLTagStore` and `DatabaseSync.createTagStore` were
already in place. The only behavior gap was that `size` was exposed as a
method (`#[fast]`) rather than a getter, which diverges from Node's
`sql.size` property access. This PR:
- Changes `size` to `#[getter]` so `sql.size` matches Node.
- Extends the existing unit tests: closed-database `ERR_INVALID_STATE`,
descriptive `ERR_SQLITE_ERROR` for missing columns/tables, and a check
that the default capacity is `1000`. Updates the existing tests to use
property access for `size`.
Co-authored-by: divybot <divybot@users.noreply.github.com>
## Summary
Upgrades rusty_v8 from 147.2.1 to 147.4.0. No Deno code changes needed.
### Performance improvements
- **simdutf string conversion hot paths** (`to_rust_string_lossy`,
`to_rust_cow_lossy`, `write_utf8_into`, `ValueView::to_cow_lossy`):
Latin-1 strings 2-4x faster, UTF-16 3-6x faster, ASCII unchanged.
Already wired up via the `simdutf` feature flag.
- **IsolateAnnex pointer caching** in scope structs reduces indirection
on every scope operation.
### Other changes
- feat: `Module::EvaluateForImportDefer` and `GetModuleNamespace` with
phase (for future `import.defer()` support)
- fix: enable linux shared-library-safe v8 TLS mode by default
- refactor: store `IsolateHandle` data in its own allocation
Fixes 3 SQLite Node.js compat tests from #32706:
- **`prepare()` options**: Support `readBigInts`, `returnArrays`,
`allowBareNamedParameters`, and `allowUnknownNamedParameters` as second
argument to `DatabaseSync.prepare(sql, options)`
- **Defensive mode default**: Changed from `false` to `true` to match
Node.js
- **Iterator invalidation**: Calling `get()`, `all()`, `run()`, or
`iterate()` on a statement now invalidates any active iterator, causing
`next()` to throw `ERR_INVALID_STATE` with "iterator was invalidated"
### Enabled tests
- `test-sqlite-config.js`
- `test-sqlite-named-parameters.js`
- `test-sqlite-statement-sync.js`
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
I let claude loose on improving compile times and it suggested splitting
up `deno_node` by extracting the crypto stuff and node sqlite. And the
other change is getting rid of all unused constants in `winerror.rs`.
That changes it from 17k lines down to 240.
| What | Time | Delta |
|---|--:|--:|
| main | 214.6s | - |
| PR | **162.8s** | **-51.8s** |
## Summary of changes
1. deno_node compile: -13.4s (48%) - Splitting out crypto and sqlite
plus removing 16K lines of unused winerror constants nearly halved
compile time.
2. deno_runtime: -5.2s (36%) - Less debug info to process.
3. Parallelism gains - deno_node_crypto (8.3s) and deno_node_sqlite
(2.5s) now compile concurrently with other work instead of being
serialized inside the monolithic deno_node.
Total CPU time for node crates
- Before: deno_node = 50.1s (both passes)
- After: deno_node (27.5s) + deno_node_crypto (16.5s) + deno_node_sqlite
(4.7s) = 48.6s
- Similar CPU time, but wall-clock is much better due to parallelism.
denobot