From 62d9d39f7889ffb668203d78599e734d2d3d36bd 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, leaving the SQL load_extension() function reachable after enableLoadExtension(false). 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 372606eb6de0..88ab50e279f9 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -968,8 +968,7 @@ Intercepted DatabaseSyncLimits::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; } @@ -1149,15 +1148,14 @@ bool DatabaseSync::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 " @@ -1176,6 +1174,15 @@ bool DatabaseSync::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(), DatabaseSync::AuthorizerCallback, this); + CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false); + } + opened = true; return true; } @@ -1223,6 +1230,11 @@ inline sqlite3* DatabaseSync::Connection() { return connection_.get(); } +void DatabaseSync::SetLimit(int sqlite_limit_id, int value) { + sqlite3_limit(connection_.get(), sqlite_limit_id, value); + open_config_.set_limit(sqlite_limit_id, value); +} + void DatabaseSync::SetIgnoreNextSQLiteError(bool ignore) { ignore_next_sqlite_error_ = ignore; } @@ -1564,7 +1576,7 @@ void DatabaseSync::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 5b91e27d5736..9a7b5b564eda 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 DatabaseSync; @@ -279,6 +279,7 @@ class DatabaseSync : 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 eda923e258b9..96aa3eeed67c 100644 --- a/test/parallel/test-sqlite-authz.js +++ b/test/parallel/test-sqlite-authz.js @@ -287,6 +287,37 @@ suite('DatabaseSync.prototype.setAuthorizer()', () => { message: 'database is not open', }); }); + + it('remains installed after close() and open()', (t) => { + const db = new DatabaseSync(':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 DatabaseSync(':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 1a038df05445..91a9a1d9d581 100644 --- a/test/parallel/test-sqlite-limits.js +++ b/test/parallel/test-sqlite-limits.js @@ -301,4 +301,19 @@ suite('DatabaseSync limits', () => { message: /too many attached databases/, }); }); + + test('limits set at runtime survive close() and open()', (t) => { + const db = new DatabaseSync(':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/, + }); + }); });