nodejs/node · #66748

sqlite: propagate option getter errors and dedupe boolean options

araujogui · merged Oct 22, 20262 files · 97 + / 386 −
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);