From 09d10b50dcdf44b5ab0e97d22fc56dc52c594bd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Tue, 8 Oct 2024 23:49:38 +0200 Subject: [PATCH] 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 Reviewed-By: Yagiz Nizipli Reviewed-By: Chemi Atlow Reviewed-By: Jake Yuesong Li --- doc/api/sqlite.md | 5 +++ src/node_sqlite.cc | 42 ++++++++++++++++++++-- src/node_sqlite.h | 4 ++- test/parallel/test-sqlite-database-sync.js | 28 +++++++++++++++ 4 files changed, 75 insertions(+), 4 deletions(-) diff --git a/doc/api/sqlite.md b/doc/api/sqlite.md index 59202a1dc25c35..cfb3f48d70a42c 100644 --- a/doc/api/sqlite.md +++ b/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 diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 30c58d83f28a61..df6bb45ad60e7d 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -96,13 +96,15 @@ DatabaseSync::DatabaseSync(Environment* env, Local object, Local 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(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(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& 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& args) { } enable_foreign_keys = enable_foreign_keys_v.As()->Value(); } + + Local enable_dqs_string = FIXED_ONE_BYTE_STRING( + env->isolate(), "enableDoubleQuotedStringLiterals"); + Local 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()->Value(); + } } - new DatabaseSync( - env, args.This(), args[0].As(), open, enable_foreign_keys); + new DatabaseSync(env, + args.This(), + args[0].As(), + open, + enable_foreign_keys, + enable_dqs); } void DatabaseSync::Open(const FunctionCallbackInfo& args) { diff --git a/src/node_sqlite.h b/src/node_sqlite.h index 95203a353b21d2..40abf7b0d3edc3 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -22,7 +22,8 @@ class DatabaseSync : public BaseObject { v8::Local object, v8::Local 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& args); static void Open(const v8::FunctionCallbackInfo& args); @@ -45,6 +46,7 @@ class DatabaseSync : public BaseObject { sqlite3* connection_; std::unordered_set statements_; bool enable_foreign_keys_on_open_; + bool enable_dqs_on_open_; }; class StatementSync : public BaseObject { diff --git a/test/parallel/test-sqlite-database-sync.js b/test/parallel/test-sqlite-database-sync.js index 777b273e9a5aac..7979d01b568aa1 100644 --- a/test/parallel/test-sqlite-database-sync.js +++ b/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()', () => {