[pull] main from nodejs:main

doc/api/assert.md

@@ -317,7 +317,7 @@ const assert2 = new Assert({ skipPrototype: true });
 assert2.deepStrictEqual(foo, bar); // OK
 ```
 
-When destructured, methods lose access to the instance's `this` context and revert to default assertion behavior
+When destructured, methods lose access to the instance's `this` context and revert to the default assertion behavior
 (diff: 'simple', non-strict mode).
 To maintain custom options when using destructured methods, avoid
 destructuring and call methods directly on the instance.
@@ -423,8 +423,8 @@ are also recursively evaluated by the following rules.
 ### Comparison details
 
 * Primitive values are compared with the [`==` operator][],
-  with the exception of {NaN}. It is treated as being identical in case
-  both sides are {NaN}.
+  except for {NaN}, which is treated as identical when both
+  sides are {NaN}.
 * [Type tags][Object.prototype.toString()] of objects should be the same.
 * Only [enumerable "own" properties][] are considered.
 * Object constructors are compared when available.
@@ -938,7 +938,7 @@ error messages as expressive as possible.
 If specified, `error` can be a [`Class`][], {RegExp} or a validation
 function. See [`assert.throws()`][] for more details.
 
-Besides the async nature to await the completion behaves identically to
+Aside from asynchronously awaiting completion, it behaves identically to
 [`assert.doesNotThrow()`][].
 
 ```mjs

doc/api/sqlite.md

@@ -670,6 +670,10 @@
 
 <!-- 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

@@ -1581,6 +1581,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());
@@ -3659,6 +3669,13 @@ BaseObjectPtr<StatementSync> SQLTagStore::PrepareStatement(
       return BaseObjectPtr<StatementSync>();
     }
 
+    // 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>();
+    }
+
     BaseObjectPtr<StatementSync> stmt_obj = StatementSync::Create(
         env, BaseObjectPtr<DatabaseSync>(session->database_), s);
 

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()', () => {

test/parallel/test-sqlite-template-tag.js

@@ -317,6 +317,29 @@ test('sql error messages are descriptive', () => {
   });
 });
 
+test('rejects SQL that contains no statements', () => {
+  const expectedError = {
+    code: 'ERR_INVALID_ARG_VALUE',
+    message: /contains no statements/,
+  };
+
+  for (const method of ['run', 'get', 'all', 'iterate']) {
+    assert.throws(() => {
+      // eslint-disable-next-line no-unused-expressions
+      sql[method]`-- comment`;
+    }, expectedError);
+
+    assert.throws(() => {
+      // eslint-disable-next-line no-unused-expressions
+      sql[method]``;
+    }, expectedError);
+  }
+
+  // A rejected statement must not be cached, so a later valid query with the
+  // same tag store still works.
+  assert.strictEqual(sql.run`INSERT INTO foo (text) VALUES (${'bob'})`.changes, 1);
+});
+
 test('a tag store keeps the database alive by itself', () => {
   const sql = new DatabaseSync(':memory:').createTagStore();