sqlite: reject busy statement finalization in authorizer

doc/api/sqlite.md

@@ -1128,7 +1128,7 @@ Finalizes the prepared statement. An exception is thrown if the statement is
 already finalized. An [`ERR_INVALID_STATE`][] error is thrown if this statement
 is currently executing, which happens when the method is called from a callback
 that the statement itself triggered, such as a user-defined function, an
-aggregate function, or a [`'sqlite.db.query'`][] subscriber. Other statements
+aggregate function, or a [`'sqlite.db.query'`][] subscriber. Idle statements
 on the same connection can be finalized from such a callback. This method is a
 wrapper around [`sqlite3_finalize()`][].
 

src/node_sqlite.cc

@@ -160,6 +160,15 @@ inline MaybeLocal<Value> IntegerToValue(Isolate* isolate,
  (db)->IsInAuthorizerCallback(), \
  "database cannot be accessed from an authorizer callback")
 
+// Finalizing a busy statement from an authorizer can release its locks and
+// change the outer statement's outcome.
+#define THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt) \
+ THROW_AND_RETURN_ON_BAD_STATE( \
+ (env), \
+ (stmt)->db_->IsInAuthorizerCallback() && \
+ sqlite3_stmt_busy((stmt)->statement_.get()), \
+ "database cannot be accessed from an authorizer callback")
+
 // A statement's virtual machine cannot be reentered while sqlite3_step() is
 // running it. Finalizing it frees the VM outright, and re-running it resets the
 // VM mid-execution; both are use-after-free rather than merely a contract
@@ -2900,6 +2909,7 @@ void StatementSync::Close(const FunctionCallbackInfo<Value>& args) {
  THROW_AND_RETURN_ON_BAD_STATE(
  env, stmt->IsFinalized(), "statement has been finalized");
  THROW_AND_RETURN_IF_STEPPING(env, stmt);
+ THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt);
  stmt->Close();
 }
 
@@ -2913,6 +2923,7 @@ void StatementSync::Dispose(const FunctionCallbackInfo<Value>& args) {
  return;
  }
  THROW_AND_RETURN_IF_STEPPING(env, stmt);
+ THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt);
  stmt->Close();
 }
 

src/node_sqlite.h

@@ -299,9 +299,8 @@ class DatabaseSync : public BaseObject {
  void DecrementAuthorizerDepth() { --authorizer_depth_; }
  bool IsInAuthorizerCallback() const { return authorizer_depth_ > 0; }
 
- // Finalizing a statement frees its virtual machine, so a callback that
- // SQLite invokes from inside sqlite3_step() must not finalize the statement
- // being stepped. Other statements on the connection are safe to finalize.
+ // A callback must not finalize the statement being stepped. Other statements
+ // are safe unless they are busy during an authorizer callback.
  void PushSteppingStatement(sqlite3_stmt* stmt) {
  stepping_statements_.push_back(stmt);
  }

test/parallel/test-sqlite-authz.js

@@ -397,10 +397,8 @@ suite('authorizer callback reentrancy', () => {
  assert.deepStrictEqual(runInAuthorizer(db, cases), allRejected(cases));
  });
 
- // Only the statement being stepped is unsafe to finalize. Other statements
- // on the connection have their own virtual machines, so finalizing them from
- // a callback is allowed.
- it('allows finalizing a statement that is not being executed', () => {
+ // An idle statement has no virtual-machine state or locks to release.
+ it('allows finalizing an idle statement', () => {
  const db = new DatabaseSync(':memory:');
  db.exec('CREATE TABLE t (x INTEGER)');
  db.exec('INSERT INTO t VALUES (1)');
@@ -417,6 +415,43 @@ suite('authorizer callback reentrancy', () => {
  });
  });
 
+ // A paused iterator is busy and may hold locks between sqlite3_step() calls.
+ it('rejects finalizing another active statement', () => {
+ for (const method of ['close', 'dispose']) {
+ const db = new DatabaseSync(':memory:');
+ db.exec('CREATE TABLE t (x INTEGER)');
+ db.exec('INSERT INTO t VALUES (1), (2), (3)');
+ const stmt = db.prepare('SELECT x FROM t');
+ const iter = stmt.iterate();
+ iter.next();
+ let outcome = 'authorizer callback did not run';
+
+ db.setAuthorizer((actionCode) => {
+ if (actionCode === constants.SQLITE_DROP_TABLE) {
+ try {
+ if (method === 'close') {
+ stmt.close();
+ } else {
+ stmt[Symbol.dispose]();
+ }
+ outcome = 'did not throw';
+ } catch (err) {
+ outcome = `${err.code}: ${err.message}`;
+ }
+ }
+ return constants.SQLITE_OK;
+ });
+
+ assert.throws(() => db.exec('DROP TABLE t'), {
+ code: 'ERR_SQLITE_ERROR',
+ message: 'database table is locked',
+ });
+ assert.strictEqual(outcome, expectedError);
+ db.setAuthorizer(null);
+ iter.return();
+ }
+ });
+
  // Disposal is idempotent, so a statement that is already finalized must stay
  // a no-op even inside a callback. Throwing here would turn a `using` scope's
  // real exception into a SuppressedError.

doc/api/sqlite.md

@@ -1128,7 +1128,7 @@ Finalizes the prepared statement. An exception is thrown if the statement is
 already finalized. An [`ERR_INVALID_STATE`][] error is thrown if this statement
 is currently executing, which happens when the method is called from a callback
 that the statement itself triggered, such as a user-defined function, an
-aggregate function, or a [`'sqlite.db.query'`][] subscriber. Other statements
+aggregate function, or a [`'sqlite.db.query'`][] subscriber. Idle statements
 on the same connection can be finalized from such a callback. This method is a
 wrapper around [`sqlite3_finalize()`][].
 

src/node_sqlite.cc

@@ -160,6 +160,15 @@ inline MaybeLocal<Value> IntegerToValue(Isolate* isolate,
  (db)->IsInAuthorizerCallback(), \
  "database cannot be accessed from an authorizer callback")
 
+// Finalizing a busy statement from an authorizer can release its locks and
+// change the outer statement's outcome.
+#define THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt) \
+ THROW_AND_RETURN_ON_BAD_STATE( \
+ (env), \
+ (stmt)->db_->IsInAuthorizerCallback() && \
+ sqlite3_stmt_busy((stmt)->statement_.get()), \
+ "database cannot be accessed from an authorizer callback")
+
 // A statement's virtual machine cannot be reentered while sqlite3_step() is
 // running it. Finalizing it frees the VM outright, and re-running it resets the
 // VM mid-execution; both are use-after-free rather than merely a contract
@@ -2900,6 +2909,7 @@ void StatementSync::Close(const FunctionCallbackInfo<Value>& args) {
  THROW_AND_RETURN_ON_BAD_STATE(
  env, stmt->IsFinalized(), "statement has been finalized");
  THROW_AND_RETURN_IF_STEPPING(env, stmt);
+ THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt);
  stmt->Close();
 }
 
@@ -2913,6 +2923,7 @@ void StatementSync::Dispose(const FunctionCallbackInfo<Value>& args) {
  return;
  }
  THROW_AND_RETURN_IF_STEPPING(env, stmt);
+ THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt);
  stmt->Close();
 }
 

src/node_sqlite.h

@@ -299,9 +299,8 @@ class DatabaseSync : public BaseObject {
  void DecrementAuthorizerDepth() { --authorizer_depth_; }
  bool IsInAuthorizerCallback() const { return authorizer_depth_ > 0; }
 
- // Finalizing a statement frees its virtual machine, so a callback that
- // SQLite invokes from inside sqlite3_step() must not finalize the statement
- // being stepped. Other statements on the connection are safe to finalize.
+ // A callback must not finalize the statement being stepped. Other statements
+ // are safe unless they are busy during an authorizer callback.
  void PushSteppingStatement(sqlite3_stmt* stmt) {
  stepping_statements_.push_back(stmt);
  }

test/parallel/test-sqlite-authz.js

@@ -397,10 +397,8 @@ suite('authorizer callback reentrancy', () => {
  assert.deepStrictEqual(runInAuthorizer(db, cases), allRejected(cases));
  });
 
- // Only the statement being stepped is unsafe to finalize. Other statements
- // on the connection have their own virtual machines, so finalizing them from
- // a callback is allowed.
- it('allows finalizing a statement that is not being executed', () => {
+ // An idle statement has no virtual-machine state or locks to release.
+ it('allows finalizing an idle statement', () => {
  const db = new DatabaseSync(':memory:');
  db.exec('CREATE TABLE t (x INTEGER)');
  db.exec('INSERT INTO t VALUES (1)');
@@ -417,6 +415,43 @@ suite('authorizer callback reentrancy', () => {
  });
  });
 
+ // A paused iterator is busy and may hold locks between sqlite3_step() calls.
+ it('rejects finalizing another active statement', () => {
+ for (const method of ['close', 'dispose']) {
+ const db = new DatabaseSync(':memory:');
+ db.exec('CREATE TABLE t (x INTEGER)');
+ db.exec('INSERT INTO t VALUES (1), (2), (3)');
+ const stmt = db.prepare('SELECT x FROM t');
+ const iter = stmt.iterate();
+ iter.next();
+ let outcome = 'authorizer callback did not run';
+
+ db.setAuthorizer((actionCode) => {
+ if (actionCode === constants.SQLITE_DROP_TABLE) {
+ try {
+ if (method === 'close') {
+ stmt.close();
+ } else {
+ stmt[Symbol.dispose]();
+ }
+ outcome = 'did not throw';
+ } catch (err) {
+ outcome = `${err.code}: ${err.message}`;
+ }
+ }
+ return constants.SQLITE_OK;
+ });
+
+ assert.throws(() => db.exec('DROP TABLE t'), {
+ code: 'ERR_SQLITE_ERROR',
+ message: 'database table is locked',
+ });
+ assert.strictEqual(outcome, expectedError);
+ db.setAuthorizer(null);
+ iter.return();
+ }
+ });
+
  // Disposal is idempotent, so a statement that is already finalized must stay
  // a no-op even inside a callback. Throwing here would turn a `using` scope's
  // real exception into a SuppressedError.

doc/api/sqlite.md

@@ -1128,7 +1128,7 @@ Finalizes the prepared statement. An exception is thrown if the statement is
 already finalized. An [`ERR_INVALID_STATE`][] error is thrown if this statement
 is currently executing, which happens when the method is called from a callback
 that the statement itself triggered, such as a user-defined function, an
-aggregate function, or a [`'sqlite.db.query'`][] subscriber. Other statements
+aggregate function, or a [`'sqlite.db.query'`][] subscriber. Idle statements
 on the same connection can be finalized from such a callback. This method is a
 wrapper around [`sqlite3_finalize()`][].
 

src/node_sqlite.cc

@@ -160,6 +160,15 @@ inline MaybeLocal<Value> IntegerToValue(Isolate* isolate,
  (db)->IsInAuthorizerCallback(), \
  "database cannot be accessed from an authorizer callback")
 
+// Finalizing a busy statement from an authorizer can release its locks and
+// change the outer statement's outcome.
+#define THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt) \
+ THROW_AND_RETURN_ON_BAD_STATE( \
+ (env), \
+ (stmt)->db_->IsInAuthorizerCallback() && \
+ sqlite3_stmt_busy((stmt)->statement_.get()), \
+ "database cannot be accessed from an authorizer callback")
+
 // A statement's virtual machine cannot be reentered while sqlite3_step() is
 // running it. Finalizing it frees the VM outright, and re-running it resets the
 // VM mid-execution; both are use-after-free rather than merely a contract
@@ -2900,6 +2909,7 @@ void StatementSync::Close(const FunctionCallbackInfo<Value>& args) {
  THROW_AND_RETURN_ON_BAD_STATE(
  env, stmt->IsFinalized(), "statement has been finalized");
  THROW_AND_RETURN_IF_STEPPING(env, stmt);
+ THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt);
  stmt->Close();
 }
 
@@ -2913,6 +2923,7 @@ void StatementSync::Dispose(const FunctionCallbackInfo<Value>& args) {
  return;
  }
  THROW_AND_RETURN_IF_STEPPING(env, stmt);
+ THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt);
  stmt->Close();
 }
 

src/node_sqlite.h

@@ -299,9 +299,8 @@ class DatabaseSync : public BaseObject {
  void DecrementAuthorizerDepth() { --authorizer_depth_; }
  bool IsInAuthorizerCallback() const { return authorizer_depth_ > 0; }
 
- // Finalizing a statement frees its virtual machine, so a callback that
- // SQLite invokes from inside sqlite3_step() must not finalize the statement
- // being stepped. Other statements on the connection are safe to finalize.
+ // A callback must not finalize the statement being stepped. Other statements
+ // are safe unless they are busy during an authorizer callback.
  void PushSteppingStatement(sqlite3_stmt* stmt) {
  stepping_statements_.push_back(stmt);
  }

test/parallel/test-sqlite-authz.js

@@ -397,10 +397,8 @@ suite('authorizer callback reentrancy', () => {
  assert.deepStrictEqual(runInAuthorizer(db, cases), allRejected(cases));
  });
 
- // Only the statement being stepped is unsafe to finalize. Other statements
- // on the connection have their own virtual machines, so finalizing them from
- // a callback is allowed.
- it('allows finalizing a statement that is not being executed', () => {
+ // An idle statement has no virtual-machine state or locks to release.
+ it('allows finalizing an idle statement', () => {
  const db = new DatabaseSync(':memory:');
  db.exec('CREATE TABLE t (x INTEGER)');
  db.exec('INSERT INTO t VALUES (1)');
@@ -417,6 +415,43 @@ suite('authorizer callback reentrancy', () => {
  });
  });
 
+ // A paused iterator is busy and may hold locks between sqlite3_step() calls.
+ it('rejects finalizing another active statement', () => {
+ for (const method of ['close', 'dispose']) {
+ const db = new DatabaseSync(':memory:');
+ db.exec('CREATE TABLE t (x INTEGER)');
+ db.exec('INSERT INTO t VALUES (1), (2), (3)');
+ const stmt = db.prepare('SELECT x FROM t');
+ const iter = stmt.iterate();
+ iter.next();
+ let outcome = 'authorizer callback did not run';
+
+ db.setAuthorizer((actionCode) => {
+ if (actionCode === constants.SQLITE_DROP_TABLE) {
+ try {
+ if (method === 'close') {
+ stmt.close();
+ } else {
+ stmt[Symbol.dispose]();
+ }
+ outcome = 'did not throw';
+ } catch (err) {
+ outcome = `${err.code}: ${err.message}`;
+ }
+ }
+ return constants.SQLITE_OK;
+ });
+
+ assert.throws(() => db.exec('DROP TABLE t'), {
+ code: 'ERR_SQLITE_ERROR',
+ message: 'database table is locked',
+ });
+ assert.strictEqual(outcome, expectedError);
+ db.setAuthorizer(null);
+ iter.return();
+ }
+ });
+
  // Disposal is idempotent, so a statement that is already finalized must stay
  // a no-op even inside a callback. Throwing here would turn a `using` scope's
  // real exception into a SuppressedError.

doc/api/sqlite.md

@@ -1128,7 +1128,7 @@ Finalizes the prepared statement. An exception is thrown if the statement is
 already finalized. An [`ERR_INVALID_STATE`][] error is thrown if this statement
 is currently executing, which happens when the method is called from a callback
 that the statement itself triggered, such as a user-defined function, an
-aggregate function, or a [`'sqlite.db.query'`][] subscriber. Other statements
+aggregate function, or a [`'sqlite.db.query'`][] subscriber. Idle statements
 on the same connection can be finalized from such a callback. This method is a
 wrapper around [`sqlite3_finalize()`][].
 

src/node_sqlite.cc

@@ -160,6 +160,15 @@ inline MaybeLocal<Value> IntegerToValue(Isolate* isolate,
  (db)->IsInAuthorizerCallback(), \
  "database cannot be accessed from an authorizer callback")
 
+// Finalizing a busy statement from an authorizer can release its locks and
+// change the outer statement's outcome.
+#define THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt) \
+ THROW_AND_RETURN_ON_BAD_STATE( \
+ (env), \
+ (stmt)->db_->IsInAuthorizerCallback() && \
+ sqlite3_stmt_busy((stmt)->statement_.get()), \
+ "database cannot be accessed from an authorizer callback")
+
 // A statement's virtual machine cannot be reentered while sqlite3_step() is
 // running it. Finalizing it frees the VM outright, and re-running it resets the
 // VM mid-execution; both are use-after-free rather than merely a contract
@@ -2900,6 +2909,7 @@ void StatementSync::Close(const FunctionCallbackInfo<Value>& args) {
  THROW_AND_RETURN_ON_BAD_STATE(
  env, stmt->IsFinalized(), "statement has been finalized");
  THROW_AND_RETURN_IF_STEPPING(env, stmt);
+ THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt);
  stmt->Close();
 }
 
@@ -2913,6 +2923,7 @@ void StatementSync::Dispose(const FunctionCallbackInfo<Value>& args) {
  return;
  }
  THROW_AND_RETURN_IF_STEPPING(env, stmt);
+ THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt);
  stmt->Close();
 }
 

src/node_sqlite.h

@@ -299,9 +299,8 @@ class DatabaseSync : public BaseObject {
  void DecrementAuthorizerDepth() { --authorizer_depth_; }
  bool IsInAuthorizerCallback() const { return authorizer_depth_ > 0; }
 
- // Finalizing a statement frees its virtual machine, so a callback that
- // SQLite invokes from inside sqlite3_step() must not finalize the statement
- // being stepped. Other statements on the connection are safe to finalize.
+ // A callback must not finalize the statement being stepped. Other statements
+ // are safe unless they are busy during an authorizer callback.
  void PushSteppingStatement(sqlite3_stmt* stmt) {
  stepping_statements_.push_back(stmt);
  }

test/parallel/test-sqlite-authz.js

@@ -397,10 +397,8 @@ suite('authorizer callback reentrancy', () => {
  assert.deepStrictEqual(runInAuthorizer(db, cases), allRejected(cases));
  });
 
- // Only the statement being stepped is unsafe to finalize. Other statements
- // on the connection have their own virtual machines, so finalizing them from
- // a callback is allowed.
- it('allows finalizing a statement that is not being executed', () => {
+ // An idle statement has no virtual-machine state or locks to release.
+ it('allows finalizing an idle statement', () => {
  const db = new DatabaseSync(':memory:');
  db.exec('CREATE TABLE t (x INTEGER)');
  db.exec('INSERT INTO t VALUES (1)');
@@ -417,6 +415,43 @@ suite('authorizer callback reentrancy', () => {
  });
  });
 
+ // A paused iterator is busy and may hold locks between sqlite3_step() calls.
+ it('rejects finalizing another active statement', () => {
+ for (const method of ['close', 'dispose']) {
+ const db = new DatabaseSync(':memory:');
+ db.exec('CREATE TABLE t (x INTEGER)');
+ db.exec('INSERT INTO t VALUES (1), (2), (3)');
+ const stmt = db.prepare('SELECT x FROM t');
+ const iter = stmt.iterate();
+ iter.next();
+ let outcome = 'authorizer callback did not run';
+
+ db.setAuthorizer((actionCode) => {
+ if (actionCode === constants.SQLITE_DROP_TABLE) {
+ try {
+ if (method === 'close') {
+ stmt.close();
+ } else {
+ stmt[Symbol.dispose]();
+ }
+ outcome = 'did not throw';
+ } catch (err) {
+ outcome = `${err.code}: ${err.message}`;
+ }
+ }
+ return constants.SQLITE_OK;
+ });
+
+ assert.throws(() => db.exec('DROP TABLE t'), {
+ code: 'ERR_SQLITE_ERROR',
+ message: 'database table is locked',
+ });
+ assert.strictEqual(outcome, expectedError);
+ db.setAuthorizer(null);
+ iter.return();
+ }
+ });
+
  // Disposal is idempotent, so a statement that is already finalized must stay
  // a no-op even inside a callback. Throwing here would turn a `using` scope's
  // real exception into a SuppressedError.

doc/api/sqlite.md

@@ -1128,7 +1128,7 @@ Finalizes the prepared statement. An exception is thrown if the statement is
 already finalized. An [`ERR_INVALID_STATE`][] error is thrown if this statement
 is currently executing, which happens when the method is called from a callback
 that the statement itself triggered, such as a user-defined function, an
-aggregate function, or a [`'sqlite.db.query'`][] subscriber. Other statements
+aggregate function, or a [`'sqlite.db.query'`][] subscriber. Idle statements
 on the same connection can be finalized from such a callback. This method is a
 wrapper around [`sqlite3_finalize()`][].
 

src/node_sqlite.cc

@@ -160,6 +160,15 @@ inline MaybeLocal<Value> IntegerToValue(Isolate* isolate,
  (db)->IsInAuthorizerCallback(), \
  "database cannot be accessed from an authorizer callback")
 
+// Finalizing a busy statement from an authorizer can release its locks and
+// change the outer statement's outcome.
+#define THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt) \
+ THROW_AND_RETURN_ON_BAD_STATE( \
+ (env), \
+ (stmt)->db_->IsInAuthorizerCallback() && \
+ sqlite3_stmt_busy((stmt)->statement_.get()), \
+ "database cannot be accessed from an authorizer callback")
+
 // A statement's virtual machine cannot be reentered while sqlite3_step() is
 // running it. Finalizing it frees the VM outright, and re-running it resets the
 // VM mid-execution; both are use-after-free rather than merely a contract
@@ -2900,6 +2909,7 @@ void StatementSync::Close(const FunctionCallbackInfo<Value>& args) {
  THROW_AND_RETURN_ON_BAD_STATE(
  env, stmt->IsFinalized(), "statement has been finalized");
  THROW_AND_RETURN_IF_STEPPING(env, stmt);
+ THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt);
  stmt->Close();
 }
 
@@ -2913,6 +2923,7 @@ void StatementSync::Dispose(const FunctionCallbackInfo<Value>& args) {
  return;
  }
  THROW_AND_RETURN_IF_STEPPING(env, stmt);
+ THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt);
  stmt->Close();
 }
 

src/node_sqlite.h

@@ -299,9 +299,8 @@ class DatabaseSync : public BaseObject {
  void DecrementAuthorizerDepth() { --authorizer_depth_; }
  bool IsInAuthorizerCallback() const { return authorizer_depth_ > 0; }
 
- // Finalizing a statement frees its virtual machine, so a callback that
- // SQLite invokes from inside sqlite3_step() must not finalize the statement
- // being stepped. Other statements on the connection are safe to finalize.
+ // A callback must not finalize the statement being stepped. Other statements
+ // are safe unless they are busy during an authorizer callback.
  void PushSteppingStatement(sqlite3_stmt* stmt) {
  stepping_statements_.push_back(stmt);
  }

test/parallel/test-sqlite-authz.js

@@ -397,10 +397,8 @@ suite('authorizer callback reentrancy', () => {
  assert.deepStrictEqual(runInAuthorizer(db, cases), allRejected(cases));
  });
 
- // Only the statement being stepped is unsafe to finalize. Other statements
- // on the connection have their own virtual machines, so finalizing them from
- // a callback is allowed.
- it('allows finalizing a statement that is not being executed', () => {
+ // An idle statement has no virtual-machine state or locks to release.
+ it('allows finalizing an idle statement', () => {
  const db = new DatabaseSync(':memory:');
  db.exec('CREATE TABLE t (x INTEGER)');
  db.exec('INSERT INTO t VALUES (1)');
@@ -417,6 +415,43 @@ suite('authorizer callback reentrancy', () => {
  });
  });
 
+ // A paused iterator is busy and may hold locks between sqlite3_step() calls.
+ it('rejects finalizing another active statement', () => {
+ for (const method of ['close', 'dispose']) {
+ const db = new DatabaseSync(':memory:');
+ db.exec('CREATE TABLE t (x INTEGER)');
+ db.exec('INSERT INTO t VALUES (1), (2), (3)');
+ const stmt = db.prepare('SELECT x FROM t');
+ const iter = stmt.iterate();
+ iter.next();
+ let outcome = 'authorizer callback did not run';
+
+ db.setAuthorizer((actionCode) => {
+ if (actionCode === constants.SQLITE_DROP_TABLE) {
+ try {
+ if (method === 'close') {
+ stmt.close();
+ } else {
+ stmt[Symbol.dispose]();
+ }
+ outcome = 'did not throw';
+ } catch (err) {
+ outcome = `${err.code}: ${err.message}`;
+ }
+ }
+ return constants.SQLITE_OK;
+ });
+
+ assert.throws(() => db.exec('DROP TABLE t'), {
+ code: 'ERR_SQLITE_ERROR',
+ message: 'database table is locked',
+ });
+ assert.strictEqual(outcome, expectedError);
+ db.setAuthorizer(null);
+ iter.return();
+ }
+ });
+
  // Disposal is idempotent, so a statement that is already finalized must stay
  // a no-op even inside a callback. Throwing here would turn a `using` scope's
  // real exception into a SuppressedError.

doc/api/sqlite.md

@@ -1128,7 +1128,7 @@ Finalizes the prepared statement. An exception is thrown if the statement is
 already finalized. An [`ERR_INVALID_STATE`][] error is thrown if this statement
 is currently executing, which happens when the method is called from a callback
 that the statement itself triggered, such as a user-defined function, an
-aggregate function, or a [`'sqlite.db.query'`][] subscriber. Other statements
+aggregate function, or a [`'sqlite.db.query'`][] subscriber. Idle statements
 on the same connection can be finalized from such a callback. This method is a
 wrapper around [`sqlite3_finalize()`][].
 

src/node_sqlite.cc

@@ -160,6 +160,15 @@ inline MaybeLocal<Value> IntegerToValue(Isolate* isolate,
  (db)->IsInAuthorizerCallback(), \
  "database cannot be accessed from an authorizer callback")
 
+// Finalizing a busy statement from an authorizer can release its locks and
+// change the outer statement's outcome.
+#define THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt) \
+ THROW_AND_RETURN_ON_BAD_STATE( \
+ (env), \
+ (stmt)->db_->IsInAuthorizerCallback() && \
+ sqlite3_stmt_busy((stmt)->statement_.get()), \
+ "database cannot be accessed from an authorizer callback")
+
 // A statement's virtual machine cannot be reentered while sqlite3_step() is
 // running it. Finalizing it frees the VM outright, and re-running it resets the
 // VM mid-execution; both are use-after-free rather than merely a contract
@@ -2900,6 +2909,7 @@ void StatementSync::Close(const FunctionCallbackInfo<Value>& args) {
  THROW_AND_RETURN_ON_BAD_STATE(
  env, stmt->IsFinalized(), "statement has been finalized");
  THROW_AND_RETURN_IF_STEPPING(env, stmt);
+ THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt);
  stmt->Close();
 }
 
@@ -2913,6 +2923,7 @@ void StatementSync::Dispose(const FunctionCallbackInfo<Value>& args) {
  return;
  }
  THROW_AND_RETURN_IF_STEPPING(env, stmt);
+ THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt);
  stmt->Close();
 }
 

src/node_sqlite.h

@@ -299,9 +299,8 @@ class DatabaseSync : public BaseObject {
  void DecrementAuthorizerDepth() { --authorizer_depth_; }
  bool IsInAuthorizerCallback() const { return authorizer_depth_ > 0; }
 
- // Finalizing a statement frees its virtual machine, so a callback that
- // SQLite invokes from inside sqlite3_step() must not finalize the statement
- // being stepped. Other statements on the connection are safe to finalize.
+ // A callback must not finalize the statement being stepped. Other statements
+ // are safe unless they are busy during an authorizer callback.
  void PushSteppingStatement(sqlite3_stmt* stmt) {
  stepping_statements_.push_back(stmt);
  }

test/parallel/test-sqlite-authz.js

@@ -397,10 +397,8 @@ suite('authorizer callback reentrancy', () => {
  assert.deepStrictEqual(runInAuthorizer(db, cases), allRejected(cases));
  });
 
- // Only the statement being stepped is unsafe to finalize. Other statements
- // on the connection have their own virtual machines, so finalizing them from
- // a callback is allowed.
- it('allows finalizing a statement that is not being executed', () => {
+ // An idle statement has no virtual-machine state or locks to release.
+ it('allows finalizing an idle statement', () => {
  const db = new DatabaseSync(':memory:');
  db.exec('CREATE TABLE t (x INTEGER)');
  db.exec('INSERT INTO t VALUES (1)');
@@ -417,6 +415,43 @@ suite('authorizer callback reentrancy', () => {
  });
  });
 
+ // A paused iterator is busy and may hold locks between sqlite3_step() calls.
+ it('rejects finalizing another active statement', () => {
+ for (const method of ['close', 'dispose']) {
+ const db = new DatabaseSync(':memory:');
+ db.exec('CREATE TABLE t (x INTEGER)');
+ db.exec('INSERT INTO t VALUES (1), (2), (3)');
+ const stmt = db.prepare('SELECT x FROM t');
+ const iter = stmt.iterate();
+ iter.next();
+ let outcome = 'authorizer callback did not run';
+
+ db.setAuthorizer((actionCode) => {
+ if (actionCode === constants.SQLITE_DROP_TABLE) {
+ try {
+ if (method === 'close') {
+ stmt.close();
+ } else {
+ stmt[Symbol.dispose]();
+ }
+ outcome = 'did not throw';
+ } catch (err) {
+ outcome = `${err.code}: ${err.message}`;
+ }
+ }
+ return constants.SQLITE_OK;
+ });
+
+ assert.throws(() => db.exec('DROP TABLE t'), {
+ code: 'ERR_SQLITE_ERROR',
+ message: 'database table is locked',
+ });
+ assert.strictEqual(outcome, expectedError);
+ db.setAuthorizer(null);
+ iter.return();
+ }
+ });
+
  // Disposal is idempotent, so a statement that is already finalized must stay
  // a no-op even inside a callback. Throwing here would turn a `using` scope's
  // real exception into a SuppressedError.

doc/api/sqlite.md

@@ -1128,7 +1128,7 @@ Finalizes the prepared statement. An exception is thrown if the statement is
 already finalized. An [`ERR_INVALID_STATE`][] error is thrown if this statement
 is currently executing, which happens when the method is called from a callback
 that the statement itself triggered, such as a user-defined function, an
-aggregate function, or a [`'sqlite.db.query'`][] subscriber. Other statements
+aggregate function, or a [`'sqlite.db.query'`][] subscriber. Idle statements
 on the same connection can be finalized from such a callback. This method is a
 wrapper around [`sqlite3_finalize()`][].
 

src/node_sqlite.cc

@@ -160,6 +160,15 @@ inline MaybeLocal<Value> IntegerToValue(Isolate* isolate,
  (db)->IsInAuthorizerCallback(), \
  "database cannot be accessed from an authorizer callback")
 
+// Finalizing a busy statement from an authorizer can release its locks and
+// change the outer statement's outcome.
+#define THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt) \
+ THROW_AND_RETURN_ON_BAD_STATE( \
+ (env), \
+ (stmt)->db_->IsInAuthorizerCallback() && \
+ sqlite3_stmt_busy((stmt)->statement_.get()), \
+ "database cannot be accessed from an authorizer callback")
+
 // A statement's virtual machine cannot be reentered while sqlite3_step() is
 // running it. Finalizing it frees the VM outright, and re-running it resets the
 // VM mid-execution; both are use-after-free rather than merely a contract
@@ -2900,6 +2909,7 @@ void StatementSync::Close(const FunctionCallbackInfo<Value>& args) {
  THROW_AND_RETURN_ON_BAD_STATE(
  env, stmt->IsFinalized(), "statement has been finalized");
  THROW_AND_RETURN_IF_STEPPING(env, stmt);
+ THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt);
  stmt->Close();
 }
 
@@ -2913,6 +2923,7 @@ void StatementSync::Dispose(const FunctionCallbackInfo<Value>& args) {
  return;
  }
  THROW_AND_RETURN_IF_STEPPING(env, stmt);
+ THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt);
  stmt->Close();
 }
 

src/node_sqlite.h

@@ -299,9 +299,8 @@ class DatabaseSync : public BaseObject {
  void DecrementAuthorizerDepth() { --authorizer_depth_; }
  bool IsInAuthorizerCallback() const { return authorizer_depth_ > 0; }
 
- // Finalizing a statement frees its virtual machine, so a callback that
- // SQLite invokes from inside sqlite3_step() must not finalize the statement
- // being stepped. Other statements on the connection are safe to finalize.
+ // A callback must not finalize the statement being stepped. Other statements
+ // are safe unless they are busy during an authorizer callback.
  void PushSteppingStatement(sqlite3_stmt* stmt) {
  stepping_statements_.push_back(stmt);
  }

test/parallel/test-sqlite-authz.js

@@ -397,10 +397,8 @@ suite('authorizer callback reentrancy', () => {
  assert.deepStrictEqual(runInAuthorizer(db, cases), allRejected(cases));
  });
 
- // Only the statement being stepped is unsafe to finalize. Other statements
- // on the connection have their own virtual machines, so finalizing them from
- // a callback is allowed.
- it('allows finalizing a statement that is not being executed', () => {
+ // An idle statement has no virtual-machine state or locks to release.
+ it('allows finalizing an idle statement', () => {
  const db = new DatabaseSync(':memory:');
  db.exec('CREATE TABLE t (x INTEGER)');
  db.exec('INSERT INTO t VALUES (1)');
@@ -417,6 +415,43 @@ suite('authorizer callback reentrancy', () => {
  });
  });
 
+ // A paused iterator is busy and may hold locks between sqlite3_step() calls.
+ it('rejects finalizing another active statement', () => {
+ for (const method of ['close', 'dispose']) {
+ const db = new DatabaseSync(':memory:');
+ db.exec('CREATE TABLE t (x INTEGER)');
+ db.exec('INSERT INTO t VALUES (1), (2), (3)');
+ const stmt = db.prepare('SELECT x FROM t');
+ const iter = stmt.iterate();
+ iter.next();
+ let outcome = 'authorizer callback did not run';
+
+ db.setAuthorizer((actionCode) => {
+ if (actionCode === constants.SQLITE_DROP_TABLE) {
+ try {
+ if (method === 'close') {
+ stmt.close();
+ } else {
+ stmt[Symbol.dispose]();
+ }
+ outcome = 'did not throw';
+ } catch (err) {
+ outcome = `${err.code}: ${err.message}`;
+ }
+ }
+ return constants.SQLITE_OK;
+ });
+
+ assert.throws(() => db.exec('DROP TABLE t'), {
+ code: 'ERR_SQLITE_ERROR',
+ message: 'database table is locked',
+ });
+ assert.strictEqual(outcome, expectedError);
+ db.setAuthorizer(null);
+ iter.return();
+ }
+ });
+
  // Disposal is idempotent, so a statement that is already finalized must stay
  // a no-op even inside a callback. Throwing here would turn a `using` scope's
  // real exception into a SuppressedError.

doc/api/sqlite.md

@@ -1128,7 +1128,7 @@ Finalizes the prepared statement. An exception is thrown if the statement is
 already finalized. An [`ERR_INVALID_STATE`][] error is thrown if this statement
 is currently executing, which happens when the method is called from a callback
 that the statement itself triggered, such as a user-defined function, an
-aggregate function, or a [`'sqlite.db.query'`][] subscriber. Other statements
+aggregate function, or a [`'sqlite.db.query'`][] subscriber. Idle statements
 on the same connection can be finalized from such a callback. This method is a
 wrapper around [`sqlite3_finalize()`][].
 

src/node_sqlite.cc

@@ -160,6 +160,15 @@ inline MaybeLocal<Value> IntegerToValue(Isolate* isolate,
  (db)->IsInAuthorizerCallback(), \
  "database cannot be accessed from an authorizer callback")
 
+// Finalizing a busy statement from an authorizer can release its locks and
+// change the outer statement's outcome.
+#define THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt) \
+ THROW_AND_RETURN_ON_BAD_STATE( \
+ (env), \
+ (stmt)->db_->IsInAuthorizerCallback() && \
+ sqlite3_stmt_busy((stmt)->statement_.get()), \
+ "database cannot be accessed from an authorizer callback")
+
 // A statement's virtual machine cannot be reentered while sqlite3_step() is
 // running it. Finalizing it frees the VM outright, and re-running it resets the
 // VM mid-execution; both are use-after-free rather than merely a contract
@@ -2900,6 +2909,7 @@ void StatementSync::Close(const FunctionCallbackInfo<Value>& args) {
  THROW_AND_RETURN_ON_BAD_STATE(
  env, stmt->IsFinalized(), "statement has been finalized");
  THROW_AND_RETURN_IF_STEPPING(env, stmt);
+ THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt);
  stmt->Close();
 }
 
@@ -2913,6 +2923,7 @@ void StatementSync::Dispose(const FunctionCallbackInfo<Value>& args) {
  return;
  }
  THROW_AND_RETURN_IF_STEPPING(env, stmt);
+ THROW_AND_RETURN_IF_BUSY_IN_AUTHORIZER(env, stmt);
  stmt->Close();
 }
 

src/node_sqlite.h

@@ -299,9 +299,8 @@ class DatabaseSync : public BaseObject {
  void DecrementAuthorizerDepth() { --authorizer_depth_; }
  bool IsInAuthorizerCallback() const { return authorizer_depth_ > 0; }
 
- // Finalizing a statement frees its virtual machine, so a callback that
- // SQLite invokes from inside sqlite3_step() must not finalize the statement
- // being stepped. Other statements on the connection are safe to finalize.
+ // A callback must not finalize the statement being stepped. Other statements
+ // are safe unless they are busy during an authorizer callback.
  void PushSteppingStatement(sqlite3_stmt* stmt) {
  stepping_statements_.push_back(stmt);
  }

test/parallel/test-sqlite-authz.js

@@ -397,10 +397,8 @@ suite('authorizer callback reentrancy', () => {
  assert.deepStrictEqual(runInAuthorizer(db, cases), allRejected(cases));
  });
 
- // Only the statement being stepped is unsafe to finalize. Other statements
- // on the connection have their own virtual machines, so finalizing them from
- // a callback is allowed.
- it('allows finalizing a statement that is not being executed', () => {
+ // An idle statement has no virtual-machine state or locks to release.
+ it('allows finalizing an idle statement', () => {
  const db = new DatabaseSync(':memory:');
  db.exec('CREATE TABLE t (x INTEGER)');
  db.exec('INSERT INTO t VALUES (1)');
@@ -417,6 +415,43 @@ suite('authorizer callback reentrancy', () => {
  });
  });
 
+ // A paused iterator is busy and may hold locks between sqlite3_step() calls.
+ it('rejects finalizing another active statement', () => {
+ for (const method of ['close', 'dispose']) {
+ const db = new DatabaseSync(':memory:');
+ db.exec('CREATE TABLE t (x INTEGER)');
+ db.exec('INSERT INTO t VALUES (1), (2), (3)');
+ const stmt = db.prepare('SELECT x FROM t');
+ const iter = stmt.iterate();
+ iter.next();
+ let outcome = 'authorizer callback did not run';
+
+ db.setAuthorizer((actionCode) => {
+ if (actionCode === constants.SQLITE_DROP_TABLE) {
+ try {
+ if (method === 'close') {
+ stmt.close();
+ } else {
+ stmt[Symbol.dispose]();
+ }
+ outcome = 'did not throw';
+ } catch (err) {
+ outcome = `${err.code}: ${err.message}`;
+ }
+ }
+ return constants.SQLITE_OK;
+ });
+
+ assert.throws(() => db.exec('DROP TABLE t'), {
+ code: 'ERR_SQLITE_ERROR',
+ message: 'database table is locked',
+ });
+ assert.strictEqual(outcome, expectedError);
+ db.setAuthorizer(null);
+ iter.return();
+ }
+ });
+
  // Disposal is idempotent, so a statement that is already finalized must stay
  // a no-op even inside a callback. Throwing here would turn a `using` scope's
  // real exception into a SuppressedError.