diff --git a/src/env_properties.h b/src/env_properties.h index c4e3bc5c72a0..65366152e057 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -346,8 +346,6 @@ V(stream_count_string, "streamCount") \ V(synthetic_string, "synthetic") \ V(syscall_string, "syscall") \ - V(table_string, "table") \ - V(target_string, "target") \ V(then_string, "then") \ V(thread_id_string, "threadId") \ V(thread_name_string, "threadName") \ diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 36aab3f3a421..f6ddf3761672 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -1835,10 +1835,10 @@ void Database::CreateTagStore(const FunctionCallbackInfo& args) { // undefined. Returns false if a JavaScript exception is pending, either from // the property getter or because the value is not a boolean. template -static bool ReadOptionalBool(Environment* env, - Local options, - const char* key, - T* out) { +[[nodiscard]] static bool ReadOptionalBool(Environment* env, + Local options, + const char* key, + T* out) { Local key_string = OneByteString(env->isolate(), key, -1, NewStringType::kInternalized); Local value; @@ -1857,6 +1857,31 @@ static bool ReadOptionalBool(Environment* env, return true; } +// Reads options[key] into *out as a string, leaving *out unchanged if the value +// is undefined. Returns false if a JavaScript exception is pending, either from +// the property getter or because the value is not a string. +[[nodiscard]] static bool ReadOptionalString(Environment* env, + Local options, + const char* key, + std::string* out) { + Local key_string = + OneByteString(env->isolate(), key, -1, NewStringType::kInternalized); + Local value; + if (!options->Get(env->context(), key_string).ToLocal(&value)) { + return false; + } + if (value->IsUndefined()) { + return true; + } + if (!value->IsString()) { + THROW_ERR_INVALID_ARG_TYPE( + env->isolate(), "The \"options.%s\" argument must be a string.", key); + return false; + } + *out = Utf8Value(env->isolate(), value.As()).ToString(); + return true; +} + std::optional ValidateDatabasePath(Environment* env, Local path, const std::string& field_name) { @@ -2427,22 +2452,7 @@ void Database::Deserialize(const FunctionCallbackInfo& args) { } Local options = args[1].As(); - Local db_name_key = FIXED_ONE_BYTE_STRING(env->isolate(), "dbName"); - Local db_name_value; - if (!options->Get(env->context(), db_name_key).ToLocal(&db_name_value)) { - return; - } - - if (!db_name_value->IsUndefined()) { - if (!db_name_value->IsString()) { - THROW_ERR_INVALID_ARG_TYPE( - env->isolate(), - "The \"options.dbName\" argument must be a string."); - return; - } - db_name = - Utf8Value(env->isolate(), db_name_value.As()).ToString(); - } + if (!ReadOptionalString(env, options, "dbName", &db_name)) return; } // Reading the options bag above can run user JavaScript through a property @@ -2843,46 +2853,8 @@ void Database::CreateSession(const FunctionCallbackInfo& args) { } Local options = args[0].As(); - - Local table_key = env->table_string(); - bool hasIt; - if (!options->HasOwnProperty(env->context(), table_key).To(&hasIt)) { - return; - } - if (hasIt) { - Local table_value; - if (!options->Get(env->context(), table_key).ToLocal(&table_value)) { - return; - } - - if (table_value->IsString()) { - table = Utf8Value(env->isolate(), table_value).ToString(); - } else { - THROW_ERR_INVALID_ARG_TYPE( - env->isolate(), "The \"options.table\" argument must be a string."); - return; - } - } - - Local db_key = FIXED_ONE_BYTE_STRING(env->isolate(), "db"); - - if (!options->HasOwnProperty(env->context(), db_key).To(&hasIt)) { - return; - } - if (hasIt) { - Local db_value; - if (!options->Get(env->context(), db_key).ToLocal(&db_value)) { - // An error will have been scheduled. - return; - } - if (db_value->IsString()) { - db_name = Utf8Value(env->isolate(), db_value).ToString(); - } else { - THROW_ERR_INVALID_ARG_TYPE( - env->isolate(), "The \"options.db\" argument must be a string."); - return; - } - } + if (!ReadOptionalString(env, options, "table", &table)) return; + if (!ReadOptionalString(env, options, "db", &db_name)) return; } // Reading the options bag above can run user JavaScript through a property @@ -2965,39 +2937,8 @@ void Backup(const FunctionCallbackInfo& args) { } } - Local source_v; - if (!options->Get(env->context(), env->source_string()) - .ToLocal(&source_v)) { - return; - } - - if (!source_v->IsUndefined()) { - if (!source_v->IsString()) { - THROW_ERR_INVALID_ARG_TYPE( - env->isolate(), - "The \"options.source\" argument must be a string."); - return; - } - - source_db = Utf8Value(env->isolate(), source_v.As()).ToString(); - } - - Local target_v; - if (!options->Get(env->context(), env->target_string()) - .ToLocal(&target_v)) { - return; - } - - if (!target_v->IsUndefined()) { - if (!target_v->IsString()) { - THROW_ERR_INVALID_ARG_TYPE( - env->isolate(), - "The \"options.target\" argument must be a string."); - return; - } - - dest_db = Utf8Value(env->isolate(), target_v.As()).ToString(); - } + if (!ReadOptionalString(env, options, "source", &source_db)) return; + if (!ReadOptionalString(env, options, "target", &dest_db)) return; Local progress_v; if (!options->Get(env->context(), env->progress_string()) diff --git a/test/parallel/test-sqlite-backup.mjs b/test/parallel/test-sqlite-backup.mjs index d3d6a433993b..212bf54e98ca 100644 --- a/test/parallel/test-sqlite-backup.mjs +++ b/test/parallel/test-sqlite-backup.mjs @@ -195,6 +195,20 @@ test('database backup', async (t) => { }); }); +test('backup treats undefined source and target as main', async (t) => { + const database = makeSourceDb(); + const destDb = nextDb(); + + await backup(database, destDb, { source: undefined, target: undefined }); + + const backupDb = new Database(destDb); + t.after(() => { + database.close(); + backupDb.close(); + }); + t.assert.strictEqual(backupDb.prepare('SELECT count(*) AS n FROM main.data').get().n, 2); +}); + test('backup database using location as URL', async (t) => { const database = makeSourceDb(); const destDb = pathToFileURL(nextDb()); diff --git a/test/parallel/test-sqlite-serialize.js b/test/parallel/test-sqlite-serialize.js index 29e02699cb0c..f52d4268f11f 100644 --- a/test/parallel/test-sqlite-serialize.js +++ b/test/parallel/test-sqlite-serialize.js @@ -251,6 +251,18 @@ suite('Database.prototype.deserialize()', () => { db.close(); }); + test('treats undefined options.dbName as main', (t) => { + const db1 = new Database(':memory:'); + db1.exec('CREATE TABLE t(x INTEGER)'); + const buf = db1.serialize(); + db1.close(); + + const db2 = new Database(':memory:'); + db2.deserialize(buf, { dbName: undefined }); + t.assert.strictEqual(db2.prepare('SELECT count(*) AS n FROM main.t').get().n, 0); + db2.close(); + }); + test('accepts a Buffer as input', (t) => { const db1 = new Database(':memory:'); db1.exec('CREATE TABLE t(x INTEGER)'); diff --git a/test/parallel/test-sqlite-session.js b/test/parallel/test-sqlite-session.js index e5a478e17061..ee0c2cfb3af9 100644 --- a/test/parallel/test-sqlite-session.js +++ b/test/parallel/test-sqlite-session.js @@ -540,6 +540,14 @@ test('database.createSession() - specify other database', (t) => { t.assert.strictEqual(sessionTest.changeset().length, 0); }); +test('database.createSession() - undefined options use defaults', (t) => { + const database = new Database(':memory:'); + const session = database.createSession({ table: undefined, db: undefined }); + database.exec('CREATE TABLE data (key INTEGER PRIMARY KEY)'); + database.exec('INSERT INTO data (key) VALUES (1)'); + t.assert.notStrictEqual(session.changeset().length, 0); +}); + test('database.createSession() - wrong arguments', (t) => { const database = new Database(':memory:'); t.assert.throws(() => {