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>

Author: TrevorBurnham

doc/api/sqlite.md

@@ -668,6 +668,10 @@ console.log(query.get());
 
 <!-- YAML
 added: v22.5.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/65157
+    description: Throw `ERR_INVALID_ARG_VALUE` if `sql` contains no statements.
 -->
 
 * `sql` {string} A SQL string to compile to a prepared statement.

src/node_sqlite.cc

@@ -1575,6 +1575,16 @@ void DatabaseSync::Prepare(const FunctionCallbackInfo<Value>& args) {
   int r = sqlite3_prepare_v2(db->connection_, *sql, -1, &s, nullptr);
 
   CHECK_ERROR_OR_THROW(env->isolate(), db, r, SQLITE_OK, void());
+
+  // sqlite3_prepare_v2() reports success without producing a statement when
+  // the input holds no SQL, such as a comment. Such a statement can never be
+  // stepped, and tracking it would leave a dangling pointer in statements_
+  // because its destructor treats a null statement as already finalized.
+  if (s == nullptr) {
+    THROW_ERR_INVALID_ARG_VALUE(env, "The SQL query contains no statements.");
+    return;
+  }
+
   BaseObjectPtr<StatementSync> stmt =
       StatementSync::Create(env, BaseObjectPtr<DatabaseSync>(db), s);
   db->statements_.insert(stmt.get());
@@ -3648,9 +3658,8 @@ BaseObjectPtr<StatementSync> SQLTagStore::PrepareStatement(
       return BaseObjectPtr<StatementSync>();
     }
 
-    // sqlite3_prepare_v2() reports success without producing a statement when
-    // the input holds no SQL, such as a comment. Such a statement cannot be
-    // bound or executed, so reject it instead of caching it.
+    // As in DatabaseSync::Prepare(), reject input that holds no SQL rather
+    // than caching a statement that can never be bound or stepped.
     if (s == nullptr) {
       THROW_ERR_INVALID_ARG_VALUE(env, "The SQL query contains no statements.");
       return BaseObjectPtr<StatementSync>();

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

@@ -397,6 +397,32 @@ suite('DatabaseSync.prototype.prepare()', () => {
       message: /The "sql" argument must be a string/,
     });
   });
+
+  test('throws if sql contains no statements', (t) => {
+    using db = new DatabaseSync(nextDb());
+
+    for (const sql of ['', '   ', ';', '-- comment', '/* comment */']) {
+      t.assert.throws(() => {
+        db.prepare(sql);
+      }, {
+        code: 'ERR_INVALID_ARG_VALUE',
+        message: /contains no statements/,
+      });
+    }
+  });
+
+  test('prepares statements that contain comments', (t) => {
+    using db = new DatabaseSync(nextDb());
+    const queries = [
+      '-- lead\nSELECT 1 AS v',
+      'SELECT 1 AS v -- trail',
+      'SELECT /* mid */ 1 AS v',
+    ];
+
+    for (const sql of queries) {
+      t.assert.strictEqual(db.prepare(sql).get().v, 1);
+    }
+  });
 });
 
 suite('DatabaseSync.prototype.exec()', () => {