Skip to content

Commit 62d9d39

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 c1e2478 commit 62d9d39

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
@@ -968,8 +968,7 @@ Intercepted DatabaseSyncLimits::LimitsSetter(
968968
}
969969
}
970970

971-
sqlite3_limit(
972-
limits->database_->Connection(), limit_info->sqlite_limit_id, new_value);
971+
limits->database_->SetLimit(limit_info->sqlite_limit_id, new_value);
973972
return Intercepted::kYes;
974973
}
975974

@@ -1149,15 +1148,14 @@ bool DatabaseSync::Open() {
11491148

11501149
sqlite3_busy_timeout(connection_.get(), open_config_.get_timeout());
11511150

1152-
// Apply initial limits
11531151
for (const auto& [js_name, sqlite_limit_id] : kLimitMapping) {
1154-
const auto& limit_value = open_config_.initial_limits()[sqlite_limit_id];
1152+
const auto& limit_value = open_config_.limits()[sqlite_limit_id];
11551153
if (limit_value.has_value()) {
11561154
sqlite3_limit(connection_.get(), sqlite_limit_id, *limit_value);
11571155
}
11581156
}
11591157

1160-
if (allow_load_extension_) {
1158+
if (enable_load_extension_) {
11611159
if (env()->permission()->enabled()) [[unlikely]] {
11621160
THROW_ERR_LOAD_SQLITE_EXTENSION(env(),
11631161
"Cannot load SQLite extensions when the "
@@ -1176,6 +1174,15 @@ bool DatabaseSync::Open() {
11761174
connection_.get(), SQLITE_TRACE_PROFILE, TraceCallback, this);
11771175
}
11781176

1177+
// The authorizer outlives the connection, so reopening must reinstall it.
1178+
Local<Value> authorizer =
1179+
object()->GetInternalField(kAuthorizerCallback).template As<Value>();
1180+
if (authorizer->IsFunction()) {
1181+
r = sqlite3_set_authorizer(
1182+
connection_.get(), DatabaseSync::AuthorizerCallback, this);
1183+
CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false);
1184+
}
1185+
11791186
opened = true;
11801187
return true;
11811188
}
@@ -1223,6 +1230,11 @@ inline sqlite3* DatabaseSync::Connection() {
12231230
return connection_.get();
12241231
}
12251232

1233+
void DatabaseSync::SetLimit(int sqlite_limit_id, int value) {
1234+
sqlite3_limit(connection_.get(), sqlite_limit_id, value);
1235+
open_config_.set_limit(sqlite_limit_id, value);
1236+
}
1237+
12261238
void DatabaseSync::SetIgnoreNextSQLiteError(bool ignore) {
12271239
ignore_next_sqlite_error_ = ignore;
12281240
}
@@ -1564,7 +1576,7 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
15641576
return;
15651577
}
15661578

1567-
open_config.set_initial_limit(sqlite_limit_id, limit_val);
1579+
open_config.set_limit(sqlite_limit_id, limit_val);
15681580
}
15691581
}
15701582
}

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 DatabaseSync;
@@ -279,6 +279,7 @@ class DatabaseSync : public BaseObject {
279279
return open_config_.get_allow_unknown_named_params();
280280
}
281281
sqlite3* Connection();
282+
void SetLimit(int sqlite_limit_id, int value);
282283

283284
// In some situations, such as when using custom functions, it is possible
284285
// 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('DatabaseSync.prototype.setAuthorizer()', () => {
287287
message: 'database is not open',
288288
});
289289
});
290+
291+
it('remains installed after close() and open()', (t) => {
292+
const db = new DatabaseSync(':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 DatabaseSync(':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('DatabaseSync 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 DatabaseSync(':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)