From a49abec6c310d2905a5296c66da3bc3b9e47a5da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Sun, 6 Oct 2024 17:57:23 +0200 Subject: [PATCH] sqlite: enable foreign key constraints by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For historical reasons and to maintain compatibibility with legacy database schemas, SQLite does not enable foreign key constraints by default. For new applications, however, this behavior is undesirable. Currently, any application that wishes to use foreign keys must use PRAGMA foreign_keys = ON; to explicitly enable enforcement of such constraints. This commit changes the behavior of the SQLite API built into Node.js to enable foreign key constraints by default. This behavior can be overridden by users to maintain compatibility with legacy database schemas. PR-URL: https://github.com/nodejs/node/pull/54777 Reviewed-By: Yagiz Nizipli Reviewed-By: Chemi Atlow Reviewed-By: Michaël Zasso Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- doc/api/sqlite.md | 6 ++++ src/node_sqlite.cc | 35 +++++++++++++++++++-- src/node_sqlite.h | 4 ++- test/parallel/test-sqlite-database-sync.js | 36 ++++++++++++++++++++++ 4 files changed, 78 insertions(+), 3 deletions(-) diff --git a/doc/api/sqlite.md b/doc/api/sqlite.md index e0d102a1c47..c85b7f5898e 100644 --- a/doc/api/sqlite.md +++ b/doc/api/sqlite.md @@ -107,6 +107,11 @@ added: v22.5.0 * `open` {boolean} If `true`, the database is opened by the constructor. When this value is `false`, the database must be opened via the `open()` method. **Default:** `true`. + * `enableForeignKeyConstraints` {boolean} If `true`, foreign key constraints + are enabled. This is recommended but can be disabled for compatibility with + legacy database schemas. The enforcement of foreign key constraints can be + enabled and disabled after opening the database using + [`PRAGMA foreign_keys`][]. **Default:** `true`. Constructs a new `DatabaseSync` instance. @@ -317,6 +322,7 @@ exception. [SQL injection]: https://en.wikipedia.org/wiki/SQL_injection [`--experimental-sqlite`]: cli.md#--experimental-sqlite +[`PRAGMA foreign_keys`]: https://www.sqlite.org/pragma.html#pragma_foreign_keys [`sqlite3_changes64()`]: https://www.sqlite.org/c3ref/changes.html [`sqlite3_close_v2()`]: https://www.sqlite.org/c3ref/close.html [`sqlite3_exec()`]: https://www.sqlite.org/c3ref/exec.html diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index fbef321dcab..c3c4ab04e22 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -91,12 +91,14 @@ inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, const char* message) { DatabaseSync::DatabaseSync(Environment* env, Local object, Local location, - bool open) + bool open, + bool enable_foreign_keys_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; if (open) { Open(); @@ -125,6 +127,15 @@ bool DatabaseSync::Open() { int flags = SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE; int r = sqlite3_open_v2(location_.c_str(), &connection_, flags, 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, + static_cast(enable_foreign_keys_on_open_), + &foreign_keys_enabled); + CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false); + CHECK_EQ(foreign_keys_enabled, enable_foreign_keys_on_open_); + return true; } @@ -166,6 +177,7 @@ void DatabaseSync::New(const FunctionCallbackInfo& args) { } bool open = true; + bool enable_foreign_keys = true; if (args.Length() > 1) { if (!args[1]->IsObject()) { @@ -188,9 +200,28 @@ void DatabaseSync::New(const FunctionCallbackInfo& args) { } open = open_v.As()->Value(); } + + Local enable_foreign_keys_string = + FIXED_ONE_BYTE_STRING(env->isolate(), "enableForeignKeyConstraints"); + Local enable_foreign_keys_v; + if (!options->Get(env->context(), enable_foreign_keys_string) + .ToLocal(&enable_foreign_keys_v)) { + return; + } + if (!enable_foreign_keys_v->IsUndefined()) { + if (!enable_foreign_keys_v->IsBoolean()) { + node::THROW_ERR_INVALID_ARG_TYPE( + env->isolate(), + "The \"options.enableForeignKeyConstraints\" argument must be a " + "boolean."); + return; + } + enable_foreign_keys = enable_foreign_keys_v.As()->Value(); + } } - new DatabaseSync(env, args.This(), args[0].As(), open); + new DatabaseSync( + env, args.This(), args[0].As(), open, enable_foreign_keys); } void DatabaseSync::Open(const FunctionCallbackInfo& args) { diff --git a/src/node_sqlite.h b/src/node_sqlite.h index f62b2e991cb..8399f3dc4da 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -21,7 +21,8 @@ class DatabaseSync : public BaseObject { DatabaseSync(Environment* env, v8::Local object, v8::Local location, - bool open); + bool open, + bool enable_foreign_keys_on_open); void MemoryInfo(MemoryTracker* tracker) const override; static void New(const v8::FunctionCallbackInfo& args); static void Open(const v8::FunctionCallbackInfo& args); @@ -43,6 +44,7 @@ class DatabaseSync : public BaseObject { std::string location_; sqlite3* connection_; std::unordered_set statements_; + bool enable_foreign_keys_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 e528daf227c..777b273e9a5 100644 --- a/test/parallel/test-sqlite-database-sync.js +++ b/test/parallel/test-sqlite-database-sync.js @@ -50,6 +50,42 @@ suite('DatabaseSync() constructor', () => { message: /The "options\.open" argument must be a boolean/, }); }); + + test('throws if options.enableForeignKeyConstraints is provided but is not a boolean', (t) => { + t.assert.throws(() => { + new DatabaseSync('foo', { enableForeignKeyConstraints: 5 }); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: /The "options\.enableForeignKeyConstraints" argument must be a boolean/, + }); + }); + + test('enables foreign key constraints by default', (t) => { + const dbPath = nextDb(); + const db = new DatabaseSync(dbPath); + db.exec(` + CREATE TABLE foo (id INTEGER PRIMARY KEY); + CREATE TABLE bar (foo_id INTEGER REFERENCES foo(id)); + `); + t.after(() => { db.close(); }); + t.assert.throws(() => { + db.exec('INSERT INTO bar (foo_id) VALUES (1)'); + }, { + code: 'ERR_SQLITE_ERROR', + message: 'FOREIGN KEY constraint failed', + }); + }); + + test('allows disabling foreign key constraints', (t) => { + const dbPath = nextDb(); + const db = new DatabaseSync(dbPath, { enableForeignKeyConstraints: false }); + db.exec(` + CREATE TABLE foo (id INTEGER PRIMARY KEY); + CREATE TABLE bar (foo_id INTEGER REFERENCES foo(id)); + `); + t.after(() => { db.close(); }); + db.exec('INSERT INTO bar (foo_id) VALUES (1)'); + }); }); suite('DatabaseSync.prototype.open()', () => {