Repository: nodejs/node
Pull request: 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.

<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: doc/api/sqlite.md
patch: @@ -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

filename: src/node_sqlite.cc
patch: @@ -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(
-      env, args.This(), args[0].As<String>(), open, enable_foreign_keys);
+  new DatabaseSync(env,
+                   args.This(),
+                   args[0].As<String>(),
+                   open,
+                   enable_foreign_keys,
+                   enable_dqs);
 }
 
 void DatabaseSync::Open(const FunctionCallbackInfo<Value>& args) {

filename: src/node_sqlite.h
patch: @@ -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 {

filename: test/parallel/test-sqlite-database-sync.js
patch: @@ -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()', () => {