From 25491b3469fe239d35e86f2cc1d8ed4b6575ef48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guilherme=20Ara=C3=BAjo?= Date: Tue, 15 Sep 2026 17:14:13 -0300 Subject: [PATCH] sqlite: restore connection state on reopen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit close() destroys the connection but keeps the DatabaseSync object, and open() did not replay the state held on it. An authorizer set with setAuthorizer() was not reinstalled, silently dropping a deny-all policy. Limits written through db.limits.* reverted to the constructor values. Extension loading was re-enabled from the constructor ceiling rather than the current setting, so the connection flag contradicted a previous enableLoadExtension(false), though loadExtension() itself stayed blocked by its own check. Signed-off-by: Guilherme Araújo Assisted-by: Claude Code --- src/node_sqlite.cc | 24 ++++++++++++++++------ src/node_sqlite.h | 13 ++++++------ test/parallel/test-sqlite-authz.js | 31 +++++++++++++++++++++++++++++ test/parallel/test-sqlite-limits.js | 15 ++++++++++++++ 4 files changed, 71 insertions(+), 12 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 3234d9881b51..392651f60bee 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -971,8 +971,7 @@ Intercepted DatabaseLimits::LimitsSetter( } } - sqlite3_limit( - limits->database_->Connection(), limit_info->sqlite_limit_id, new_value); + limits->database_->SetLimit(limit_info->sqlite_limit_id, new_value); return Intercepted::kYes; } @@ -1650,15 +1649,14 @@ bool Database::Open() { sqlite3_busy_timeout(connection_.get(), open_config_.get_timeout()); - // Apply initial limits for (const auto& [js_name, sqlite_limit_id] : kLimitMapping) { - const auto& limit_value = open_config_.initial_limits()[sqlite_limit_id]; + const auto& limit_value = open_config_.limits()[sqlite_limit_id]; if (limit_value.has_value()) { sqlite3_limit(connection_.get(), sqlite_limit_id, *limit_value); } } - if (allow_load_extension_) { + if (enable_load_extension_) { if (env()->permission()->enabled()) [[unlikely]] { THROW_ERR_LOAD_SQLITE_EXTENSION(env(), "Cannot load SQLite extensions when the " @@ -1677,6 +1675,15 @@ bool Database::Open() { connection_.get(), SQLITE_TRACE_PROFILE, TraceCallback, this); } + // The authorizer outlives the connection, so reopening must reinstall it. + Local authorizer = + object()->GetInternalField(kAuthorizerCallback).template As(); + if (authorizer->IsFunction()) { + r = sqlite3_set_authorizer( + connection_.get(), Database::AuthorizerCallback, this); + CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false); + } + opened = true; return true; } @@ -1724,6 +1731,11 @@ inline sqlite3* Database::Connection() { return connection_.get(); } +void Database::SetLimit(int sqlite_limit_id, int value) { + sqlite3_limit(connection_.get(), sqlite_limit_id, value); + open_config_.set_limit(sqlite_limit_id, value); +} + void Database::SetIgnoreNextSQLiteError(bool ignore) { ignore_next_sqlite_error_ = ignore; } @@ -2069,7 +2081,7 @@ void Database::New(const FunctionCallbackInfo& args) { return; } - open_config.set_initial_limit(sqlite_limit_id, limit_val); + open_config.set_limit(sqlite_limit_id, limit_val); } } } diff --git a/src/node_sqlite.h b/src/node_sqlite.h index fc2f6cc1518f..98f9beabe8f8 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -139,13 +139,13 @@ class DatabaseOpenConfiguration { inline bool get_enable_defensive() const { return defensive_; } - inline void set_initial_limit(int sqlite_limit_id, int value) { - initial_limits_.at(sqlite_limit_id) = value; + inline void set_limit(int sqlite_limit_id, int value) { + limits_.at(sqlite_limit_id) = value; } - inline const std::array, kLimitMapping.size()>& - initial_limits() const { - return initial_limits_; + inline const std::array, kLimitMapping.size()>& limits() + const { + return limits_; } private: @@ -159,7 +159,7 @@ class DatabaseOpenConfiguration { bool allow_bare_named_params_ = true; bool allow_unknown_named_params_ = false; bool defensive_ = true; - std::array, kLimitMapping.size()> initial_limits_{}; + std::array, kLimitMapping.size()> limits_{}; }; class Database; @@ -277,6 +277,7 @@ class Database : public BaseObject { return open_config_.get_allow_unknown_named_params(); } sqlite3* Connection(); + void SetLimit(int sqlite_limit_id, int value); // In some situations, such as when using custom functions, it is possible // that SQLite reports an error while JavaScript already has a pending diff --git a/test/parallel/test-sqlite-authz.js b/test/parallel/test-sqlite-authz.js index db241dd951f1..bca11396e57d 100644 --- a/test/parallel/test-sqlite-authz.js +++ b/test/parallel/test-sqlite-authz.js @@ -287,6 +287,37 @@ suite('Database.prototype.setAuthorizer()', () => { message: 'database is not open', }); }); + + it('remains installed after close() and open()', (t) => { + const db = new Database(':memory:'); + const authorizer = t.mock.fn(() => constants.SQLITE_DENY); + db.setAuthorizer(authorizer); + + assert.throws(() => { + db.exec('CREATE TABLE x (a)'); + }, { code: 'ERR_SQLITE_ERROR' }); + const callsBefore = authorizer.mock.callCount(); + assert.ok(callsBefore > 0); + + db.close(); + db.open(); + + assert.throws(() => { + db.exec('CREATE TABLE x (a)'); + }, { code: 'ERR_SQLITE_ERROR' }); + assert.ok(authorizer.mock.callCount() > callsBefore); + }); + + it('stays cleared after close() and open()', () => { + const db = new Database(':memory:'); + db.setAuthorizer(() => constants.SQLITE_DENY); + db.setAuthorizer(null); + + db.close(); + db.open(); + + db.exec('CREATE TABLE x (a)'); + }); }); // SQLite forbids an authorizer callback from modifying the connection that diff --git a/test/parallel/test-sqlite-limits.js b/test/parallel/test-sqlite-limits.js index c41cbf2e773d..2600dd027f26 100644 --- a/test/parallel/test-sqlite-limits.js +++ b/test/parallel/test-sqlite-limits.js @@ -301,4 +301,19 @@ suite('Database limits', () => { message: /too many attached databases/, }); }); + + test('limits set at runtime survive close() and open()', (t) => { + const db = new Database(':memory:'); + + db.limits.attach = 0; + db.close(); + db.open(); + + t.assert.strictEqual(db.limits.attach, 0); + t.assert.throws(() => { + db.exec("ATTACH DATABASE ':memory:' AS db1"); + }, { + message: /too many attached databases/, + }); + }); });