Skip to content

Commit 5ebe0eb

Browse files
araujoguiaraujogui
authored
sqlite: restore connection state on reopen
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 <[email protected]> Assisted-by: Claude Code PR-URL: #66042 Reviewed-By: Trivikram Kamat <[email protected]>
1 parent 5e3bbc4 commit 5ebe0eb

4 files changed

Lines changed: 71 additions & 12 deletions

File tree

‎src/node_sqlite.cc‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -982,8 +982,7 @@ Intercepted DatabaseLimits::LimitsSetter(
982982
}
983983
}
984984

985-
sqlite3_limit(
986-
limits->database_->Connection(), limit_info->sqlite_limit_id, new_value);
985+
limits->database_->SetLimit(limit_info->sqlite_limit_id, new_value);
987986
return Intercepted::kYes;
988987
}
989988

@@ -1676,15 +1675,14 @@ bool Database::Open() {
16761675

16771676
sqlite3_busy_timeout(connection_.get(), open_config_.get_timeout());
16781677

1679-
// Apply initial limits
16801678
for (const auto& [js_name, sqlite_limit_id] : kLimitMapping) {
1681-
const auto& limit_value = open_config_.initial_limits()[sqlite_limit_id];
1679+
const auto& limit_value = open_config_.limits()[sqlite_limit_id];
16821680
if (limit_value.has_value()) {
16831681
sqlite3_limit(connection_.get(), sqlite_limit_id, *limit_value);
16841682
}
16851683
}
16861684

1687-
if (allow_load_extension_) {
1685+
if (enable_load_extension_) {
16881686
if (env()->permission()->enabled()) [[unlikely]] {
16891687
THROW_ERR_LOAD_SQLITE_EXTENSION(env(),
16901688
"Cannot load SQLite extensions when the "
@@ -1703,6 +1701,15 @@ bool Database::Open() {
17031701
connection_.get(), SQLITE_TRACE_PROFILE, TraceCallback, this);
17041702
}
17051703

1704+
// The authorizer outlives the connection, so reopening must reinstall it.
1705+
Local<Value> authorizer =
1706+
object()->GetInternalField(kAuthorizerCallback).template As<Value>();
1707+
if (authorizer->IsFunction()) {
1708+
r = sqlite3_set_authorizer(
1709+
connection_.get(), Database::AuthorizerCallback, this);
1710+
CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false);
1711+
}
1712+
17061713
opened = true;
17071714
return true;
17081715
}
@@ -1750,6 +1757,11 @@ inline sqlite3* Database::Connection() {
17501757
return connection_.get();
17511758
}
17521759

1760+
void Database::SetLimit(int sqlite_limit_id, int value) {
1761+
sqlite3_limit(connection_.get(), sqlite_limit_id, value);
1762+
open_config_.set_limit(sqlite_limit_id, value);
1763+
}
1764+
17531765
void Database::SetIgnoreNextSQLiteError(bool ignore) {
17541766
ignore_next_sqlite_error_ = ignore;
17551767
}
@@ -2095,7 +2107,7 @@ void Database::New(const FunctionCallbackInfo<Value>& args) {
20952107
return;
20962108
}
20972109

2098-
open_config.set_initial_limit(sqlite_limit_id, limit_val);
2110+
open_config.set_limit(sqlite_limit_id, limit_val);
20992111
}
21002112
}
21012113
}

‎src/node_sqlite.h‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -139,13 +139,13 @@ class DatabaseOpenConfiguration {
139139

140140
inline bool get_enable_defensive() const { return defensive_; }
141141

142-
inline void set_initial_limit(int sqlite_limit_id, int value) {
143-
initial_limits_.at(sqlite_limit_id) = value;
142+
inline void set_limit(int sqlite_limit_id, int value) {
143+
limits_.at(sqlite_limit_id) = value;
144144
}
145145

146-
inline const std::array<std::optional<int>, kLimitMapping.size()>&
147-
initial_limits() const {
148-
return initial_limits_;
146+
inline const std::array<std::optional<int>, kLimitMapping.size()>& limits()
147+
const {
148+
return limits_;
149149
}
150150

151151
private:
@@ -159,7 +159,7 @@ class DatabaseOpenConfiguration {
159159
bool allow_bare_named_params_ = true;
160160
bool allow_unknown_named_params_ = false;
161161
bool defensive_ = true;
162-
std::array<std::optional<int>, kLimitMapping.size()> initial_limits_{};
162+
std::array<std::optional<int>, kLimitMapping.size()> limits_{};
163163
};
164164

165165
class Database;
@@ -277,6 +277,7 @@ class Database : public BaseObject {
277277
return open_config_.get_allow_unknown_named_params();
278278
}
279279
sqlite3* Connection();
280+
void SetLimit(int sqlite_limit_id, int value);
280281

281282
// In some situations, such as when using custom functions, it is possible
282283
// that SQLite reports an error while JavaScript already has a pending

‎test/parallel/test-sqlite-authz.js‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,37 @@ suite('Database.prototype.setAuthorizer()', () => {
287287
message: 'database is not open',
288288
});
289289
});
290+
291+
it('remains installed after close() and open()', (t) => {
292+
const db = new Database(':memory:');
293+
const authorizer = t.mock.fn(() => constants.SQLITE_DENY);
294+
db.setAuthorizer(authorizer);
295+
296+
assert.throws(() => {
297+
db.exec('CREATE TABLE x (a)');
298+
}, { code: 'ERR_SQLITE_ERROR' });
299+
const callsBefore = authorizer.mock.callCount();
300+
assert.ok(callsBefore > 0);
301+
302+
db.close();
303+
db.open();
304+
305+
assert.throws(() => {
306+
db.exec('CREATE TABLE x (a)');
307+
}, { code: 'ERR_SQLITE_ERROR' });
308+
assert.ok(authorizer.mock.callCount() > callsBefore);
309+
});
310+
311+
it('stays cleared after close() and open()', () => {
312+
const db = new Database(':memory:');
313+
db.setAuthorizer(() => constants.SQLITE_DENY);
314+
db.setAuthorizer(null);
315+
316+
db.close();
317+
db.open();
318+
319+
db.exec('CREATE TABLE x (a)');
320+
});
290321
});
291322

292323
// SQLite forbids an authorizer callback from modifying the connection that

‎test/parallel/test-sqlite-limits.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,4 +301,19 @@ suite('Database limits', () => {
301301
message: /too many attached databases/,
302302
});
303303
});
304+
305+
test('limits set at runtime survive close() and open()', (t) => {
306+
const db = new Database(':memory:');
307+
308+
db.limits.attach = 0;
309+
db.close();
310+
db.open();
311+
312+
t.assert.strictEqual(db.limits.attach, 0);
313+
t.assert.throws(() => {
314+
db.exec("ATTACH DATABASE ':memory:' AS db1");
315+
}, {
316+
message: /too many attached databases/,
317+
});
318+
});
304319
});

0 commit comments

Comments
 (0)