nodejs/node · #66748
sqlite: propagate option getter errors and dedupe boolean options
src/node_sqlite.cc79 + / 386 −
@@ -1818,6 +1818,32 @@ void Database::CreateTagStore(const FunctionCallbackInfo<Value>& args) { args.GetReturnValue().Set(session->object()); } +// Reads an optional boolean property, leaving |out| unchanged when it is+// undefined. Returns false if a JavaScript exception is pending, either from+// the property getter or because the value is not a boolean.+template <typename T>+static bool ReadOptionalBool(Environment* env,+ Local<Object> options,+ const char* key,+ T* out) {+ Local<String> key_string =+ OneByteString(env->isolate(), key, -1, NewStringType::kInternalized);+ Local<Value> value;+ if (!options->Get(env->context(), key_string).ToLocal(&value)) {+ return false;+ }+ if (value->IsUndefined()) {+ return true;+ }+ if (!value->IsBoolean()) {+ THROW_ERR_INVALID_ARG_TYPE(+ env->isolate(), "The \"options.%s\" argument must be a boolean.", key);+ return false;+ }+ *out = value->IsTrue();+ return true;+}+ std::optional<std::string> ValidateDatabasePath(Environment* env, Local<Value> path, const std::string& field_name) {@@ -1893,90 +1919,23 @@ void Database::New(const FunctionCallbackInfo<Value>& args) { } Local<Object> options = args[1].As<Object>();- Local<String> open_string = FIXED_ONE_BYTE_STRING(env->isolate(), "open");- Local<Value> open_v;- if (!options->Get(env->context(), open_string).ToLocal(&open_v)) {+ bool read_only = open_config.get_read_only();+ bool enable_foreign_keys = open_config.get_enable_foreign_keys();+ bool enable_dqs = open_config.get_enable_dqs();+ if (!ReadOptionalBool(env, options, "open", &open)) return;+ if (!ReadOptionalBool(env, options, "readOnly", &read_only)) return;+ if (!ReadOptionalBool(+ env, options, "enableForeignKeyConstraints", &enable_foreign_keys)) return;- }- if (!open_v->IsUndefined()) {- if (!open_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(), "The \"options.open\" argument must be a boolean.");- return;- }- open = open_v.As<Boolean>()->Value();- }-- Local<String> read_only_string =- FIXED_ONE_BYTE_STRING(env->isolate(), "readOnly");- Local<Value> read_only_v;- if (!options->Get(env->context(), read_only_string).ToLocal(&read_only_v)) {+ if (!ReadOptionalBool(+ env, options, "enableDoubleQuotedStringLiterals", &enable_dqs)) return;- }- if (!read_only_v->IsUndefined()) {- if (!read_only_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- "The \"options.readOnly\" argument must be a boolean.");- return;- }- open_config.set_read_only(read_only_v.As<Boolean>()->Value());- }-- Local<String> enable_foreign_keys_string =- FIXED_ONE_BYTE_STRING(env->isolate(), "enableForeignKeyConstraints");- Local<Value> enable_foreign_keys_v;- if (!options->Get(env->context(), enable_foreign_keys_string)- .ToLocal(&enable_foreign_keys_v)) {+ if (!ReadOptionalBool(+ env, options, "allowExtension", &allow_load_extension)) return;- }- if (!enable_foreign_keys_v->IsUndefined()) {- if (!enable_foreign_keys_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- "The \"options.enableForeignKeyConstraints\" argument must be a "- "boolean.");- return;- }- open_config.set_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()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- "The \"options.enableDoubleQuotedStringLiterals\" argument must be "- "a boolean.");- return;- }- open_config.set_enable_dqs(enable_dqs_v.As<Boolean>()->Value());- }-- Local<String> allow_extension_string =- FIXED_ONE_BYTE_STRING(env->isolate(), "allowExtension");- Local<Value> allow_extension_v;- if (!options->Get(env->context(), allow_extension_string)- .ToLocal(&allow_extension_v)) {- return;- }-- if (!allow_extension_v->IsUndefined()) {- if (!allow_extension_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- "The \"options.allowExtension\" argument must be a boolean.");- return;- }- allow_load_extension = allow_extension_v.As<Boolean>()->Value();- }+ open_config.set_read_only(read_only);+ open_config.set_enable_foreign_keys(enable_foreign_keys);+ open_config.set_enable_dqs(enable_dqs); Local<Value> timeout_v; if (!options->Get(env->context(), env->timeout_string())@@ -1995,80 +1954,28 @@ void Database::New(const FunctionCallbackInfo<Value>& args) { open_config.set_timeout(timeout_v.As<Int32>()->Value()); } - Local<Value> read_bigints_v;- if (options->Get(env->context(), env->read_bigints_string())- .ToLocal(&read_bigints_v)) {- if (!read_bigints_v->IsUndefined()) {- if (!read_bigints_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- R"(The "options.readBigInts" argument must be a boolean.)");- return;- }- open_config.set_use_big_ints(read_bigints_v.As<Boolean>()->Value());- }- }-- Local<Value> return_arrays_v;- if (options->Get(env->context(), env->return_arrays_string())- .ToLocal(&return_arrays_v)) {- if (!return_arrays_v->IsUndefined()) {- if (!return_arrays_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- R"(The "options.returnArrays" argument must be a boolean.)");- return;- }- open_config.set_return_arrays(return_arrays_v.As<Boolean>()->Value());- }- }-- Local<Value> allow_bare_named_params_v;- if (options->Get(env->context(), env->allow_bare_named_params_string())- .ToLocal(&allow_bare_named_params_v)) {- if (!allow_bare_named_params_v->IsUndefined()) {- if (!allow_bare_named_params_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- R"(The "options.allowBareNamedParameters" )"- "argument must be a boolean.");- return;- }- open_config.set_allow_bare_named_params(- allow_bare_named_params_v.As<Boolean>()->Value());- }- }-- Local<Value> allow_unknown_named_params_v;- if (options->Get(env->context(), env->allow_unknown_named_params_string())- .ToLocal(&allow_unknown_named_params_v)) {- if (!allow_unknown_named_params_v->IsUndefined()) {- if (!allow_unknown_named_params_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- R"(The "options.allowUnknownNamedParameters" )"- "argument must be a boolean.");- return;- }- open_config.set_allow_unknown_named_params(- allow_unknown_named_params_v.As<Boolean>()->Value());- }- }-- Local<Value> defensive_v;- if (!options->Get(env->context(), env->defensive_string())- .ToLocal(&defensive_v)) {+ bool use_big_ints = open_config.get_use_big_ints();+ bool return_arrays = open_config.get_return_arrays();+ bool allow_bare_named_params = open_config.get_allow_bare_named_params();+ bool allow_unknown_named_params =+ open_config.get_allow_unknown_named_params();+ bool defensive = open_config.get_enable_defensive();+ if (!ReadOptionalBool(env, options, "readBigInts", &use_big_ints)) return;+ if (!ReadOptionalBool(env, options, "returnArrays", &return_arrays)) return;+ if (!ReadOptionalBool(+ env, options, "allowBareNamedParameters", &allow_bare_named_params)) return;- }- if (!defensive_v->IsUndefined()) {- if (!defensive_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- "The \"options.defensive\" argument must be a boolean.");- return;- }- open_config.set_enable_defensive(defensive_v.As<Boolean>()->Value());- }+ if (!ReadOptionalBool(env,+ options,+ "allowUnknownNamedParameters",+ &allow_unknown_named_params))+ return;+ if (!ReadOptionalBool(env, options, "defensive", &defensive)) return;+ open_config.set_use_big_ints(use_big_ints);+ open_config.set_return_arrays(return_arrays);+ open_config.set_allow_bare_named_params(allow_bare_named_params);+ open_config.set_allow_unknown_named_params(allow_unknown_named_params);+ open_config.set_enable_defensive(defensive); // Parse limits option Local<Value> limits_v;@@ -2218,85 +2125,17 @@ void Database::Prepare(const FunctionCallbackInfo<Value>& args) { return; } Local<Object> options = args[1].As<Object>();-- Local<Value> return_arrays_v;- if (!options->Get(env->context(), env->return_arrays_string())- .ToLocal(&return_arrays_v)) {+ if (!ReadOptionalBool(env, options, "returnArrays", &return_arrays)) return;+ if (!ReadOptionalBool(env, options, "readBigInts", &use_big_ints)) return;+ if (!ReadOptionalBool(+ env, options, "allowBareNamedParameters", &allow_bare_named_params)) return;- }- if (!return_arrays_v->IsUndefined()) {- if (!return_arrays_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- "The \"options.returnArrays\" argument must be a boolean.");- return;- }- return_arrays = return_arrays_v->IsTrue();- }-- Local<Value> read_big_ints_v;- if (!options->Get(env->context(), env->read_bigints_string())- .ToLocal(&read_big_ints_v)) {- return;- }- if (!read_big_ints_v->IsUndefined()) {- if (!read_big_ints_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- "The \"options.readBigInts\" argument must be a boolean.");- return;- }- use_big_ints = read_big_ints_v->IsTrue();- }-- Local<Value> allow_bare_named_params_v;- if (!options->Get(env->context(), env->allow_bare_named_params_string())- .ToLocal(&allow_bare_named_params_v)) {- return;- }- if (!allow_bare_named_params_v->IsUndefined()) {- if (!allow_bare_named_params_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- "The \"options.allowBareNamedParameters\" argument must be a "- "boolean.");- return;- }- allow_bare_named_params = allow_bare_named_params_v->IsTrue();- }-- Local<Value> allow_unknown_named_params_v;- if (!options->Get(env->context(), env->allow_unknown_named_params_string())- .ToLocal(&allow_unknown_named_params_v)) {+ if (!ReadOptionalBool(env,+ options,+ "allowUnknownNamedParameters",+ &allow_unknown_named_params)) return;- }- if (!allow_unknown_named_params_v->IsUndefined()) {- if (!allow_unknown_named_params_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- "The \"options.allowUnknownNamedParameters\" argument must be a "- "boolean.");- return;- }- allow_unknown_named_params = allow_unknown_named_params_v->IsTrue();- }-- Local<Value> persistent_v;- if (!options- ->Get(env->context(),- FIXED_ONE_BYTE_STRING(env->isolate(), "persistent"))- .ToLocal(&persistent_v)) {- return;- }- if (!persistent_v->IsUndefined()) {- if (!persistent_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- "The \"options.persistent\" argument must be a boolean.");- return;- }- persistent = persistent_v->IsTrue();- }+ if (!ReadOptionalBool(env, options, "persistent", &persistent)) return; } // Reading the options bag above can run user JavaScript through a property@@ -2400,77 +2239,12 @@ void Database::CustomFunction(const FunctionCallbackInfo<Value>& args) { } Local<Object> options = args[1].As<Object>();- Local<Value> use_bigint_args_v;- if (!options- ->Get(env->context(),- FIXED_ONE_BYTE_STRING(env->isolate(), "useBigIntArguments"))- .ToLocal(&use_bigint_args_v)) {+ if (!ReadOptionalBool(env, options, "useBigIntArguments", &use_bigint_args)) return;- }-- if (!use_bigint_args_v->IsUndefined()) {- if (!use_bigint_args_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- "The \"options.useBigIntArguments\" argument must be a boolean.");- return;- }- use_bigint_args = use_bigint_args_v.As<Boolean>()->Value();- }-- Local<Value> varargs_v;- if (!options- ->Get(env->context(),- FIXED_ONE_BYTE_STRING(env->isolate(), "varargs"))- .ToLocal(&varargs_v)) {+ if (!ReadOptionalBool(env, options, "varargs", &varargs)) return;+ if (!ReadOptionalBool(env, options, "deterministic", &deterministic)) return;- }-- if (!varargs_v->IsUndefined()) {- if (!varargs_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- "The \"options.varargs\" argument must be a boolean.");- return;- }- varargs = varargs_v.As<Boolean>()->Value();- }-- Local<Value> deterministic_v;- if (!options- ->Get(env->context(),- FIXED_ONE_BYTE_STRING(env->isolate(), "deterministic"))- .ToLocal(&deterministic_v)) {- return;- }-- if (!deterministic_v->IsUndefined()) {- if (!deterministic_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- "The \"options.deterministic\" argument must be a boolean.");- return;- }- deterministic = deterministic_v.As<Boolean>()->Value();- }-- Local<Value> direct_only_v;- if (!options- ->Get(env->context(),- FIXED_ONE_BYTE_STRING(env->isolate(), "directOnly"))- .ToLocal(&direct_only_v)) {- return;- }-- if (!direct_only_v->IsUndefined()) {- if (!direct_only_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- "The \"options.directOnly\" argument must be a boolean.");- return;- }- direct_only = direct_only_v.As<Boolean>()->Value();- }+ if (!ReadOptionalBool(env, options, "directOnly", &direct_only)) return; } if (!args[fn_index]->IsFunction()) {@@ -2750,60 +2524,11 @@ void Database::AggregateFunction(const FunctionCallbackInfo<Value>& args) { bool use_bigint_args = false; bool varargs = false; bool direct_only = false;- Local<Value> use_bigint_args_v; Local<Function> inverseFunc = Local<Function>();- if (!options- ->Get(env->context(),- FIXED_ONE_BYTE_STRING(env->isolate(), "useBigIntArguments"))- .ToLocal(&use_bigint_args_v)) {+ if (!ReadOptionalBool(env, options, "useBigIntArguments", &use_bigint_args)) return;- }-- if (!use_bigint_args_v->IsUndefined()) {- if (!use_bigint_args_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- "The \"options.useBigIntArguments\" argument must be a boolean.");- return;- }- use_bigint_args = use_bigint_args_v.As<Boolean>()->Value();- }-- Local<Value> varargs_v;- if (!options- ->Get(env->context(),- FIXED_ONE_BYTE_STRING(env->isolate(), "varargs"))- .ToLocal(&varargs_v)) {- return;- }-- if (!varargs_v->IsUndefined()) {- if (!varargs_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- "The \"options.varargs\" argument must be a boolean.");- return;- }- varargs = varargs_v.As<Boolean>()->Value();- }-- Local<Value> direct_only_v;- if (!options- ->Get(env->context(),- FIXED_ONE_BYTE_STRING(env->isolate(), "directOnly"))- .ToLocal(&direct_only_v)) {- return;- }-- if (!direct_only_v->IsUndefined()) {- if (!direct_only_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- "The \"options.directOnly\" argument must be a boolean.");- return;- }- direct_only = direct_only_v.As<Boolean>()->Value();- }+ if (!ReadOptionalBool(env, options, "varargs", &varargs)) return;+ if (!ReadOptionalBool(env, options, "directOnly", &direct_only)) return; Local<Value> inverse_v; if (!options->Get(env->context(), env->inverse_string())@@ -2954,42 +2679,10 @@ void Database::CreateModule(const FunctionCallbackInfo<Value>& args) { // Extract optional boolean options. bool direct_only = false;- Local<Value> direct_only_v;- if (!options- ->Get(env->context(),- FIXED_ONE_BYTE_STRING(env->isolate(), "directOnly"))- .ToLocal(&direct_only_v)) {- return;- }-- if (!direct_only_v->IsUndefined()) {- if (!direct_only_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- "The \"options.directOnly\" argument must be a boolean.");- return;- }- direct_only = direct_only_v.As<Boolean>()->Value();- }- bool use_bigint_args = false;- Local<Value> use_bigint_args_v;- if (!options- ->Get(env->context(),- FIXED_ONE_BYTE_STRING(env->isolate(), "useBigIntArguments"))- .ToLocal(&use_bigint_args_v)) {+ if (!ReadOptionalBool(env, options, "directOnly", &direct_only)) return;+ if (!ReadOptionalBool(env, options, "useBigIntArguments", &use_bigint_args)) return;- }-- if (!use_bigint_args_v->IsUndefined()) {- if (!use_bigint_args_v->IsBoolean()) {- THROW_ERR_INVALID_ARG_TYPE(- env->isolate(),- "The \"options.useBigIntArguments\" argument must be a boolean.");- return;- }- use_bigint_args = use_bigint_args_v.As<Boolean>()->Value();- } // Build CREATE TABLE schema SQL from columns. std::string schema_sql = "CREATE TABLE x(";test/parallel/test-sqlite-database.js18 + / 0 −
@@ -279,6 +279,24 @@ suite('Database() constructor', () => { ); }); + for (const option of [+ 'readBigInts',+ 'returnArrays',+ 'allowBareNamedParameters',+ 'allowUnknownNamedParameters',+ ]) {+ test(`propagates an error thrown by the options.${option} getter`, (t) => {+ const dbPath = nextDb();+ const error = new Error('boom');+ t.assert.throws(() => {+ new Database(dbPath, {+ get [option]() { throw error; },+ });+ }, error);+ t.assert.strictEqual(existsSync(dbPath), false);+ });+ }+ test('has sqlite-type symbol property', (t) => { const dbPath = nextDb(); using db = new Database(dbPath);