From 528d65a472b8be5df0fe1e0f346afa337941b429 Mon Sep 17 00:00:00 2001 From: Lazizbek Ergashev Date: Thu, 27 Aug 2026 18:51:46 +0500 Subject: [PATCH 1/2] sqlite: validate connection after reading options Signed-off-by: Lazizbek Ergashev --- src/node_sqlite.cc | 14 ++++ .../test-sqlite-close-from-options-getter.js | 82 +++++++++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 test/parallel/test-sqlite-close-from-options-getter.js diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 05c4904fdd9e..969e8f44856c 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1908,6 +1908,9 @@ void DatabaseSync::CustomFunction(const FunctionCallbackInfo& args) { argc = js_len.As()->Value(); } + THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db); + UserDefinedFunction* user_data = new UserDefinedFunction( env, fn, BaseObjectWeakPtr(db), use_bigint_args); int text_rep = SQLITE_UTF8; @@ -2070,6 +2073,12 @@ void DatabaseSync::Deserialize(const FunctionCallbackInfo& args) { } } + THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); + THROW_AND_RETURN_ON_BAD_STATE( + env, + db->IsInCallback(), + "database cannot be deserialized while in a callback"); + // sqlite3_malloc64 is required because SQLITE_DESERIALIZE_FREEONCLOSE // transfers ownership to SQLite, which calls sqlite3_free() on close. // See: https://www.sqlite.org/c3ref/deserialize.html @@ -2229,6 +2238,9 @@ void DatabaseSync::AggregateFunction(const FunctionCallbackInfo& args) { argc = std::max({argc, js_len.As()->Value() - 1, 0}); } + THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db); + int text_rep = SQLITE_UTF8; if (direct_only) { text_rep |= SQLITE_DIRECTONLY; @@ -2441,6 +2453,8 @@ void Backup(const FunctionCallbackInfo& args) { } } + THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); + Local resolver; if (!Promise::Resolver::New(env->context()).ToLocal(&resolver)) { return; diff --git a/test/parallel/test-sqlite-close-from-options-getter.js b/test/parallel/test-sqlite-close-from-options-getter.js new file mode 100644 index 000000000000..e14b9d00b0bb --- /dev/null +++ b/test/parallel/test-sqlite-close-from-options-getter.js @@ -0,0 +1,82 @@ +'use strict'; + +const { skipIfSQLiteMissing } = require('../common'); +skipIfSQLiteMissing(); +const tmpdir = require('../common/tmpdir'); +const assert = require('node:assert'); +const { join } = require('node:path'); +const { test } = require('node:test'); +const { backup, DatabaseSync } = require('node:sqlite'); + +tmpdir.refresh(); + +const closedError = { + code: 'ERR_INVALID_STATE', + message: 'database is not open', +}; + +// Reading the options bag runs a user getter, so the connection validated on +// entry can already be closed by the time the method reaches SQLite. +test('function() with a getter that closes the database', () => { + const db = new DatabaseSync(':memory:'); + const options = { + get useBigIntArguments() { + db.close(); + return false; + }, + }; + + assert.throws(() => { + db.function('custom', options, () => 1); + }, closedError); +}); + +test('aggregate() with a getter that closes the database', () => { + const db = new DatabaseSync(':memory:'); + const options = { + start: 0, + step: (acc, value) => acc + value, + get useBigIntArguments() { + db.close(); + return false; + }, + }; + + assert.throws(() => { + db.aggregate('custom', options); + }, closedError); +}); + +test('deserialize() with a getter that closes the database', () => { + const source = new DatabaseSync(':memory:'); + source.exec('CREATE TABLE data (value INTEGER)'); + const serialized = source.serialize(); + source.close(); + + const db = new DatabaseSync(':memory:'); + const options = { + get dbName() { + db.close(); + return 'main'; + }, + }; + + assert.throws(() => { + db.deserialize(serialized, options); + }, closedError); +}); + +test('backup() with a getter that closes the database', () => { + const db = new DatabaseSync(':memory:'); + db.exec('CREATE TABLE data (value INTEGER)'); + const options = { + get rate() { + db.close(); + return 1; + }, + }; + + assert.throws(() => { + backup(db, join(tmpdir.path, 'backup.db'), options); + }, closedError); +}); From 3e733a730529b79868c6e061e0ae7d73ee551ad9 Mon Sep 17 00:00:00 2001 From: Lazizbek Ergashev Date: Thu, 27 Aug 2026 19:32:11 +0500 Subject: [PATCH 2/2] sqlite: guard prepare, applyChangeset and tag store Signed-off-by: Lazizbek Ergashev --- src/node_sqlite.cc | 18 +++++++ .../test-sqlite-close-from-options-getter.js | 52 +++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 969e8f44856c..bc6015926b37 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1718,6 +1718,10 @@ void DatabaseSync::Prepare(const FunctionCallbackInfo& args) { } } + // Reading the options bag can run a getter that closes the connection. + THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db); + Utf8Value sql(env->isolate(), args[0].As()); sqlite3_stmt* s = nullptr; @@ -1908,6 +1912,7 @@ void DatabaseSync::CustomFunction(const FunctionCallbackInfo& args) { argc = js_len.As()->Value(); } + // Reading the options bag can run a getter that closes the connection. THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db); @@ -2073,6 +2078,7 @@ void DatabaseSync::Deserialize(const FunctionCallbackInfo& args) { } } + // Reading the options bag can run a getter that closes the connection. THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); THROW_AND_RETURN_ON_BAD_STATE( env, @@ -2238,6 +2244,7 @@ void DatabaseSync::AggregateFunction(const FunctionCallbackInfo& args) { argc = std::max({argc, js_len.As()->Value() - 1, 0}); } + // Reading the options bag can run a getter that closes the connection. THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db); @@ -2453,6 +2460,7 @@ void Backup(const FunctionCallbackInfo& args) { } } + // Reading the options bag can run a getter that closes the connection. THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); Local resolver; @@ -2598,6 +2606,10 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo& args) { } } + // Reading the options bag can run a getter that closes the connection. + THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); + THROW_AND_RETURN_IF_IN_AUTHORIZER(env, db); + // Keep the database alive during sqlite3changeset_apply(), which may // call conflict or filter callbacks that trigger JavaScript execution. // If the JavaScript callback drops all references to the database, @@ -3982,6 +3994,12 @@ BaseObjectPtr SQLTagStore::PrepareStatement( } } + // Reading the template parts can run a getter that closes the connection. + if (!session->database_->IsOpen()) { + THROW_ERR_INVALID_STATE(env, "database is not open"); + return BaseObjectPtr(); + } + BaseObjectPtr stmt = nullptr; if (session->sql_tags_.Exists(sql)) { stmt = session->sql_tags_.Get(sql); diff --git a/test/parallel/test-sqlite-close-from-options-getter.js b/test/parallel/test-sqlite-close-from-options-getter.js index e14b9d00b0bb..d2fcc669d167 100644 --- a/test/parallel/test-sqlite-close-from-options-getter.js +++ b/test/parallel/test-sqlite-close-from-options-getter.js @@ -66,6 +66,58 @@ test('deserialize() with a getter that closes the database', () => { }, closedError); }); +test('prepare() with a getter that closes the database', () => { + const db = new DatabaseSync(':memory:'); + const options = { + get persistent() { + db.close(); + return false; + }, + }; + + assert.throws(() => { + db.prepare('SELECT 1', options); + }, closedError); +}); + +test('applyChangeset() with a getter that closes the database', () => { + const source = new DatabaseSync(':memory:'); + source.exec('CREATE TABLE data (value INTEGER PRIMARY KEY)'); + const session = source.createSession(); + source.exec('INSERT INTO data VALUES (1)'); + const changeset = session.changeset(); + source.close(); + + const db = new DatabaseSync(':memory:'); + db.exec('CREATE TABLE data (value INTEGER PRIMARY KEY)'); + const options = { + get onConflict() { + db.close(); + return undefined; + }, + }; + + assert.throws(() => { + db.applyChangeset(changeset, options); + }, closedError); +}); + +test('tag store with a getter that closes the database', () => { + const db = new DatabaseSync(':memory:'); + const sql = db.createTagStore(10); + const strings = ['SELECT ', '']; + Object.defineProperty(strings, 0, { + get() { + db.close(); + return 'SELECT '; + }, + }); + + assert.throws(() => { + sql.get(strings, 1); + }, closedError); +}); + test('backup() with a getter that closes the database', () => { const db = new DatabaseSync(':memory:'); db.exec('CREATE TABLE data (value INTEGER)');