diff --git a/src/env_properties.h b/src/env_properties.h index fc1772f926e5..8a1fa2daa117 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -478,6 +478,7 @@ V(socketaddress_constructor_template, v8::FunctionTemplate) \ V(space_stats_template, v8::DictionaryTemplate) \ V(sqlite_column_template, v8::DictionaryTemplate) \ + V(sqlite_database_sync_constructor_template, v8::FunctionTemplate) \ V(sqlite_limits_template, v8::ObjectTemplate) \ V(sqlite_run_result_template, v8::DictionaryTemplate) \ V(sqlite_statement_sync_constructor_template, v8::FunctionTemplate) \ diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 76accf1c2731..4ae04615f073 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -82,6 +82,23 @@ inline MaybeLocal Utf8StringMaybeOneByte(Isolate* isolate, isolate, input.data(), NewStringType::kNormal, len); } +static inline void SetSideEffectFreeGetter( + Isolate* isolate, + Local class_template, + Local name, + FunctionCallback fn) { + Local getter = + FunctionTemplate::New(isolate, + fn, + Local(), + v8::Signature::New(isolate, class_template), + /* length */ 0, + ConstructorBehavior::kThrow, + SideEffectType::kHasNoSideEffect); + class_template->InstanceTemplate()->SetAccessorProperty( + name, getter, Local(), DontDelete); +} + BindingData::BindingData(Realm* realm, Local wrap) : BaseObject(realm, wrap) { MakeWeak(); @@ -1279,12 +1296,21 @@ std::optional ValidateDatabasePath(Environment* env, } else if (path->IsObject()) { // When is URL auto url = path.As(); Local href; - if (url->Get(env->context(), env->href_string()).ToLocal(&href) && - href->IsString()) { + // Let an exception thrown by the href getter propagate instead of + // replacing it with ERR_INVALID_ARG_TYPE. + if (!url->Get(env->context(), env->href_string()).ToLocal(&href)) { + return std::nullopt; + } + if (href->IsString()) { Utf8Value location_value(env->isolate(), href.As()); auto location = location_value.ToStringView(); if (!has_null_bytes(location)) { - CHECK(ada::can_parse(location)); + // A real URL always has a parseable href, but any object with a string + // href reaches this branch, so the value cannot be assumed to be one. + if (!ada::can_parse(location)) { + THROW_ERR_INVALID_URL(env->isolate(), "Invalid URL"); + return std::nullopt; + } if (!location.starts_with("file:")) { THROW_ERR_INVALID_URL_SCHEME(env->isolate()); return std::nullopt; @@ -1303,6 +1329,62 @@ std::optional ValidateDatabasePath(Environment* env, return std::nullopt; } +Local DatabaseSync::GetConstructorTemplate(Environment* env) { + Local tmpl = + env->sqlite_database_sync_constructor_template(); + if (tmpl.IsEmpty()) { + Isolate* isolate = env->isolate(); + tmpl = NewFunctionTemplate(isolate, DatabaseSync::New); + tmpl->InstanceTemplate()->SetInternalFieldCount( + DatabaseSync::kInternalFieldCount); + SetProtoMethod(isolate, tmpl, "open", DatabaseSync::Open); + SetProtoMethod(isolate, tmpl, "close", DatabaseSync::Close); + SetProtoDispose(isolate, tmpl, DatabaseSync::Dispose); + SetProtoMethod(isolate, tmpl, "prepare", DatabaseSync::Prepare); + SetProtoMethod(isolate, tmpl, "exec", DatabaseSync::Exec); + SetProtoMethod(isolate, tmpl, "function", DatabaseSync::CustomFunction); + SetProtoMethod( + isolate, tmpl, "createTagStore", DatabaseSync::CreateTagStore); + SetProtoMethodNoSideEffect( + isolate, tmpl, "location", DatabaseSync::Location); + SetProtoMethod(isolate, tmpl, "aggregate", DatabaseSync::AggregateFunction); + SetProtoMethod(isolate, tmpl, "createSession", DatabaseSync::CreateSession); + SetProtoMethod( + isolate, tmpl, "applyChangeset", DatabaseSync::ApplyChangeset); + SetProtoMethod(isolate, + tmpl, + "enableLoadExtension", + DatabaseSync::EnableLoadExtension); + SetProtoMethod( + isolate, tmpl, "enableDefensive", DatabaseSync::EnableDefensive); + SetProtoMethod(isolate, tmpl, "loadExtension", DatabaseSync::LoadExtension); + SetProtoMethod(isolate, tmpl, "serialize", DatabaseSync::Serialize); + SetProtoMethod(isolate, tmpl, "deserialize", DatabaseSync::Deserialize); + SetProtoMethod(isolate, tmpl, "setAuthorizer", DatabaseSync::SetAuthorizer); + SetSideEffectFreeGetter(isolate, + tmpl, + FIXED_ONE_BYTE_STRING(isolate, "isOpen"), + DatabaseSync::IsOpenGetter); + SetSideEffectFreeGetter(isolate, + tmpl, + FIXED_ONE_BYTE_STRING(isolate, "isTransaction"), + DatabaseSync::IsTransactionGetter); + SetSideEffectFreeGetter(isolate, + tmpl, + FIXED_ONE_BYTE_STRING(isolate, "limits"), + DatabaseSync::LimitsGetter); + Local sqlite_type_key = + FIXED_ONE_BYTE_STRING(isolate, "sqlite-type"); + Local sqlite_type_symbol = + v8::Symbol::For(isolate, sqlite_type_key); + Local database_sync_string = + FIXED_ONE_BYTE_STRING(isolate, "node:sqlite"); + tmpl->InstanceTemplate()->Set(sqlite_type_symbol, database_sync_string); + env->set_sqlite_database_sync_constructor_template(tmpl); + } + return tmpl; +} + void DatabaseSync::New(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); if (!args.IsConstructCall()) { @@ -2419,9 +2501,13 @@ void DatabaseSync::CreateSession(const FunctionCallbackInfo& args) { void Backup(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - if (args.Length() < 1 || !args[0]->IsObject()) { - THROW_ERR_INVALID_ARG_TYPE(env->isolate(), - "The \"sourceDb\" argument must be an object."); + // Unlike the other unwrap sites in this file, which rely on V8's signature + // check for args.This(), this one takes a value out of args[] and so has to + // check the type itself before unwrapping it. + if (!DatabaseSync::GetConstructorTemplate(env)->HasInstance(args[0])) { + THROW_ERR_INVALID_ARG_TYPE( + env->isolate(), + "The \"sourceDb\" argument must be an instance of DatabaseSync."); return; } @@ -3811,23 +3897,6 @@ SQLTagStore::SQLTagStore(Environment* env, MakeWeak(); } -static inline void SetSideEffectFreeGetter( - Isolate* isolate, - Local class_template, - Local name, - FunctionCallback fn) { - Local getter = - FunctionTemplate::New(isolate, - fn, - Local(), - v8::Signature::New(isolate, class_template), - /* length */ 0, - ConstructorBehavior::kThrow, - SideEffectType::kHasNoSideEffect); - class_template->InstanceTemplate()->SetAccessorProperty( - name, getter, Local(), DontDelete); -} - SQLTagStore::~SQLTagStore() {} Local SQLTagStore::GetConstructorTemplate(Environment* env) { @@ -4569,62 +4638,14 @@ static void Initialize(Local target, } }); } - Local db_tmpl = - NewFunctionTemplate(isolate, DatabaseSync::New); - db_tmpl->InstanceTemplate()->SetInternalFieldCount( - DatabaseSync::kInternalFieldCount); Local constants = Object::New(isolate); DefineConstants(constants); - SetProtoMethod(isolate, db_tmpl, "open", DatabaseSync::Open); - SetProtoMethod(isolate, db_tmpl, "close", DatabaseSync::Close); - SetProtoDispose(isolate, db_tmpl, DatabaseSync::Dispose); - SetProtoMethod(isolate, db_tmpl, "prepare", DatabaseSync::Prepare); - SetProtoMethod(isolate, db_tmpl, "exec", DatabaseSync::Exec); - SetProtoMethod(isolate, db_tmpl, "function", DatabaseSync::CustomFunction); - SetProtoMethod( - isolate, db_tmpl, "createTagStore", DatabaseSync::CreateTagStore); - SetProtoMethodNoSideEffect( - isolate, db_tmpl, "location", DatabaseSync::Location); - SetProtoMethod( - isolate, db_tmpl, "aggregate", DatabaseSync::AggregateFunction); - SetProtoMethod( - isolate, db_tmpl, "createSession", DatabaseSync::CreateSession); - SetProtoMethod( - isolate, db_tmpl, "applyChangeset", DatabaseSync::ApplyChangeset); - SetProtoMethod(isolate, - db_tmpl, - "enableLoadExtension", - DatabaseSync::EnableLoadExtension); - SetProtoMethod( - isolate, db_tmpl, "enableDefensive", DatabaseSync::EnableDefensive); - SetProtoMethod( - isolate, db_tmpl, "loadExtension", DatabaseSync::LoadExtension); - SetProtoMethod(isolate, db_tmpl, "serialize", DatabaseSync::Serialize); - SetProtoMethod(isolate, db_tmpl, "deserialize", DatabaseSync::Deserialize); - SetProtoMethod( - isolate, db_tmpl, "setAuthorizer", DatabaseSync::SetAuthorizer); - SetSideEffectFreeGetter(isolate, - db_tmpl, - FIXED_ONE_BYTE_STRING(isolate, "isOpen"), - DatabaseSync::IsOpenGetter); - SetSideEffectFreeGetter(isolate, - db_tmpl, - FIXED_ONE_BYTE_STRING(isolate, "isTransaction"), - DatabaseSync::IsTransactionGetter); - SetSideEffectFreeGetter(isolate, - db_tmpl, - FIXED_ONE_BYTE_STRING(isolate, "limits"), - DatabaseSync::LimitsGetter); - Local sqlite_type_key = FIXED_ONE_BYTE_STRING(isolate, "sqlite-type"); - Local sqlite_type_symbol = - v8::Symbol::For(isolate, sqlite_type_key); - Local database_sync_string = - FIXED_ONE_BYTE_STRING(isolate, "node:sqlite"); - db_tmpl->InstanceTemplate()->Set(sqlite_type_symbol, database_sync_string); - - SetConstructorFunction(context, target, "DatabaseSync", db_tmpl); + SetConstructorFunction(context, + target, + "DatabaseSync", + DatabaseSync::GetConstructorTemplate(env)); SetConstructorFunction(context, target, "StatementSync", diff --git a/src/node_sqlite.h b/src/node_sqlite.h index 306a47f6c47f..a80ec479fac6 100644 --- a/src/node_sqlite.h +++ b/src/node_sqlite.h @@ -226,6 +226,8 @@ class DatabaseSync : public BaseObject { bool open, bool allow_load_extension); void MemoryInfo(MemoryTracker* tracker) const override; + static v8::Local GetConstructorTemplate( + Environment* env); static void New(const v8::FunctionCallbackInfo& args); static void Open(const v8::FunctionCallbackInfo& args); static void IsOpenGetter(const v8::FunctionCallbackInfo& args); diff --git a/test/parallel/test-sqlite-backup.mjs b/test/parallel/test-sqlite-backup.mjs index f995ae3ca72a..6528418eaf65 100644 --- a/test/parallel/test-sqlite-backup.mjs +++ b/test/parallel/test-sqlite-backup.mjs @@ -48,10 +48,31 @@ describe('backup()', () => { backup(); }, { code: 'ERR_INVALID_ARG_TYPE', - message: 'The "sourceDb" argument must be an object.' + message: 'The "sourceDb" argument must be an instance of DatabaseSync.' }); }); + test('throws if the source database is not a DatabaseSync', (t) => { + const database = makeSourceDb(); + const values = [ + {}, + [], + { p0: 1, p1: 2, p2: 3, p3: 4 }, + { __proto__: DatabaseSync.prototype }, + database.prepare('SELECT 1'), + database.createSession(), + ]; + + for (const value of values) { + t.assert.throws(() => { + backup(value, nextDb()); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: 'The "sourceDb" argument must be an instance of DatabaseSync.' + }); + } + }); + test('throws if path is not a string, URL, or Buffer', (t) => { const database = makeSourceDb(); @@ -88,6 +109,17 @@ describe('backup()', () => { }); }); + test('throws if the database path has an unparsable href', (t) => { + const database = makeSourceDb(); + + t.assert.throws(() => { + backup(database, { href: 'zzz' }); + }, { + code: 'ERR_INVALID_URL', + message: 'Invalid URL' + }); + }); + test('throws if options is not an object', (t) => { const database = makeSourceDb(); diff --git a/test/parallel/test-sqlite-database-sync.js b/test/parallel/test-sqlite-database-sync.js index 08a636c9cbdc..4e0c7f742f85 100644 --- a/test/parallel/test-sqlite-database-sync.js +++ b/test/parallel/test-sqlite-database-sync.js @@ -51,6 +51,24 @@ suite('DatabaseSync() constructor', () => { }); }); + test('throws if the database location has an unparsable href', (t) => { + t.assert.throws(() => { + new DatabaseSync({ href: 'zzz' }); + }, { + code: 'ERR_INVALID_URL', + message: 'Invalid URL', + }); + }); + + test('propagates an exception thrown by the href getter', (t) => { + t.assert.throws(() => { + new DatabaseSync({ get href() { throw new RangeError('boom'); } }); + }, { + name: 'RangeError', + message: 'boom', + }); + }); + test('throws if options is provided but is not an object', (t) => { t.assert.throws(() => { new DatabaseSync('foo', null);