From d8416b717d32dec78b43020104ee0788a3e56058 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Mon, 26 Aug 2024 19:20:09 +0200 Subject: [PATCH] src: handle errors correctly in webstorage PR-URL: https://github.com/nodejs/node/pull/54544 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Mohammed Keyvanzadeh --- src/node_webstorage.cc | 178 +++++++++++++++++++++-------------------- src/node_webstorage.h | 16 ++-- 2 files changed, 99 insertions(+), 95 deletions(-) diff --git a/src/node_webstorage.cc b/src/node_webstorage.cc index 15d142e3d8f6e6..9974101338ffac 100644 --- a/src/node_webstorage.cc +++ b/src/node_webstorage.cc @@ -24,12 +24,16 @@ using v8::IndexedPropertyHandlerConfiguration; using v8::Integer; using v8::Intercepted; using v8::Isolate; +using v8::JustVoid; using v8::Local; using v8::Map; using v8::Maybe; using v8::MaybeLocal; using v8::Name; using v8::NamedPropertyHandlerConfiguration; +using v8::NewStringType; +using v8::Nothing; +using v8::Null; using v8::Object; using v8::PropertyAttribute; using v8::PropertyCallbackInfo; @@ -96,7 +100,7 @@ void Storage::MemoryInfo(MemoryTracker* tracker) const { tracker->TrackField("location", location_); } -bool Storage::Open() { +Maybe Storage::Open() { static const int kCurrentSchemaVersion = 1; static constexpr std::string_view get_schema_version_sql = "SELECT schema_version FROM nodejs_webstorage_state"; @@ -161,22 +165,23 @@ bool Storage::Open() { sqlite3* db = db_.get(); if (db != nullptr) { - return true; + return JustVoid(); } int r = sqlite3_open(location_.c_str(), &db); - CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, false); + CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Nothing()); r = sqlite3_exec(db, init_sql_v0.data(), 0, 0, nullptr); - CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, false); + CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Nothing()); // Get the current schema version, used to determine schema migrations. sqlite3_stmt* s = nullptr; r = sqlite3_prepare_v2( db, get_schema_version_sql.data(), get_schema_version_sql.size(), &s, 0); r = sqlite3_exec(db, init_sql_v0.data(), 0, 0, nullptr); - CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, false); + CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Nothing()); auto stmt = stmt_unique_ptr(s); - CHECK_ERROR_OR_THROW(env(), sqlite3_step(stmt.get()), SQLITE_ROW, false); + CHECK_ERROR_OR_THROW( + env(), sqlite3_step(stmt.get()), SQLITE_ROW, Nothing()); CHECK(sqlite3_column_type(stmt.get(), 0) == SQLITE_INTEGER); int schema_version = sqlite3_column_int(stmt.get(), 0); stmt = nullptr; // Force finalization. @@ -184,7 +189,7 @@ bool Storage::Open() { if (schema_version > kCurrentSchemaVersion) { THROW_ERR_INVALID_STATE( env(), "localStorage was created with a newer version of Node.js"); - return false; + return Nothing(); } if (schema_version < kCurrentSchemaVersion) { @@ -193,11 +198,11 @@ bool Storage::Open() { "UPDATE nodejs_webstorage_state SET schema_version = " + std::to_string(kCurrentSchemaVersion) + ";"; r = sqlite3_exec(db, set_user_version_sql.c_str(), 0, 0, nullptr); - CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, false); + CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Nothing()); } db_ = conn_unique_ptr(db); - return true; + return JustVoid(); } void Storage::New(const FunctionCallbackInfo& args) { @@ -222,9 +227,9 @@ void Storage::New(const FunctionCallbackInfo& args) { new Storage(env, args.This(), location.ToStringView()); } -void Storage::Clear() { - if (!Open()) { - return; +Maybe Storage::Clear() { + if (!Open().IsJust()) { + return Nothing(); } static constexpr std::string_view sql = "DELETE FROM nodejs_webstorage"; @@ -233,13 +238,15 @@ void Storage::Clear() { env(), sqlite3_prepare_v2(db_.get(), sql.data(), sql.size(), &s, 0), SQLITE_OK, - void()); + Nothing()); auto stmt = stmt_unique_ptr(s); - CHECK_ERROR_OR_THROW(env(), sqlite3_step(stmt.get()), SQLITE_DONE, void()); + CHECK_ERROR_OR_THROW( + env(), sqlite3_step(stmt.get()), SQLITE_DONE, Nothing()); + return JustVoid(); } -Local Storage::Enumerate() { - if (!Open()) { +MaybeLocal Storage::Enumerate() { + if (!Open().IsJust()) { return Local(); } @@ -256,7 +263,7 @@ Local Storage::Enumerate() { if (!String::NewFromTwoByte(env()->isolate(), reinterpret_cast( sqlite3_column_blob(stmt.get(), 0)), - v8::NewStringType::kNormal, + NewStringType::kNormal, size) .ToLocal(&value)) { return Local(); @@ -267,8 +274,8 @@ Local Storage::Enumerate() { return Array::New(env()->isolate(), values.data(), values.size()); } -Local Storage::Length() { - if (!Open()) { +MaybeLocal Storage::Length() { + if (!Open().IsJust()) { return {}; } @@ -285,14 +292,13 @@ Local Storage::Length() { return Integer::New(env()->isolate(), result); } -Local Storage::Load(Local key) { +MaybeLocal Storage::Load(Local key) { if (key->IsSymbol()) { auto symbol_map = symbols_.Get(env()->isolate()); - MaybeLocal result = symbol_map->Get(env()->context(), key); - return result.FromMaybe(Local()); + return symbol_map->Get(env()->context(), key); } - if (!Open()) { + if (!Open().IsJust()) { return {}; } @@ -306,28 +312,26 @@ Local Storage::Load(Local key) { auto key_size = utf16key.length() * sizeof(uint16_t); r = sqlite3_bind_blob(stmt.get(), 1, utf16key.out(), key_size, SQLITE_STATIC); CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Local()); - auto value = Local(); r = sqlite3_step(stmt.get()); if (r == SQLITE_ROW) { CHECK(sqlite3_column_type(stmt.get(), 0) == SQLITE_BLOB); auto size = sqlite3_column_bytes(stmt.get(), 0) / sizeof(uint16_t); - if (!String::NewFromTwoByte(env()->isolate(), - reinterpret_cast( - sqlite3_column_blob(stmt.get(), 0)), - v8::NewStringType::kNormal, - size) - .ToLocal(&value)) { - return {}; - } + return String::NewFromTwoByte(env()->isolate(), + reinterpret_cast( + sqlite3_column_blob(stmt.get(), 0)), + NewStringType::kNormal, + size) + .As(); } else if (r != SQLITE_DONE) { THROW_SQLITE_ERROR(env(), r); + return {}; + } else { + return Null(env()->isolate()); } - - return value; } -Local Storage::LoadKey(const int index) { - if (!Open()) { +MaybeLocal Storage::LoadKey(const int index) { + if (!Open().IsJust()) { return {}; } @@ -340,65 +344,64 @@ Local Storage::LoadKey(const int index) { r = sqlite3_bind_int(stmt.get(), 1, index); CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Local()); - auto value = Local(); r = sqlite3_step(stmt.get()); if (r == SQLITE_ROW) { CHECK(sqlite3_column_type(stmt.get(), 0) == SQLITE_BLOB); auto size = sqlite3_column_bytes(stmt.get(), 0) / sizeof(uint16_t); - if (!String::NewFromTwoByte(env()->isolate(), - reinterpret_cast( - sqlite3_column_blob(stmt.get(), 0)), - v8::NewStringType::kNormal, - size) - .ToLocal(&value)) { - return {}; - } + return String::NewFromTwoByte(env()->isolate(), + reinterpret_cast( + sqlite3_column_blob(stmt.get(), 0)), + NewStringType::kNormal, + size) + .As(); } else if (r != SQLITE_DONE) { THROW_SQLITE_ERROR(env(), r); + return {}; + } else { + return Null(env()->isolate()); } - - return value; } -bool Storage::Remove(Local key) { +Maybe Storage::Remove(Local key) { if (key->IsSymbol()) { auto symbol_map = symbols_.Get(env()->isolate()); Maybe result = symbol_map->Delete(env()->context(), key); - return !result.IsNothing(); + return result.IsNothing() ? Nothing() : JustVoid(); } - if (!Open()) { - return false; + if (!Open().IsJust()) { + return Nothing(); } static constexpr std::string_view sql = "DELETE FROM nodejs_webstorage WHERE key = ?"; sqlite3_stmt* s = nullptr; int r = sqlite3_prepare_v2(db_.get(), sql.data(), sql.size(), &s, 0); - CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, false); + CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Nothing()); auto stmt = stmt_unique_ptr(s); TwoByteValue utf16key(env()->isolate(), key); auto key_size = utf16key.length() * sizeof(uint16_t); r = sqlite3_bind_blob(stmt.get(), 1, utf16key.out(), key_size, SQLITE_STATIC); - CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, false); - CHECK_ERROR_OR_THROW(env(), sqlite3_step(stmt.get()), SQLITE_DONE, false); - return true; + CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Nothing()); + CHECK_ERROR_OR_THROW( + env(), sqlite3_step(stmt.get()), SQLITE_DONE, Nothing()); + return JustVoid(); } -bool Storage::Store(Local key, Local value) { +Maybe Storage::Store(Local key, Local value) { if (key->IsSymbol()) { auto symbol_map = symbols_.Get(env()->isolate()); MaybeLocal result = symbol_map->Set(env()->context(), key, value); - return !result.IsEmpty(); + return result.IsEmpty() ? Nothing() : JustVoid(); } Local val; if (!value->ToString(env()->context()).ToLocal(&val)) { - return false; + return Nothing(); } - if (!Open()) { - return false; + if (!Open().IsJust()) { + return Nothing(); } static constexpr std::string_view sql = @@ -409,23 +412,23 @@ bool Storage::Store(Local key, Local value) { TwoByteValue utf16key(env()->isolate(), key); TwoByteValue utf16val(env()->isolate(), val); int r = sqlite3_prepare_v2(db_.get(), sql.data(), sql.size(), &s, 0); - CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, false); + CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Nothing()); auto stmt = stmt_unique_ptr(s); auto key_size = utf16key.length() * sizeof(uint16_t); r = sqlite3_bind_blob(stmt.get(), 1, utf16key.out(), key_size, SQLITE_STATIC); - CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, false); + CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Nothing()); auto val_size = utf16val.length() * sizeof(uint16_t); r = sqlite3_bind_blob(stmt.get(), 2, utf16val.out(), val_size, SQLITE_STATIC); - CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, false); + CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Nothing()); r = sqlite3_step(stmt.get()); if (r == SQLITE_CONSTRAINT) { ThrowQuotaExceededException(env()->context()); - return false; + return Nothing(); } - CHECK_ERROR_OR_THROW(env(), r, SQLITE_DONE, false); - return true; + CHECK_ERROR_OR_THROW(env(), r, SQLITE_DONE, Nothing()); + return JustVoid(); } static MaybeLocal Uint32ToName(Local context, uint32_t index) { @@ -453,12 +456,11 @@ static void GetItem(const FunctionCallbackInfo& info) { return; } - Local result = storage->Load(prop); - if (result.IsEmpty()) { - info.GetReturnValue().SetNull(); - } else { - info.GetReturnValue().Set(result); + Local result; + if (!storage->Load(prop).ToLocal(&result)) { + return; } + info.GetReturnValue().Set(result); } static void Key(const FunctionCallbackInfo& info) { @@ -481,10 +483,8 @@ static void Key(const FunctionCallbackInfo& info) { return; } - Local result = storage->LoadKey(index); - if (result.IsEmpty()) { - info.GetReturnValue().SetNull(); - } else { + Local result; + if (storage->LoadKey(index).ToLocal(&result)) { info.GetReturnValue().Set(result); } } @@ -555,11 +555,9 @@ static Intercepted StorageGetter(Local property, Storage* storage; ASSIGN_OR_RETURN_UNWRAP(&storage, info.This(), Intercepted::kNo); - Local result = storage->Load(property); + Local result; - if (result.IsEmpty()) { - info.GetReturnValue().SetUndefined(); - } else { + if (storage->Load(property).ToLocal(&result) && !result->IsNull()) { info.GetReturnValue().Set(result); } @@ -572,7 +570,7 @@ static Intercepted StorageSetter(Local property, Storage* storage; ASSIGN_OR_RETURN_UNWRAP(&storage, info.This(), Intercepted::kNo); - if (storage->Store(property, value)) { + if (storage->Store(property, value).IsJust()) { info.GetReturnValue().Set(value); } @@ -587,8 +585,8 @@ static Intercepted StorageQuery(Local property, Storage* storage; ASSIGN_OR_RETURN_UNWRAP(&storage, info.This(), Intercepted::kNo); - Local result = storage->Load(property); - if (result.IsEmpty()) { + Local result; + if (!storage->Load(property).ToLocal(&result) || result->IsNull()) { return Intercepted::kNo; } @@ -601,9 +599,7 @@ static Intercepted StorageDeleter(Local property, Storage* storage; ASSIGN_OR_RETURN_UNWRAP(&storage, info.This(), Intercepted::kNo); - if (storage->Remove(property)) { - info.GetReturnValue().Set(true); - } + info.GetReturnValue().Set(storage->Remove(property).IsJust()); return Intercepted::kYes; } @@ -611,7 +607,11 @@ static Intercepted StorageDeleter(Local property, static void StorageEnumerator(const PropertyCallbackInfo& info) { Storage* storage; ASSIGN_OR_RETURN_UNWRAP(&storage, info.This()); - info.GetReturnValue().Set(storage->Enumerate()); + Local result; + if (!storage->Enumerate().ToLocal(&result)) { + return; + } + info.GetReturnValue().Set(result); } static Intercepted StorageDefiner(Local property, @@ -697,7 +697,11 @@ static Intercepted IndexedDefiner(uint32_t index, static void StorageLengthGetter(const FunctionCallbackInfo& info) { Storage* storage; ASSIGN_OR_RETURN_UNWRAP(&storage, info.This()); - info.GetReturnValue().Set(storage->Length()); + Local result; + if (!storage->Length().ToLocal(&result)) { + return; + } + info.GetReturnValue().Set(result); } static void Initialize(Local target, diff --git a/src/node_webstorage.h b/src/node_webstorage.h index 2c0c3ab688cae5..c2548d32e993fd 100644 --- a/src/node_webstorage.h +++ b/src/node_webstorage.h @@ -33,19 +33,19 @@ class Storage : public BaseObject { void MemoryInfo(MemoryTracker* tracker) const override; static void New(const v8::FunctionCallbackInfo& args); - void Clear(); - v8::Local Enumerate(); - v8::Local Length(); - v8::Local Load(v8::Local key); - v8::Local LoadKey(const int index); - bool Remove(v8::Local key); - bool Store(v8::Local key, v8::Local value); + v8::Maybe Clear(); + v8::MaybeLocal Enumerate(); + v8::MaybeLocal Length(); + v8::MaybeLocal Load(v8::Local key); + v8::MaybeLocal LoadKey(const int index); + v8::Maybe Remove(v8::Local key); + v8::Maybe Store(v8::Local key, v8::Local value); SET_MEMORY_INFO_NAME(Storage) SET_SELF_SIZE(Storage) private: - bool Open(); + v8::Maybe Open(); ~Storage() override; std::string location_;