sqlite: disable DQS misfeature by default

Double-quoted string (DQS) literals are not allowed by the SQL standard,
which defines that text enclosed in double quotes is to be interpreted
as an identifier only and never as a string literal. Nevertheless, for
historical reasons, SQLite allows double-quoted string literals in some
cases, which leads to inconsistent behavior and subtle bugs.

This commit changes the behavior of the built-in Node.js API for SQLite
such that the DQS misfeature is disabled by default. This is recommended
by the developers of SQLite. Users can explicitly enable DQS for
compatibility with legacy database schemas if necessary.

PR-URL: https://github.com/nodejs/node/pull/55297
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>

doc/api/sqlite.md

@@ -112,6 +112,10 @@ added: v22.5.0
     legacy database schemas. The enforcement of foreign key constraints can be
     enabled and disabled after opening the database using
     [`PRAGMA foreign_keys`][]. **Default:** `true`.
+  * `enableDoubleQuotedStringLiterals` {boolean} If `true`, SQLite will accept
+    [double-quoted string literals][]. This is not recommended but can be
+    enabled for compatibility with legacy database schemas.
+    **Default:** `false`.
 
 Constructs a new `DatabaseSync` instance.
 
@@ -332,6 +336,7 @@ exception.
 [`sqlite3_sql()`]: https://www.sqlite.org/c3ref/expanded_sql.html
 [connection]: https://www.sqlite.org/c3ref/sqlite3.html
 [data types]: https://www.sqlite.org/datatype3.html
+[double-quoted string literals]: https://www.sqlite.org/quirks.html#dblquote
 [in memory]: https://www.sqlite.org/inmemorydb.html
 [parameters are bound]: https://www.sqlite.org/c3ref/bind_blob.html
 [prepared statement]: https://www.sqlite.org/c3ref/stmt.html

src/node_sqlite.cc

@@ -96,13 +96,15 @@ DatabaseSync::DatabaseSync(Environment* env,
                            Local<Object> object,
                            Local<String> location,
                            bool open,
-                           bool enable_foreign_keys_on_open)
+                           bool enable_foreign_keys_on_open,
+                           bool enable_dqs_on_open)
     : BaseObject(env, object) {
   MakeWeak();
   node::Utf8Value utf8_location(env->isolate(), location);
   location_ = utf8_location.ToString();
   connection_ = nullptr;
   enable_foreign_keys_on_open_ = enable_foreign_keys_on_open;
+  enable_dqs_on_open_ = enable_dqs_on_open;
 
   if (open) {
     Open();
@@ -132,6 +134,17 @@ bool DatabaseSync::Open() {
   int r = sqlite3_open_v2(location_.c_str(), &connection_, flags, nullptr);
   CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
 
+  r = sqlite3_db_config(connection_,
+                        SQLITE_DBCONFIG_DQS_DML,
+                        static_cast<int>(enable_dqs_on_open_),
+                        nullptr);
+  CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
+  r = sqlite3_db_config(connection_,
+                        SQLITE_DBCONFIG_DQS_DDL,
+                        static_cast<int>(enable_dqs_on_open_),
+                        nullptr);
+  CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
+
   int foreign_keys_enabled;
   r = sqlite3_db_config(connection_,
                         SQLITE_DBCONFIG_ENABLE_FKEY,
@@ -182,6 +195,7 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
 
   bool open = true;
   bool enable_foreign_keys = true;
+  bool enable_dqs = false;
 
   if (args.Length() > 1) {
     if (!args[1]->IsObject()) {
@@ -222,10 +236,32 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
       }
       enable_foreign_keys = enable_foreign_keys_v.As<Boolean>()->Value();
     }
+
+    Local<String> enable_dqs_string = FIXED_ONE_BYTE_STRING(
+        env->isolate(), "enableDoubleQuotedStringLiterals");
+    Local<Value> enable_dqs_v;
+    if (!options->Get(env->context(), enable_dqs_string)
+             .ToLocal(&enable_dqs_v)) {
+      return;
+    }
+    if (!enable_dqs_v->IsUndefined()) {
+      if (!enable_dqs_v->IsBoolean()) {
+        node::THROW_ERR_INVALID_ARG_TYPE(
+            env->isolate(),
+            "The \"options.enableDoubleQuotedStringLiterals\" argument must be "
+            "a boolean.");
+        return;
+      }
+      enable_dqs = enable_dqs_v.As<Boolean>()->Value();
+    }
   }
 
-  new DatabaseSync(
+  new DatabaseSync(env,
-      env, args.This(), args[0].As<String>(), open, enable_foreign_keys);
+                   args.This(),
+                   args[0].As<String>(),
+                   open,
+                   enable_foreign_keys,
+                   enable_dqs);
 }
 
 void DatabaseSync::Open(const FunctionCallbackInfo<Value>& args) {

src/node_sqlite.h

@@ -22,7 +22,8 @@ class DatabaseSync : public BaseObject {
                v8::Local<v8::Object> object,
                v8::Local<v8::String> location,
                bool open,
-               bool enable_foreign_keys_on_open);
+               bool enable_foreign_keys_on_open,
+               bool enable_dqs_on_open);
   void MemoryInfo(MemoryTracker* tracker) const override;
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Open(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -45,6 +46,7 @@ class DatabaseSync : public BaseObject {
   sqlite3* connection_;
   std::unordered_set<StatementSync*> statements_;
   bool enable_foreign_keys_on_open_;
+  bool enable_dqs_on_open_;
 };
 
 class StatementSync : public BaseObject {

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

@@ -86,6 +86,34 @@ suite('DatabaseSync() constructor', () => {
     t.after(() => { db.close(); });
     db.exec('INSERT INTO bar (foo_id) VALUES (1)');
   });
+
+  test('throws if options.enableDoubleQuotedStringLiterals is provided but is not a boolean', (t) => {
+    t.assert.throws(() => {
+      new DatabaseSync('foo', { enableDoubleQuotedStringLiterals: 5 });
+    }, {
+      code: 'ERR_INVALID_ARG_TYPE',
+      message: /The "options\.enableDoubleQuotedStringLiterals" argument must be a boolean/,
+    });
+  });
+
+  test('disables double-quoted string literals by default', (t) => {
+    const dbPath = nextDb();
+    const db = new DatabaseSync(dbPath);
+    t.after(() => { db.close(); });
+    t.assert.throws(() => {
+      db.exec('SELECT "foo";');
+    }, {
+      code: 'ERR_SQLITE_ERROR',
+      message: /no such column: "foo"/,
+    });
+  });
+
+  test('allows enabling double-quoted string literals', (t) => {
+    const dbPath = nextDb();
+    const db = new DatabaseSync(dbPath, { enableDoubleQuotedStringLiterals: true });
+    t.after(() => { db.close(); });
+    db.exec('SELECT "foo";');
+  });
 });
 
 suite('DatabaseSync.prototype.open()', () => {