diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 41b0094a606b..cea02baefda4 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -2661,12 +2661,17 @@ void StatementSync::Close(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE( env, stmt->IsFinalized(), "statement has been finalized"); + THROW_AND_RETURN_ON_BAD_STATE( + env, stmt->IsStepping(), "statement is currently being executed"); stmt->Close(); } void StatementSync::Dispose(const FunctionCallbackInfo& args) { StatementSync* stmt; ASSIGN_OR_RETURN_UNWRAP(&stmt, args.This()); + Environment* env = Environment::GetCurrent(args); + THROW_AND_RETURN_ON_BAD_STATE( + env, stmt->IsStepping(), "statement is currently being executed"); stmt->Close(); } @@ -3111,7 +3116,10 @@ void StatementSync::All(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE( env, stmt->IsFinalized(), "statement has been finalized"); + THROW_AND_RETURN_ON_BAD_STATE( + env, stmt->IsStepping(), "statement is currently being executed"); Isolate* isolate = env->isolate(); + auto stepping = stmt->MarkStepping(); int r = stmt->ResetStatement(); CHECK_ERROR_OR_THROW(isolate, stmt->db_.get(), r, SQLITE_OK, void()); @@ -3138,6 +3146,9 @@ void StatementSync::Iterate(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE( env, stmt->IsFinalized(), "statement has been finalized"); + THROW_AND_RETURN_ON_BAD_STATE( + env, stmt->IsStepping(), "statement is currently being executed"); + auto stepping = stmt->MarkStepping(); int r = stmt->ResetStatement(); CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void()); @@ -3161,6 +3172,9 @@ void StatementSync::Get(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE( env, stmt->IsFinalized(), "statement has been finalized"); + THROW_AND_RETURN_ON_BAD_STATE( + env, stmt->IsStepping(), "statement is currently being executed"); + auto stepping = stmt->MarkStepping(); int r = stmt->ResetStatement(); CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void()); @@ -3185,6 +3199,9 @@ void StatementSync::Run(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE( env, stmt->IsFinalized(), "statement has been finalized"); + THROW_AND_RETURN_ON_BAD_STATE( + env, stmt->IsStepping(), "statement is currently being executed"); + auto stepping = stmt->MarkStepping(); int r = stmt->ResetStatement(); CHECK_ERROR_OR_THROW(env->isolate(), stmt->db_.get(), r, SQLITE_OK, void()); @@ -3474,6 +3491,9 @@ void SQLTagStore::Run(const FunctionCallbackInfo& args) { return; } + THROW_AND_RETURN_ON_BAD_STATE( + env, stmt->IsStepping(), "statement is currently being executed"); + auto stepping = stmt->MarkStepping(); if (!ResetAndBindStatement(env, stmt.get(), args)) { return; } @@ -3500,6 +3520,9 @@ void SQLTagStore::Iterate(const FunctionCallbackInfo& args) { return; } + THROW_AND_RETURN_ON_BAD_STATE( + env, stmt->IsStepping(), "statement is currently being executed"); + auto stepping = stmt->MarkStepping(); if (!ResetAndBindStatement(env, stmt.get(), args)) { return; } @@ -3528,6 +3551,9 @@ void SQLTagStore::Get(const FunctionCallbackInfo& args) { return; } + THROW_AND_RETURN_ON_BAD_STATE( + env, stmt->IsStepping(), "statement is currently being executed"); + auto stepping = stmt->MarkStepping(); if (!ResetAndBindStatement(env, stmt.get(), args)) { return; } @@ -3557,6 +3583,9 @@ void SQLTagStore::All(const FunctionCallbackInfo& args) { return; } + THROW_AND_RETURN_ON_BAD_STATE( + env, stmt->IsStepping(), "statement is currently being executed"); + auto stepping = stmt->MarkStepping(); if (!ResetAndBindStatement(env, stmt.get(), args)) { return; } @@ -3769,6 +3798,8 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE( env, iter->stmt_->IsFinalized(), "statement has been finalized"); + THROW_AND_RETURN_ON_BAD_STATE( + env, iter->stmt_->IsStepping(), "statement is currently being executed"); Isolate* isolate = env->isolate(); auto iter_template = getLazyIterTemplate(env); @@ -3791,6 +3822,7 @@ void StatementSyncIterator::Next(const FunctionCallbackInfo& args) { iter->statement_reset_generation_ != iter->stmt_->reset_generation_, "iterator was invalidated"); + auto stepping = iter->stmt_->MarkStepping(); int r = sqlite3_step(iter->stmt_->statement_); if (r != SQLITE_ROW) { CHECK_ERROR_OR_THROW( @@ -3846,6 +3878,8 @@ void StatementSyncIterator::Return(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); THROW_AND_RETURN_ON_BAD_STATE( env, iter->stmt_->IsFinalized(), "statement has been finalized"); + THROW_AND_RETURN_ON_BAD_STATE( + env, iter->stmt_->IsStepping(), "statement is currently being executed"); Isolate* isolate = env->isolate(); sqlite3_reset(iter->stmt_->statement_); diff --git a/src/node_sqlite.h b/src/node_sqlite.h index 675595e55025..ba2d62d41a96 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -291,6 +291,19 @@ class StatementSync : public BaseObject { bool GetCachedColumnNames(v8::LocalVector* keys); void Finalize(); bool IsFinalized(); + bool IsStepping() const { return stepping_; } + + // SQLite forbids stepping, resetting, or finalizing a statement while that + // same statement's user-defined function callback is on the stack. The + // callback depth tracked by DatabaseSync is per-database, so it cannot + // distinguish reentry into the running statement from the common pattern of + // querying a *different* statement from a callback. This flag marks the + // statement that is currently being stepped so that only the former is + // rejected. + inline auto MarkStepping() { + stepping_ = true; + return OnScopeLeave([this]() { stepping_ = false; }); + } SET_MEMORY_INFO_NAME(StatementSync) SET_SELF_SIZE(StatementSync) @@ -304,6 +317,7 @@ class StatementSync : public BaseObject { bool use_big_ints_; bool allow_bare_named_params_; bool allow_unknown_named_params_; + bool stepping_ = false; uint64_t reset_generation_ = 0; std::optional> bare_named_params_; inline int ResetStatement(); diff --git a/test/parallel/test-sqlite-udf-statement-reentry.js b/test/parallel/test-sqlite-udf-statement-reentry.js new file mode 100644 index 000000000000..8e8b755d5dc2 --- /dev/null +++ b/test/parallel/test-sqlite-udf-statement-reentry.js @@ -0,0 +1,229 @@ +'use strict'; + +const { skipIfSQLiteMissing, mustCall } = require('../common'); +skipIfSQLiteMissing(); +const assert = require('node:assert'); +const { suite, test } = require('node:test'); +const { DatabaseSync } = require('node:sqlite'); + +const reentryError = { + code: 'ERR_INVALID_STATE', + message: 'statement is currently being executed', +}; + +function newDbWithRows() { + const db = new DatabaseSync(':memory:'); + db.exec(` + CREATE TABLE data (value INTEGER); + INSERT INTO data VALUES (1), (2), (3); + `); + return db; +} + +suite('reentry into the running statement is rejected', () => { + for (const method of ['all', 'get', 'run']) { + test(`statement.${method}() from its own UDF`, () => { + const db = newDbWithRows(); + let statement; + db.function('reenter', mustCall((value) => { + assert.throws(() => statement[method](), reentryError); + return value; + })); + + statement = db.prepare('SELECT reenter(value) AS value FROM data LIMIT 1'); + assert.deepStrictEqual(statement.get(), { __proto__: null, value: 1 }); + assert.strictEqual(db.isOpen, true); + }); + } + + test('iterator next() from its own UDF', () => { + const db = newDbWithRows(); + let iterator; + db.function('reenter', mustCall((value) => { + assert.throws(() => iterator.next(), reentryError); + return value; + }, 3)); + + iterator = db.prepare('SELECT reenter(value) AS value FROM data').iterate(); + assert.deepStrictEqual([...iterator].map((row) => row.value), [1, 2, 3]); + assert.strictEqual(db.isOpen, true); + }); + + test('iterator return() from its own UDF', () => { + const db = newDbWithRows(); + let iterator; + db.function('reenter', mustCall((value) => { + assert.throws(() => iterator.return(), reentryError); + return value; + })); + + iterator = db.prepare('SELECT reenter(value) AS value FROM data').iterate(); + assert.strictEqual(iterator.next().done, false); + iterator.return(); + assert.strictEqual(db.isOpen, true); + }); + + test( + 'recursive get() reports the reentry rather than overflowing the stack', + () => { + const db = new DatabaseSync(':memory:'); + let statement; + db.function('reenter', mustCall(() => { + assert.throws(() => statement.get(), reentryError); + return 1; + })); + + statement = db.prepare('SELECT reenter() AS value'); + assert.deepStrictEqual(statement.get(), { __proto__: null, value: 1 }); + }); + + for (const method of ['all', 'get', 'run', 'iterate']) { + test(`${method}() reentry during parameter binding is rejected`, () => { + const db = newDbWithRows(); + let statement; + const invoke = (params) => (method === 'iterate' ? + [...statement.iterate(params)] : + statement[method](params)); + const reenter = mustCall(() => { + assert.throws(() => invoke({ $min: 2 }), reentryError); + return 1; + }); + const params = { get $min() { return reenter(); } }; + + statement = db.prepare('SELECT value FROM data WHERE value >= $min'); + invoke(params); + }); + } + + test('statement.close() from its own UDF', () => { + const db = newDbWithRows(); + let statement; + db.function('reenter', mustCall((value) => { + assert.throws(() => statement.close(), reentryError); + return value; + })); + + statement = db.prepare('SELECT reenter(value) AS value FROM data LIMIT 1'); + assert.deepStrictEqual(statement.get(), { __proto__: null, value: 1 }); + statement.close(); + }); + + test('statement[Symbol.dispose]() from its own UDF', () => { + const db = newDbWithRows(); + let statement; + db.function('reenter', mustCall((value) => { + assert.throws(() => statement[Symbol.dispose](), reentryError); + return value; + })); + + statement = db.prepare('SELECT reenter(value) AS value FROM data LIMIT 1'); + assert.deepStrictEqual(statement.get(), { __proto__: null, value: 1 }); + statement[Symbol.dispose](); + }); + + test('statement is usable again after the callback returns', () => { + const db = newDbWithRows(); + let statement; + db.function('reenter', mustCall((value) => { + assert.throws(() => statement.all(), reentryError); + return value; + }, 2)); + + statement = db.prepare('SELECT reenter(value) AS value FROM data LIMIT 1'); + assert.deepStrictEqual(statement.all(), [{ __proto__: null, value: 1 }]); + assert.deepStrictEqual(statement.all(), [{ __proto__: null, value: 1 }]); + }); +}); + +suite('a different statement remains usable from a callback', () => { + test('the lookup pattern still works', () => { + const db = newDbWithRows(); + db.exec('CREATE TABLE names (value INTEGER, name TEXT);' + + "INSERT INTO names VALUES (1, 'one'), (2, 'two'), (3, 'three');"); + const lookup = db.prepare('SELECT name FROM names WHERE value = ?'); + + db.function('name_of', mustCall((value) => lookup.get(value).name, 3)); + + assert.deepStrictEqual( + db.prepare('SELECT name_of(value) AS name FROM data').all(), + [ + { __proto__: null, name: 'one' }, + { __proto__: null, name: 'two' }, + { __proto__: null, name: 'three' }, + ], + ); + }); + + test('a nested iterator over a different statement still works', () => { + const db = newDbWithRows(); + const inner = db.prepare('SELECT value FROM data'); + + db.function('sum_all', mustCall(() => { + let total = 0; + for (const row of inner.iterate()) { + total += row.value; + } + return total; + })); + + assert.deepStrictEqual( + db.prepare('SELECT sum_all() AS total LIMIT 1').get(), + { __proto__: null, total: 6 }, + ); + }); +}); + +suite('SQL tag store reentry is rejected', () => { + for (const method of ['all', 'get', 'run']) { + test(`tag store ${method} re-executing the same tag`, () => { + const db = newDbWithRows(); + const sql = db.createTagStore(4); + db.function('reenter', mustCall((value) => { + assert.throws( + () => sql[method]`SELECT reenter(value) AS value FROM data LIMIT 1`, + reentryError, + ); + return value; + })); + + assert.deepStrictEqual( + sql.get`SELECT reenter(value) AS value FROM data LIMIT 1`, + { __proto__: null, value: 1 }, + ); + assert.strictEqual(db.isOpen, true); + }); + } +}); + +suite('aggregate functions', () => { + test('reentry from an aggregate step is rejected', () => { + const db = newDbWithRows(); + let statement; + db.aggregate('reenter_agg', { + start: 0, + step: mustCall((total, value) => { + assert.throws(() => statement.get(), reentryError); + return total + value; + }, 3), + }); + + statement = db.prepare('SELECT reenter_agg(value) AS total FROM data'); + assert.deepStrictEqual(statement.get(), { __proto__: null, total: 6 }); + }); + + test('reentry from an aggregate result is rejected', () => { + const db = newDbWithRows(); + let statement; + db.aggregate('reenter_result', { + start: 0, + step: (total, value) => total + value, + result: mustCall((total) => { + assert.throws(() => statement.get(), reentryError); + return total; + }), + }); + + statement = db.prepare('SELECT reenter_result(value) AS total FROM data'); + assert.deepStrictEqual(statement.get(), { __proto__: null, total: 6 }); + }); +});