Skip to content

Commit 28b9ebf

Browse files
committed
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, leaving the SQL load_extension() function reachable after enableLoadExtension(false). Signed-off-by: Guilherme Araújo <arauujogui@gmail.com> Assisted-by: Claude Code
1 parent 1f26576 commit 28b9ebf

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
@@ -971,8 +971,7 @@ Intercepted DatabaseLimits::LimitsSetter(
971971
}
972972
}
973973

974-
sqlite3_limit(
975-
limits->database_->Connection(), limit_info->sqlite_limit_id, new_value);
974+
limits->database_->SetLimit(limit_info->sqlite_limit_id, new_value);
976975
return Intercepted::kYes;
977976
}
978977

@@ -1650,15 +1649,14 @@ bool Database::Open() {
16501649

16511650
sqlite3_busy_timeout(connection_.get(), open_config_.get_timeout());
16521651

1653-
// Apply initial limits
16541652
for (const auto& [js_name, sqlite_limit_id] : kLimitMapping) {
1655-
const auto& limit_value = open_config_.initial_limits()[sqlite_limit_id];
1653+
const auto& limit_value = open_config_.limits()[sqlite_limit_id];
16561654
if (limit_value.has_value()) {
16571655
sqlite3_limit(connection_.get(), sqlite_limit_id, *limit_value);
16581656
}
16591657
}
16601658

1661-
if (allow_load_extension_) {
1659+
if (enable_load_extension_) {
16621660
if (env()->permission()->enabled()) [[unlikely]] {
16631661
THROW_ERR_LOAD_SQLITE_EXTENSION(env(),
16641662
"Cannot load SQLite extensions when the "
@@ -1677,6 +1675,15 @@ bool Database::Open() {
16771675
connection_.get(), SQLITE_TRACE_PROFILE, TraceCallback, this);
16781676
}
16791677

1678+
// The authorizer outlives the connection, so reopening must reinstall it.
1679+
Local<Value> authorizer =
1680+
object()->GetInternalField(kAuthorizerCallback).template As<Value>();
1681+
if (authorizer->IsFunction()) {
1682+
r = sqlite3_set_authorizer(
1683+
connection_.get(), Database::AuthorizerCallback, this);
1684+
CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false);
1685+
}
1686+
16801687
opened = true;
16811688
return true;
16821689
}
@@ -1724,6 +1731,11 @@ inline sqlite3* Database::Connection() {
17241731
return connection_.get();
17251732
}
17261733

1734+
void Database::SetLimit(int sqlite_limit_id, int value) {
1735+
sqlite3_limit(connection_.get(), sqlite_limit_id, value);
1736+
open_config_.set_limit(sqlite_limit_id, value);
1737+
}
1738+
17271739
void Database::SetIgnoreNextSQLiteError(bool ignore) {
17281740
ignore_next_sqlite_error_ = ignore;
17291741
}
@@ -2069,7 +2081,7 @@ void Database::New(const FunctionCallbackInfo<Value>& args) {
20692081
return;
20702082
}
20712083

2072-
open_config.set_initial_limit(sqlite_limit_id, limit_val);
2084+
open_config.set_limit(sqlite_limit_id, limit_val);
20732085
}
20742086
}
20752087
}

‎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)