Skip to content

Commit 0999949

Browse files
committedOct 6, 2024··
src: handle errors correctly in webstorage
PR-URL: #54544 Backport-PR-URL: #54683 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
1 parent 195df65 commit 0999949

File tree

2 files changed

+102
-95
lines changed

2 files changed

+102
-95
lines changed
 

‎src/node_webstorage.cc

+94-87
Original file line numberDiff line numberDiff line change
@@ -24,19 +24,24 @@ using v8::IndexedPropertyHandlerConfiguration;
2424
using v8::Integer;
2525
using v8::Intercepted;
2626
using v8::Isolate;
27+
using v8::JustVoid;
2728
using v8::Local;
2829
using v8::Map;
2930
using v8::Maybe;
3031
using v8::MaybeLocal;
3132
using v8::Name;
3233
using v8::NamedPropertyHandlerConfiguration;
34+
using v8::NewStringType;
35+
using v8::Nothing;
36+
using v8::Null;
3337
using v8::Object;
3438
using v8::PropertyAttribute;
3539
using v8::PropertyCallbackInfo;
3640
using v8::PropertyDescriptor;
3741
using v8::PropertyHandlerFlags;
3842
using v8::String;
3943
using v8::Uint32;
44+
using v8::Undefined;
4045
using v8::Value;
4146

4247
#define THROW_SQLITE_ERROR(env, r) \
@@ -96,7 +101,7 @@ void Storage::MemoryInfo(MemoryTracker* tracker) const {
96101
tracker->TrackField("location", location_);
97102
}
98103

99-
bool Storage::Open() {
104+
Maybe<void> Storage::Open() {
100105
static const int kCurrentSchemaVersion = 1;
101106
static constexpr std::string_view get_schema_version_sql =
102107
"SELECT schema_version FROM nodejs_webstorage_state";
@@ -161,30 +166,31 @@ bool Storage::Open() {
161166

162167
sqlite3* db = db_.get();
163168
if (db != nullptr) {
164-
return true;
169+
return JustVoid();
165170
}
166171

167172
int r = sqlite3_open(location_.c_str(), &db);
168-
CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, false);
173+
CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Nothing<void>());
169174
r = sqlite3_exec(db, init_sql_v0.data(), 0, 0, nullptr);
170-
CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, false);
175+
CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Nothing<void>());
171176

172177
// Get the current schema version, used to determine schema migrations.
173178
sqlite3_stmt* s = nullptr;
174179
r = sqlite3_prepare_v2(
175180
db, get_schema_version_sql.data(), get_schema_version_sql.size(), &s, 0);
176181
r = sqlite3_exec(db, init_sql_v0.data(), 0, 0, nullptr);
177-
CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, false);
182+
CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Nothing<void>());
178183
auto stmt = stmt_unique_ptr(s);
179-
CHECK_ERROR_OR_THROW(env(), sqlite3_step(stmt.get()), SQLITE_ROW, false);
184+
CHECK_ERROR_OR_THROW(
185+
env(), sqlite3_step(stmt.get()), SQLITE_ROW, Nothing<void>());
180186
CHECK(sqlite3_column_type(stmt.get(), 0) == SQLITE_INTEGER);
181187
int schema_version = sqlite3_column_int(stmt.get(), 0);
182188
stmt = nullptr; // Force finalization.
183189

184190
if (schema_version > kCurrentSchemaVersion) {
185191
THROW_ERR_INVALID_STATE(
186192
env(), "localStorage was created with a newer version of Node.js");
187-
return false;
193+
return Nothing<void>();
188194
}
189195

190196
if (schema_version < kCurrentSchemaVersion) {
@@ -193,11 +199,11 @@ bool Storage::Open() {
193199
"UPDATE nodejs_webstorage_state SET schema_version = " +
194200
std::to_string(kCurrentSchemaVersion) + ";";
195201
r = sqlite3_exec(db, set_user_version_sql.c_str(), 0, 0, nullptr);
196-
CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, false);
202+
CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Nothing<void>());
197203
}
198204

199205
db_ = conn_unique_ptr(db);
200-
return true;
206+
return JustVoid();
201207
}
202208

203209
void Storage::New(const FunctionCallbackInfo<Value>& args) {
@@ -222,9 +228,9 @@ void Storage::New(const FunctionCallbackInfo<Value>& args) {
222228
new Storage(env, args.This(), location.ToStringView());
223229
}
224230

225-
void Storage::Clear() {
226-
if (!Open()) {
227-
return;
231+
Maybe<void> Storage::Clear() {
232+
if (!Open().IsJust()) {
233+
return Nothing<void>();
228234
}
229235

230236
static constexpr std::string_view sql = "DELETE FROM nodejs_webstorage";
@@ -233,13 +239,15 @@ void Storage::Clear() {
233239
env(),
234240
sqlite3_prepare_v2(db_.get(), sql.data(), sql.size(), &s, 0),
235241
SQLITE_OK,
236-
void());
242+
Nothing<void>());
237243
auto stmt = stmt_unique_ptr(s);
238-
CHECK_ERROR_OR_THROW(env(), sqlite3_step(stmt.get()), SQLITE_DONE, void());
244+
CHECK_ERROR_OR_THROW(
245+
env(), sqlite3_step(stmt.get()), SQLITE_DONE, Nothing<void>());
246+
return JustVoid();
239247
}
240248

241-
Local<Array> Storage::Enumerate() {
242-
if (!Open()) {
249+
MaybeLocal<Array> Storage::Enumerate() {
250+
if (!Open().IsJust()) {
243251
return Local<Array>();
244252
}
245253

@@ -256,7 +264,7 @@ Local<Array> Storage::Enumerate() {
256264
if (!String::NewFromTwoByte(env()->isolate(),
257265
reinterpret_cast<const uint16_t*>(
258266
sqlite3_column_blob(stmt.get(), 0)),
259-
v8::NewStringType::kNormal,
267+
NewStringType::kNormal,
260268
size)
261269
.ToLocal(&value)) {
262270
return Local<Array>();
@@ -267,8 +275,8 @@ Local<Array> Storage::Enumerate() {
267275
return Array::New(env()->isolate(), values.data(), values.size());
268276
}
269277

270-
Local<Value> Storage::Length() {
271-
if (!Open()) {
278+
MaybeLocal<Value> Storage::Length() {
279+
if (!Open().IsJust()) {
272280
return {};
273281
}
274282

@@ -285,14 +293,13 @@ Local<Value> Storage::Length() {
285293
return Integer::New(env()->isolate(), result);
286294
}
287295

288-
Local<Value> Storage::Load(Local<Name> key) {
296+
MaybeLocal<Value> Storage::Load(Local<Name> key) {
289297
if (key->IsSymbol()) {
290298
auto symbol_map = symbols_.Get(env()->isolate());
291-
MaybeLocal<Value> result = symbol_map->Get(env()->context(), key);
292-
return result.FromMaybe(Local<Value>());
299+
return symbol_map->Get(env()->context(), key);
293300
}
294301

295-
if (!Open()) {
302+
if (!Open().IsJust()) {
296303
return {};
297304
}
298305

@@ -306,28 +313,26 @@ Local<Value> Storage::Load(Local<Name> key) {
306313
auto key_size = utf16key.length() * sizeof(uint16_t);
307314
r = sqlite3_bind_blob(stmt.get(), 1, utf16key.out(), key_size, SQLITE_STATIC);
308315
CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Local<Value>());
309-
auto value = Local<Value>();
310316
r = sqlite3_step(stmt.get());
311317
if (r == SQLITE_ROW) {
312318
CHECK(sqlite3_column_type(stmt.get(), 0) == SQLITE_BLOB);
313319
auto size = sqlite3_column_bytes(stmt.get(), 0) / sizeof(uint16_t);
314-
if (!String::NewFromTwoByte(env()->isolate(),
315-
reinterpret_cast<const uint16_t*>(
316-
sqlite3_column_blob(stmt.get(), 0)),
317-
v8::NewStringType::kNormal,
318-
size)
319-
.ToLocal(&value)) {
320-
return {};
321-
}
320+
return String::NewFromTwoByte(env()->isolate(),
321+
reinterpret_cast<const uint16_t*>(
322+
sqlite3_column_blob(stmt.get(), 0)),
323+
NewStringType::kNormal,
324+
size)
325+
.As<Value>();
322326
} else if (r != SQLITE_DONE) {
323327
THROW_SQLITE_ERROR(env(), r);
328+
return {};
329+
} else {
330+
return Null(env()->isolate());
324331
}
325-
326-
return value;
327332
}
328333

329-
Local<Value> Storage::LoadKey(const int index) {
330-
if (!Open()) {
334+
MaybeLocal<Value> Storage::LoadKey(const int index) {
335+
if (!Open().IsJust()) {
331336
return {};
332337
}
333338

@@ -340,65 +345,64 @@ Local<Value> Storage::LoadKey(const int index) {
340345
r = sqlite3_bind_int(stmt.get(), 1, index);
341346
CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Local<Value>());
342347

343-
auto value = Local<Value>();
344348
r = sqlite3_step(stmt.get());
345349
if (r == SQLITE_ROW) {
346350
CHECK(sqlite3_column_type(stmt.get(), 0) == SQLITE_BLOB);
347351
auto size = sqlite3_column_bytes(stmt.get(), 0) / sizeof(uint16_t);
348-
if (!String::NewFromTwoByte(env()->isolate(),
349-
reinterpret_cast<const uint16_t*>(
350-
sqlite3_column_blob(stmt.get(), 0)),
351-
v8::NewStringType::kNormal,
352-
size)
353-
.ToLocal(&value)) {
354-
return {};
355-
}
352+
return String::NewFromTwoByte(env()->isolate(),
353+
reinterpret_cast<const uint16_t*>(
354+
sqlite3_column_blob(stmt.get(), 0)),
355+
NewStringType::kNormal,
356+
size)
357+
.As<Value>();
356358
} else if (r != SQLITE_DONE) {
357359
THROW_SQLITE_ERROR(env(), r);
360+
return {};
361+
} else {
362+
return Null(env()->isolate());
358363
}
359-
360-
return value;
361364
}
362365

363-
bool Storage::Remove(Local<Name> key) {
366+
Maybe<void> Storage::Remove(Local<Name> key) {
364367
if (key->IsSymbol()) {
365368
auto symbol_map = symbols_.Get(env()->isolate());
366369
Maybe<bool> result = symbol_map->Delete(env()->context(), key);
367-
return !result.IsNothing();
370+
return result.IsNothing() ? Nothing<void>() : JustVoid();
368371
}
369372

370-
if (!Open()) {
371-
return false;
373+
if (!Open().IsJust()) {
374+
return Nothing<void>();
372375
}
373376

374377
static constexpr std::string_view sql =
375378
"DELETE FROM nodejs_webstorage WHERE key = ?";
376379
sqlite3_stmt* s = nullptr;
377380
int r = sqlite3_prepare_v2(db_.get(), sql.data(), sql.size(), &s, 0);
378-
CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, false);
381+
CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Nothing<void>());
379382
auto stmt = stmt_unique_ptr(s);
380383
TwoByteValue utf16key(env()->isolate(), key);
381384
auto key_size = utf16key.length() * sizeof(uint16_t);
382385
r = sqlite3_bind_blob(stmt.get(), 1, utf16key.out(), key_size, SQLITE_STATIC);
383-
CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, false);
384-
CHECK_ERROR_OR_THROW(env(), sqlite3_step(stmt.get()), SQLITE_DONE, false);
385-
return true;
386+
CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Nothing<void>());
387+
CHECK_ERROR_OR_THROW(
388+
env(), sqlite3_step(stmt.get()), SQLITE_DONE, Nothing<void>());
389+
return JustVoid();
386390
}
387391

388-
bool Storage::Store(Local<Name> key, Local<Value> value) {
392+
Maybe<void> Storage::Store(Local<Name> key, Local<Value> value) {
389393
if (key->IsSymbol()) {
390394
auto symbol_map = symbols_.Get(env()->isolate());
391395
MaybeLocal<Map> result = symbol_map->Set(env()->context(), key, value);
392-
return !result.IsEmpty();
396+
return result.IsEmpty() ? Nothing<void>() : JustVoid();
393397
}
394398

395399
Local<String> val;
396400
if (!value->ToString(env()->context()).ToLocal(&val)) {
397-
return false;
401+
return Nothing<void>();
398402
}
399403

400-
if (!Open()) {
401-
return false;
404+
if (!Open().IsJust()) {
405+
return Nothing<void>();
402406
}
403407

404408
static constexpr std::string_view sql =
@@ -409,23 +413,23 @@ bool Storage::Store(Local<Name> key, Local<Value> value) {
409413
TwoByteValue utf16key(env()->isolate(), key);
410414
TwoByteValue utf16val(env()->isolate(), val);
411415
int r = sqlite3_prepare_v2(db_.get(), sql.data(), sql.size(), &s, 0);
412-
CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, false);
416+
CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Nothing<void>());
413417
auto stmt = stmt_unique_ptr(s);
414418
auto key_size = utf16key.length() * sizeof(uint16_t);
415419
r = sqlite3_bind_blob(stmt.get(), 1, utf16key.out(), key_size, SQLITE_STATIC);
416-
CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, false);
420+
CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Nothing<void>());
417421
auto val_size = utf16val.length() * sizeof(uint16_t);
418422
r = sqlite3_bind_blob(stmt.get(), 2, utf16val.out(), val_size, SQLITE_STATIC);
419-
CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, false);
423+
CHECK_ERROR_OR_THROW(env(), r, SQLITE_OK, Nothing<void>());
420424

421425
r = sqlite3_step(stmt.get());
422426
if (r == SQLITE_CONSTRAINT) {
423427
ThrowQuotaExceededException(env()->context());
424-
return false;
428+
return Nothing<void>();
425429
}
426430

427-
CHECK_ERROR_OR_THROW(env(), r, SQLITE_DONE, false);
428-
return true;
431+
CHECK_ERROR_OR_THROW(env(), r, SQLITE_DONE, Nothing<void>());
432+
return JustVoid();
429433
}
430434

431435
static MaybeLocal<String> Uint32ToName(Local<Context> context, uint32_t index) {
@@ -453,12 +457,11 @@ static void GetItem(const FunctionCallbackInfo<Value>& info) {
453457
return;
454458
}
455459

456-
Local<Value> result = storage->Load(prop);
457-
if (result.IsEmpty()) {
458-
info.GetReturnValue().SetNull();
459-
} else {
460-
info.GetReturnValue().Set(result);
460+
Local<Value> result;
461+
if (!storage->Load(prop).ToLocal(&result)) {
462+
return;
461463
}
464+
info.GetReturnValue().Set(result);
462465
}
463466

464467
static void Key(const FunctionCallbackInfo<Value>& info) {
@@ -481,10 +484,8 @@ static void Key(const FunctionCallbackInfo<Value>& info) {
481484
return;
482485
}
483486

484-
Local<Value> result = storage->LoadKey(index);
485-
if (result.IsEmpty()) {
486-
info.GetReturnValue().SetNull();
487-
} else {
487+
Local<Value> result;
488+
if (storage->LoadKey(index).ToLocal(&result)) {
488489
info.GetReturnValue().Set(result);
489490
}
490491
}
@@ -555,12 +556,12 @@ static Intercepted StorageGetter(Local<Name> property,
555556

556557
Storage* storage;
557558
ASSIGN_OR_RETURN_UNWRAP(&storage, info.This(), Intercepted::kNo);
558-
Local<Value> result = storage->Load(property);
559+
Local<Value> result;
559560

560-
if (result.IsEmpty()) {
561-
info.GetReturnValue().SetUndefined();
562-
} else {
561+
if (storage->Load(property).ToLocal(&result) && !result->IsNull()) {
563562
info.GetReturnValue().Set(result);
563+
} else {
564+
info.GetReturnValue().Set(Undefined(info.GetIsolate()));
564565
}
565566

566567
return Intercepted::kYes;
@@ -572,7 +573,7 @@ static Intercepted StorageSetter(Local<Name> property,
572573
Storage* storage;
573574
ASSIGN_OR_RETURN_UNWRAP(&storage, info.This(), Intercepted::kNo);
574575

575-
if (storage->Store(property, value)) {
576+
if (storage->Store(property, value).IsJust()) {
576577
info.GetReturnValue().Set(value);
577578
}
578579

@@ -587,8 +588,8 @@ static Intercepted StorageQuery(Local<Name> property,
587588

588589
Storage* storage;
589590
ASSIGN_OR_RETURN_UNWRAP(&storage, info.This(), Intercepted::kNo);
590-
Local<Value> result = storage->Load(property);
591-
if (result.IsEmpty()) {
591+
Local<Value> result;
592+
if (!storage->Load(property).ToLocal(&result) || result->IsNull()) {
592593
return Intercepted::kNo;
593594
}
594595

@@ -601,17 +602,19 @@ static Intercepted StorageDeleter(Local<Name> property,
601602
Storage* storage;
602603
ASSIGN_OR_RETURN_UNWRAP(&storage, info.This(), Intercepted::kNo);
603604

604-
if (storage->Remove(property)) {
605-
info.GetReturnValue().Set(true);
606-
}
605+
info.GetReturnValue().Set(storage->Remove(property).IsJust());
607606

608607
return Intercepted::kYes;
609608
}
610609

611610
static void StorageEnumerator(const PropertyCallbackInfo<Array>& info) {
612611
Storage* storage;
613612
ASSIGN_OR_RETURN_UNWRAP(&storage, info.This());
614-
info.GetReturnValue().Set(storage->Enumerate());
613+
Local<Array> result;
614+
if (!storage->Enumerate().ToLocal(&result)) {
615+
return;
616+
}
617+
info.GetReturnValue().Set(result);
615618
}
616619

617620
static Intercepted StorageDefiner(Local<Name> property,
@@ -697,7 +700,11 @@ static Intercepted IndexedDefiner(uint32_t index,
697700
static void StorageLengthGetter(const FunctionCallbackInfo<Value>& info) {
698701
Storage* storage;
699702
ASSIGN_OR_RETURN_UNWRAP(&storage, info.This());
700-
info.GetReturnValue().Set(storage->Length());
703+
Local<Value> result;
704+
if (!storage->Length().ToLocal(&result)) {
705+
return;
706+
}
707+
info.GetReturnValue().Set(result);
701708
}
702709

703710
static void Initialize(Local<Object> target,

‎src/node_webstorage.h

+8-8
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,19 @@ class Storage : public BaseObject {
3333
void MemoryInfo(MemoryTracker* tracker) const override;
3434
static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
3535

36-
void Clear();
37-
v8::Local<v8::Array> Enumerate();
38-
v8::Local<v8::Value> Length();
39-
v8::Local<v8::Value> Load(v8::Local<v8::Name> key);
40-
v8::Local<v8::Value> LoadKey(const int index);
41-
bool Remove(v8::Local<v8::Name> key);
42-
bool Store(v8::Local<v8::Name> key, v8::Local<v8::Value> value);
36+
v8::Maybe<void> Clear();
37+
v8::MaybeLocal<v8::Array> Enumerate();
38+
v8::MaybeLocal<v8::Value> Length();
39+
v8::MaybeLocal<v8::Value> Load(v8::Local<v8::Name> key);
40+
v8::MaybeLocal<v8::Value> LoadKey(const int index);
41+
v8::Maybe<void> Remove(v8::Local<v8::Name> key);
42+
v8::Maybe<void> Store(v8::Local<v8::Name> key, v8::Local<v8::Value> value);
4343

4444
SET_MEMORY_INFO_NAME(Storage)
4545
SET_SELF_SIZE(Storage)
4646

4747
private:
48-
bool Open();
48+
v8::Maybe<void> Open();
4949

5050
~Storage() override;
5151
std::string location_;

0 commit comments

Comments
 (0)
Please sign in to comment.