sqlite: reject statement-less SQL in prepare() · nodejs/node@5e387ce · GitHub
Skip to content

Commit 5e387ce

Browse files
TrevorBurnhamaduh95
authored andcommitted
sqlite: reject statement-less SQL in prepare()
Apply the same check to DatabaseSync::Prepare() so that statement-less SQL is rejected at preparation instead of on first use. This matches SQLite's own oo1 JavaScript API, which throws when the SQL contains no statements rather than exposing the C API's null statement pointer. Previously db.prepare('-- comment') returned a StatementSync whose statement_ was null. Every method on it threw "statement has been finalized", which was misleading because nothing had been finalized, and the object was still inserted into statements_. Since IsFinalized() is true for a null statement, its destructor skipped UntrackStatement() and left a dangling pointer in the set that a later close() would finalize. Refs: #65157 (comment) Refs: https://sqlite.org/wasm/doc/trunk/api-oo1.md Signed-off-by: Trevor Burnham <trevorburnham@gmail.com> PR-URL: #65157 Fixes: #65149 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
1 parent 83169e5 commit 5e387ce

3 files changed

Lines changed: 42 additions & 3 deletions

File tree

doc/api/sqlite.md

Lines changed: 4 additions & 0 deletions

src/node_sqlite.cc

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1581,6 +1581,16 @@ void DatabaseSync::Prepare(const FunctionCallbackInfo<Value>& args) {
15811581
int r = sqlite3_prepare_v2(db->connection_, *sql, -1, &s, nullptr);
15821582

15831583
CHECK_ERROR_OR_THROW(env->isolate(), db, r, SQLITE_OK, void());
1584+
1585+
// sqlite3_prepare_v2() reports success without producing a statement when
1586+
// the input holds no SQL, such as a comment. Such a statement can never be
1587+
// stepped, and tracking it would leave a dangling pointer in statements_
1588+
// because its destructor treats a null statement as already finalized.
1589+
if (s == nullptr) {
1590+
THROW_ERR_INVALID_ARG_VALUE(env, "The SQL query contains no statements.");
1591+
return;
1592+
}
1593+
15841594
BaseObjectPtr<StatementSync> stmt =
15851595
StatementSync::Create(env, BaseObjectPtr<DatabaseSync>(db), s);
15861596
db->statements_.insert(stmt.get());
@@ -3659,9 +3669,8 @@ BaseObjectPtr<StatementSync> SQLTagStore::PrepareStatement(
36593669
return BaseObjectPtr<StatementSync>();
36603670
}
36613671

3662-
// sqlite3_prepare_v2() reports success without producing a statement when
3663-
// the input holds no SQL, such as a comment. Such a statement cannot be
3664-
// bound or executed, so reject it instead of caching it.
3672+
// As in DatabaseSync::Prepare(), reject input that holds no SQL rather
3673+
// than caching a statement that can never be bound or stepped.
36653674
if (s == nullptr) {
36663675
THROW_ERR_INVALID_ARG_VALUE(env, "The SQL query contains no statements.");
36673676
return BaseObjectPtr<StatementSync>();

test/parallel/test-sqlite-database-sync.js

Lines changed: 26 additions & 0 deletions

0 commit comments

Comments
 (0)