mirror of
https://github.com/nodejs/node.git
synced 2026-10-28 10:15:07 +08:00
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 <arauujogui@gmail.com> Assisted-by: Claude Code PR-URL: https://github.com/nodejs/node/pull/66042 Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
This commit is contained in:
1 parent
5e3bbc4ac4
commit
5ebe0ebf65
4 files changed
+71
-12
No files matched your search
+18
-6
@@ -982,8 +982,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;
|
||||
}
|
||||
|
||||
@@ -1676,15 +1675,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 "
|
||||
@@ -1703,6 +1701,15 @@ bool Database::Open() {
|
||||
connection_.get(), SQLITE_TRACE_PROFILE, TraceCallback, this);
|
||||
}
|
||||
|
||||
// The authorizer outlives the connection, so reopening must reinstall it.
|
||||
Local<Value> authorizer =
|
||||
object()->GetInternalField(kAuthorizerCallback).template As<Value>();
|
||||
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;
|
||||
}
|
||||
@@ -1750,6 +1757,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;
|
||||
}
|
||||
@@ -2095,7 +2107,7 @@ void Database::New(const FunctionCallbackInfo<Value>& args) {
|
||||
return;
|
||||
}
|
||||
|
||||
open_config.set_initial_limit(sqlite_limit_id, limit_val);
|
||||
open_config.set_limit(sqlite_limit_id, limit_val);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+7
-6
@@ -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<std::optional<int>, kLimitMapping.size()>&
|
||||
initial_limits() const {
|
||||
return initial_limits_;
|
||||
inline const std::array<std::optional<int>, 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<std::optional<int>, kLimitMapping.size()> initial_limits_{};
|
||||
std::array<std::optional<int>, 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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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/,
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user