Skip to content

sqlite: reject deserialize() while in a callback #64796

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
trivikr wants to merge 3 commits into
base: main
Choose a base branch
from
+25 −2
Open

sqlite: reject deserialize() while in a callback #64796

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
36331c2
sqlite: reject deserialize() while in a callback
trivikr Jul 28, 2026
6c6dcfc
fixup! test: reorder and rename test
trivikr Aug 6, 2026
0081aa8
fixup! doc: update doc
trivikr Aug 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions doc/api/sqlite.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -613,8 +613,10 @@ added:
Loads a serialized database into this connection, replacing the current
database. The deserialized database is writable. Existing prepared statements
are finalized before deserialization is attempted, even if the operation
subsequently fails. This method is a wrapper around
[`sqlite3_deserialize()`][].
subsequently fails. An \[`ERR_INVALID_STATE`]\[] error is thrown if the method is

@TrevorBurnham TrevorBurnham Aug 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The escaping silences the no-undefined-references lint warning, but it also stops this being a link — it renders as literal bracket characters. Through micromark (the copy in tools/lint-md/node_modules):

An \[`ERR_INVALID_STATE`]\[] error is thrown.
→ <p>An [<code>ERR_INVALID_STATE</code>][] error is thrown.</p>

So nodejs.org would show stray [...][] brackets and no link to the error description.

The underlying problem is just a missing link definition. This renders as a real anchor and passes lint-md cleanly (I verified exit 0 in a full checkout, so the nodejs-links cross-file check is happy too):

subsequently fails. An [`ERR_INVALID_STATE`][] error is thrown if the method is

plus, in the definition block:

[`ERR_INVALID_STATE`]: errors.md#err_invalid_state

One gotcha: that definition needs to go immediately before [`PRAGMA foreign_keys`]:. Putting it before [`SQLTagStore`]: instead trips nodejs-links with Unordered reference ("ERR_INVALID_STATE" should be before "SQLITE_MAX_FUNCTION_ARG").

called while a statement is executing, for example from a user-defined function,

@TrevorBurnham TrevorBurnham Aug 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"while a statement is executing" doesn't quite describe the guard, which is IsInCallback() — callback depth, not statement execution. I checked both directions against a build of this branch.

A statement executing but no throw:

for (const row of stmt.iterate()) {
  db.deserialize(serialized);  // succeeds
  break;                       // → "statement has been finalized"
}

No statement executing but it throws — an authorizer firing during prepare():

db.setAuthorizer(() => { db.deserialize(serialized); return 0; });
db.prepare('SELECT v FROM t');  // → ERR_INVALID_STATE

The example list is also missing applyChangeset()'s filter/conflict callbacks, which are guarded (sqlite3changeset_apply takes a CallbackDepthGuard). Something like "while a database callback is on the stack, for example a user-defined function, an aggregate function, an authorizer, or a changeset filter or conflict handler" would match the implementation.

an aggregate function, or an authorizer callback. This method is a wrapper
around [`sqlite3_deserialize()`][].

```mjs
import { DatabaseSync } from 'node:sqlite';
Expand Down
4 changes: 4 additions & 0 deletions src/node_sqlite.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1849,6 +1849,10 @@ void DatabaseSync::Deserialize(const FunctionCallbackInfo<Value>& args) {
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
THROW_AND_RETURN_ON_BAD_STATE(
Comment thread
trivikr marked this conversation as resolved.

@TrevorBurnham TrevorBurnham Aug 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider changing Fixes: to Refs: in the PR body. #64795 asks for reentrant statement finalization to be prevented generally, and the sibling hole through StatementSync::Close is still an unguarded crash on this branch:

let stmt;
db.function('f', (v) => { stmt.close(); return v; });
stmt = db.prepare('SELECT f(v) AS v FROM d');
stmt.all();   // → exit 139 (SIGSEGV)

StatementSync::Close (line 2662) checks only IsFinalized(), and Symbol.dispose routes to the same path. This is pre-existing and not a regression from your diff — DatabaseSync::IsInCallback() is per-database, so it can't distinguish reentry into the running statement from the legitimate pattern of querying a different statement from a callback; that needs a per-statement stepping flag. So this seems fine to land as-is, but merging with Fixes: would auto-close #64795 while the crash is still reachable.

env,
db->IsInCallback(),
"database cannot be deserialized while in a callback");

if (!args[0]->IsUint8Array()) {
THROW_ERR_INVALID_ARG_TYPE(env->isolate(),
Expand Down
17 changes: 17 additions & 0 deletions test/parallel/test-sqlite-serialize.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,23 @@ suite('DatabaseSync.prototype.deserialize()', () => {
});
});

test('throws if called while in a callback', (t) => {
const source = new DatabaseSync(':memory:');
const serialized = source.serialize();
source.close();

const db = new DatabaseSync(':memory:');
t.after(() => db.close());
db.function('deserialize_database', () => db.deserialize(serialized));
const stmt = db.prepare('SELECT deserialize_database()');

t.assert.throws(() => stmt.get(), {
code: 'ERR_INVALID_STATE',
message: 'database cannot be deserialized while in a callback',
});
t.assert.strictEqual(db.isOpen, true);
});

test('throws if buffer argument is not a Uint8Array', (t) => {
const db = new DatabaseSync(':memory:');
t.assert.throws(() => {
Expand Down
Loading