FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

sqlite: prevent database close during callbacks · nodejs/node@5da6eaa · GitHub

/ node Public

Commit 5da6eaa

Browse files
authored and
aduh95
committed
sqlite: prevent database close during callbacks
Co-authored-by: Asroy Cristian Sitorus <asroycristiansitorus@gmail.com> Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: #64743 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 079339a commit 5da6eaa

3 files changed

Lines changed: 92 additions & 7 deletions

File tree

‎src/node_sqlite.cc‎

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,7 @@ class CustomAggregate {
349349
Global<Function> CustomAggregate::*mptr) {
350350
CustomAggregate* self =
351351
static_cast<CustomAggregate*>(sqlite3_user_data(ctx));
352+
CallbackDepthGuard guard(self->db_);
352353
Environment* env = self->env_;
353354
Isolate* isolate = env->isolate();
354355
auto agg = self->GetAggregate(ctx);
@@ -395,12 +396,18 @@ class CustomAggregate {
395396
return;
396397
}
397398

399+
if (!self->db_->IsOpen()) {
400+
THROW_ERR_INVALID_STATE(env, "database is not open");
401+
return;
402+
}
403+
398404
agg->value.Reset(isolate, ret);
399405
}
400406

401407
static inline void xValueBase(sqlite3_context* ctx, bool is_final) {
402408
CustomAggregate* self =
403409
static_cast<CustomAggregate*>(sqlite3_user_data(ctx));
410+
CallbackDepthGuard guard(self->db_);
404411
Environment* env = self->env_;
405412
Isolate* isolate = env->isolate();
406413
auto agg = self->GetAggregate(ctx);
@@ -426,6 +433,9 @@ class CustomAggregate {
426433
.ToLocal(&result)) {
427434
self->db_->SetIgnoreNextSQLiteError(true);
428435
sqlite3_result_error(ctx, "", 0);
436+
} else if (!self->db_->IsOpen()) {
437+
THROW_ERR_INVALID_STATE(env, "database is not open");
438+
return;
429439
}
430440
} else {
431441
result = Local<Value>::New(isolate, agg->value);
@@ -457,6 +467,10 @@ class CustomAggregate {
457467
auto fn = start_v.As<Function>();
458468
MaybeLocal<Value> retval =
459469
fn->Call(env_->context(), Null(isolate), 0, nullptr);
470+
if (!db_->IsOpen()) {
471+
THROW_ERR_INVALID_STATE(env_, "database is not open");
472+
return nullptr;
473+
}
460474
if (!retval.ToLocal(&start_v)) {
461475
db_->SetIgnoreNextSQLiteError(true);
462476
sqlite3_result_error(ctx, "", 0);
@@ -669,6 +683,7 @@ void UserDefinedFunction::xFunc(sqlite3_context* ctx,
669683
sqlite3_value** argv) {
670684
UserDefinedFunction* self =
671685
static_cast<UserDefinedFunction*>(sqlite3_user_data(ctx));
686+
CallbackDepthGuard guard(self->db_);
672687
Environment* env = self->env_;
673688
Isolate* isolate = env->isolate();
674689
auto recv = Undefined(isolate);
@@ -700,6 +715,12 @@ void UserDefinedFunction::xFunc(sqlite3_context* ctx,
700715

701716
MaybeLocal<Value> retval =
702717
fn->Call(env->context(), recv, argc, js_argv.data());
718+
719+
if (!self->db_->IsOpen()) {
720+
THROW_ERR_INVALID_STATE(env, "database is not open");
721+
return;
722+
}
723+
703724
Local<Value> result;
704725
if (!retval.ToLocal(&result)) {
705726
// Ignore the SQLite error because a JavaScript exception is pending.
@@ -1433,6 +1454,8 @@ void DatabaseSync::Close(const FunctionCallbackInfo<Value>& args) {
14331454
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
14341455
Environment* env = Environment::GetCurrent(args);
14351456
THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
1457+
THROW_AND_RETURN_ON_BAD_STATE(
1458+
env, db->IsInCallback(), "database cannot be closed while in a callback");
14361459
db->FinalizeStatements();
14371460
db->DeleteSessions();
14381461
int r = sqlite3_close_v2(db->connection_);
@@ -2381,13 +2404,17 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo<Value>& args) {
23812404
BaseObjectPtr<DatabaseSync> guard(db);
23822405

23832406
ArrayBufferViewContents<uint8_t> buf(args[0]);
2384-
int r = sqlite3changeset_apply(
2385-
db->connection_,
2386-
buf.length(),
2387-
const_cast<void*>(static_cast<const void*>(buf.data())),
2388-
context.filterCallback ? xFilter : nullptr,
2389-
xConflict,
2390-
static_cast<void*>(&context));
2407+
int r;
2408+
{
2409+
CallbackDepthGuard guard(db);
2410+
r = sqlite3changeset_apply(
2411+
db->connection_,
2412+
buf.length(),
2413+
const_cast<void*>(static_cast<const void*>(buf.data())),
2414+
context.filterCallback ? xFilter : nullptr,
2415+
xConflict,
2416+
static_cast<void*>(&context));
2417+
}
23912418
if (r == SQLITE_OK) {
23922419
args.GetReturnValue().Set(true);
23932420
return;
@@ -2522,6 +2549,7 @@ int DatabaseSync::AuthorizerCallback(void* user_data,
25222549
const char* param3,
25232550
const char* param4) {
25242551
DatabaseSync* db = static_cast<DatabaseSync*>(user_data);
2552+
CallbackDepthGuard guard(db);
25252553
Environment* env = db->env();
25262554
Isolate* isolate = env->isolate();
25272555
HandleScope handle_scope(isolate);

‎src/node_sqlite.h‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,10 @@ class DatabaseSync : public BaseObject {
229229
void SetIgnoreNextSQLiteError(bool ignore);
230230
bool ShouldIgnoreSQLiteError();
231231

232+
void IncrementCallbackDepth() { ++callback_depth_; }
233+
void DecrementCallbackDepth() { --callback_depth_; }
234+
bool IsInCallback() const { return callback_depth_ > 0; }
235+
232236
SET_MEMORY_INFO_NAME(DatabaseSync)
233237
SET_SELF_SIZE(DatabaseSync)
234238

@@ -242,6 +246,7 @@ class DatabaseSync : public BaseObject {
242246
bool enable_load_extension_;
243247
sqlite3* connection_;
244248
bool ignore_next_sqlite_error_;
249+
int callback_depth_ = 0;
245250

246251
std::set<BackupJob*> backups_;
247252
std::unordered_set<Session*> sessions_;
@@ -401,6 +406,19 @@ class SQLTagStore : public BaseObject {
401406
friend class StatementExecutionHelper;
402407
};
403408

409+
class CallbackDepthGuard {
410+
public:
411+
explicit CallbackDepthGuard(DatabaseSync* db) : db_(db) {
412+
db_->IncrementCallbackDepth();
413+
}
414+
~CallbackDepthGuard() { db_->DecrementCallbackDepth(); }
415+
CallbackDepthGuard(const CallbackDepthGuard&) = delete;
416+
CallbackDepthGuard& operator=(const CallbackDepthGuard&) = delete;
417+
418+
private:
419+
DatabaseSync* db_;
420+
};
421+
404422
class UserDefinedFunction {
405423
public:
406424
UserDefinedFunction(Environment* env,

‎test/parallel/test-sqlite-udf-close.js‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
'use strict';
2+
3+
const { skipIfSQLiteMissing } = require('../common');
4+
skipIfSQLiteMissing();
5+
const assert = require('node:assert');
6+
const { test } = require('node:test');
7+
const { DatabaseSync } = require('node:sqlite');
8+
9+
for (const method of ['all', 'get', 'run', 'iterate']) {
10+
test(`database.close() from a UDF during statement.${method}()`, () => {
11+
const db = new DatabaseSync(':memory:');
12+
db.exec(`
13+
CREATE TABLE data (value INTEGER);
14+
INSERT INTO data VALUES (1), (2), (3);
15+
`);
16+
17+
db.function('close_db', (value) => {
18+
db.close();
19+
return value;
20+
});
21+
22+
const statement = db.prepare('SELECT close_db(value) FROM data');
23+
assert.throws(() => {
24+
if (method === 'iterate') {
25+
for (const row of statement.iterate()) {
26+
assert.ok(row);
27+
}
28+
} else {
29+
statement[method]();
30+
}
31+
}, {
32+
code: 'ERR_INVALID_STATE',
33+
message: 'database cannot be closed while in a callback',
34+
});
35+
36+
assert.strictEqual(db.isOpen, true);
37+
db.close();
38+
});
39+
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL