Skip to content

Commit fc0ceb7

Browse files
jasnelltargos
jasnell
authored and
targos
committed
src: correct the error handling in StatementExecutionHelper
Error handling logic was flawed in StatementExecutionHelper methods. PR-URL: #60040 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
1 parent 50fa1f4 commit fc0ceb7

File tree

2 files changed

+100
-69
lines changed

2 files changed

+100
-69
lines changed

‎src/node_sqlite.cc‎

Lines changed: 86 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ using v8::ConstructorBehavior;
2626
using v8::Context;
2727
using v8::DictionaryTemplate;
2828
using v8::DontDelete;
29+
using v8::EscapableHandleScope;
2930
using v8::Exception;
3031
using v8::Function;
3132
using v8::FunctionCallback;
@@ -2209,12 +2210,13 @@ Maybe<void> ExtractRowValues(Environment* env,
22092210
return JustVoid();
22102211
}
22112212

2212-
Local<Value> StatementExecutionHelper::All(Environment* env,
2213-
DatabaseSync* db,
2214-
sqlite3_stmt* stmt,
2215-
bool return_arrays,
2216-
bool use_big_ints) {
2213+
MaybeLocal<Value> StatementExecutionHelper::All(Environment* env,
2214+
DatabaseSync* db,
2215+
sqlite3_stmt* stmt,
2216+
bool return_arrays,
2217+
bool use_big_ints) {
22172218
Isolate* isolate = env->isolate();
2219+
EscapableHandleScope scope(isolate);
22182220
int r;
22192221
int num_cols = sqlite3_column_count(stmt);
22202222
LocalVector<Value> rows(isolate);
@@ -2224,7 +2226,7 @@ Local<Value> StatementExecutionHelper::All(Environment* env,
22242226
while ((r = sqlite3_step(stmt)) == SQLITE_ROW) {
22252227
if (ExtractRowValues(env, stmt, num_cols, use_big_ints, &row_values)
22262228
.IsNothing()) {
2227-
return Undefined(isolate);
2229+
return MaybeLocal<Value>();
22282230
}
22292231

22302232
if (return_arrays) {
@@ -2236,8 +2238,9 @@ Local<Value> StatementExecutionHelper::All(Environment* env,
22362238
row_keys.reserve(num_cols);
22372239
for (int i = 0; i < num_cols; ++i) {
22382240
Local<Name> key;
2239-
if (!ColumnNameToName(env, stmt, i).ToLocal(&key))
2240-
return Undefined(isolate);
2241+
if (!ColumnNameToName(env, stmt, i).ToLocal(&key)) {
2242+
return MaybeLocal<Value>();
2243+
}
22412244
row_keys.emplace_back(key);
22422245
}
22432246
}
@@ -2248,18 +2251,19 @@ Local<Value> StatementExecutionHelper::All(Environment* env,
22482251
}
22492252
}
22502253

2251-
CHECK_ERROR_OR_THROW(isolate, db, r, SQLITE_DONE, Undefined(isolate));
2252-
return Array::New(isolate, rows.data(), rows.size());
2254+
CHECK_ERROR_OR_THROW(isolate, db, r, SQLITE_DONE, MaybeLocal<Value>());
2255+
return scope.Escape(Array::New(isolate, rows.data(), rows.size()));
22532256
}
22542257

2255-
Local<Object> StatementExecutionHelper::Run(Environment* env,
2256-
DatabaseSync* db,
2257-
sqlite3_stmt* stmt,
2258-
bool use_big_ints) {
2258+
MaybeLocal<Object> StatementExecutionHelper::Run(Environment* env,
2259+
DatabaseSync* db,
2260+
sqlite3_stmt* stmt,
2261+
bool use_big_ints) {
22592262
Isolate* isolate = env->isolate();
2263+
EscapableHandleScope scope(isolate);
22602264
sqlite3_step(stmt);
22612265
int r = sqlite3_reset(stmt);
2262-
CHECK_ERROR_OR_THROW(isolate, db, r, SQLITE_OK, Object::New(isolate));
2266+
CHECK_ERROR_OR_THROW(isolate, db, r, SQLITE_OK, MaybeLocal<Object>());
22632267
Local<Object> result = Object::New(isolate);
22642268
sqlite3_int64 last_insert_rowid = sqlite3_last_insert_rowid(db->Connection());
22652269
sqlite3_int64 changes = sqlite3_changes64(db->Connection());
@@ -2281,10 +2285,10 @@ Local<Object> StatementExecutionHelper::Run(Environment* env,
22812285
.IsNothing() ||
22822286
result->Set(env->context(), env->changes_string(), changes_val)
22832287
.IsNothing()) {
2284-
return Object::New(isolate);
2288+
return MaybeLocal<Object>();
22852289
}
22862290

2287-
return result;
2291+
return scope.Escape(result);
22882292
}
22892293

22902294
BaseObjectPtr<StatementSyncIterator> StatementExecutionHelper::Iterate(
@@ -2321,19 +2325,20 @@ BaseObjectPtr<StatementSyncIterator> StatementExecutionHelper::Iterate(
23212325
return iter;
23222326
}
23232327

2324-
Local<Value> StatementExecutionHelper::Get(Environment* env,
2325-
DatabaseSync* db,
2326-
sqlite3_stmt* stmt,
2327-
bool return_arrays,
2328-
bool use_big_ints) {
2328+
MaybeLocal<Value> StatementExecutionHelper::Get(Environment* env,
2329+
DatabaseSync* db,
2330+
sqlite3_stmt* stmt,
2331+
bool return_arrays,
2332+
bool use_big_ints) {
23292333
Isolate* isolate = env->isolate();
2334+
EscapableHandleScope scope(isolate);
23302335
auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt); });
23312336

23322337
int r = sqlite3_step(stmt);
2333-
if (r == SQLITE_DONE) return Undefined(isolate);
2338+
if (r == SQLITE_DONE) return scope.Escape(Undefined(isolate));
23342339
if (r != SQLITE_ROW) {
23352340
THROW_ERR_SQLITE_ERROR(isolate, db);
2336-
return Undefined(isolate);
2341+
return MaybeLocal<Value>();
23372342
}
23382343

23392344
int num_cols = sqlite3_column_count(stmt);
@@ -2344,25 +2349,26 @@ Local<Value> StatementExecutionHelper::Get(Environment* env,
23442349
LocalVector<Value> row_values(isolate);
23452350
if (ExtractRowValues(env, stmt, num_cols, use_big_ints, &row_values)
23462351
.IsNothing()) {
2347-
return Undefined(isolate);
2352+
return MaybeLocal<Value>();
23482353
}
23492354

23502355
if (return_arrays) {
2351-
return Array::New(isolate, row_values.data(), row_values.size());
2356+
return scope.Escape(
2357+
Array::New(isolate, row_values.data(), row_values.size()));
23522358
} else {
23532359
LocalVector<Name> keys(isolate);
23542360
keys.reserve(num_cols);
23552361
for (int i = 0; i < num_cols; ++i) {
23562362
Local<Name> key;
23572363
if (!ColumnNameToName(env, stmt, i).ToLocal(&key)) {
2358-
return Undefined(isolate);
2364+
return MaybeLocal<Value>();
23592365
}
23602366
keys.emplace_back(key);
23612367
}
23622368

23632369
DCHECK_EQ(keys.size(), row_values.size());
2364-
return Object::New(
2365-
isolate, Null(isolate), keys.data(), row_values.data(), num_cols);
2370+
return scope.Escape(Object::New(
2371+
isolate, Null(isolate), keys.data(), row_values.data(), num_cols));
23662372
}
23672373
}
23682374

@@ -2381,11 +2387,16 @@ void StatementSync::All(const FunctionCallbackInfo<Value>& args) {
23812387
}
23822388

23832389
auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); });
2384-
args.GetReturnValue().Set(StatementExecutionHelper::All(env,
2385-
stmt->db_.get(),
2386-
stmt->statement_,
2387-
stmt->return_arrays_,
2388-
stmt->use_big_ints_));
2390+
2391+
Local<Value> result;
2392+
if (StatementExecutionHelper::All(env,
2393+
stmt->db_.get(),
2394+
stmt->statement_,
2395+
stmt->return_arrays_,
2396+
stmt->use_big_ints_)
2397+
.ToLocal(&result)) {
2398+
args.GetReturnValue().Set(result);
2399+
}
23892400
}
23902401

23912402
void StatementSync::Iterate(const FunctionCallbackInfo<Value>& args) {
@@ -2424,11 +2435,15 @@ void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
24242435
return;
24252436
}
24262437

2427-
args.GetReturnValue().Set(StatementExecutionHelper::Get(env,
2428-
stmt->db_.get(),
2429-
stmt->statement_,
2430-
stmt->return_arrays_,
2431-
stmt->use_big_ints_));
2438+
Local<Value> result;
2439+
if (StatementExecutionHelper::Get(env,
2440+
stmt->db_.get(),
2441+
stmt->statement_,
2442+
stmt->return_arrays_,
2443+
stmt->use_big_ints_)
2444+
.ToLocal(&result)) {
2445+
args.GetReturnValue().Set(result);
2446+
}
24322447
}
24332448

24342449
void StatementSync::Run(const FunctionCallbackInfo<Value>& args) {
@@ -2444,8 +2459,12 @@ void StatementSync::Run(const FunctionCallbackInfo<Value>& args) {
24442459
return;
24452460
}
24462461

2447-
args.GetReturnValue().Set(StatementExecutionHelper::Run(
2448-
env, stmt->db_.get(), stmt->statement_, stmt->use_big_ints_));
2462+
Local<Object> result;
2463+
if (StatementExecutionHelper::Run(
2464+
env, stmt->db_.get(), stmt->statement_, stmt->use_big_ints_)
2465+
.ToLocal(&result)) {
2466+
args.GetReturnValue().Set(result);
2467+
}
24492468
}
24502469

24512470
void StatementSync::Columns(const FunctionCallbackInfo<Value>& args) {
@@ -2691,8 +2710,12 @@ void SQLTagStore::Run(const FunctionCallbackInfo<Value>& info) {
26912710
}
26922711
}
26932712

2694-
info.GetReturnValue().Set(StatementExecutionHelper::Run(
2695-
env, stmt->db_.get(), stmt->statement_, stmt->use_big_ints_));
2713+
Local<Object> result;
2714+
if (StatementExecutionHelper::Run(
2715+
env, stmt->db_.get(), stmt->statement_, stmt->use_big_ints_)
2716+
.ToLocal(&result)) {
2717+
info.GetReturnValue().Set(result);
2718+
}
26962719
}
26972720

26982721
void SQLTagStore::Iterate(const FunctionCallbackInfo<Value>& args) {
@@ -2758,11 +2781,15 @@ void SQLTagStore::Get(const FunctionCallbackInfo<Value>& args) {
27582781
}
27592782
}
27602783

2761-
args.GetReturnValue().Set(StatementExecutionHelper::Get(env,
2762-
stmt->db_.get(),
2763-
stmt->statement_,
2764-
stmt->return_arrays_,
2765-
stmt->use_big_ints_));
2784+
Local<Value> result;
2785+
if (StatementExecutionHelper::Get(env,
2786+
stmt->db_.get(),
2787+
stmt->statement_,
2788+
stmt->return_arrays_,
2789+
stmt->use_big_ints_)
2790+
.ToLocal(&result)) {
2791+
args.GetReturnValue().Set(result);
2792+
}
27662793
}
27672794

27682795
void SQLTagStore::All(const FunctionCallbackInfo<Value>& args) {
@@ -2794,11 +2821,15 @@ void SQLTagStore::All(const FunctionCallbackInfo<Value>& args) {
27942821
}
27952822

27962823
auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); });
2797-
args.GetReturnValue().Set(StatementExecutionHelper::All(env,
2798-
stmt->db_.get(),
2799-
stmt->statement_,
2800-
stmt->return_arrays_,
2801-
stmt->use_big_ints_));
2824+
Local<Value> result;
2825+
if (StatementExecutionHelper::All(env,
2826+
stmt->db_.get(),
2827+
stmt->statement_,
2828+
stmt->return_arrays_,
2829+
stmt->use_big_ints_)
2830+
.ToLocal(&result)) {
2831+
args.GetReturnValue().Set(result);
2832+
}
28022833
}
28032834

28042835
void SQLTagStore::Size(const FunctionCallbackInfo<Value>& info) {
@@ -2854,7 +2885,7 @@ BaseObjectPtr<StatementSync> SQLTagStore::PrepareStatement(
28542885
return BaseObjectPtr<StatementSync>();
28552886
}
28562887
Utf8Value part(isolate, str_val);
2857-
sql += *part;
2888+
sql += part.ToStringView();
28582889
if (i < n_params) {
28592890
sql += "?";
28602891
}
@@ -2872,7 +2903,7 @@ BaseObjectPtr<StatementSync> SQLTagStore::PrepareStatement(
28722903
if (stmt == nullptr) {
28732904
sqlite3_stmt* s = nullptr;
28742905
int r = sqlite3_prepare_v2(
2875-
session->database_->connection_, sql.c_str(), -1, &s, 0);
2906+
session->database_->connection_, sql.data(), sql.size(), &s, 0);
28762907

28772908
if (r != SQLITE_OK) {
28782909
THROW_ERR_SQLITE_ERROR(isolate, "Failed to prepare statement");

‎src/node_sqlite.h‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -84,15 +84,15 @@ class BackupJob;
8484

8585
class StatementExecutionHelper {
8686
public:
87-
static v8::Local<v8::Value> All(Environment* env,
88-
DatabaseSync* db,
89-
sqlite3_stmt* stmt,
90-
bool return_arrays,
91-
bool use_big_ints);
92-
static v8::Local<v8::Object> Run(Environment* env,
93-
DatabaseSync* db,
94-
sqlite3_stmt* stmt,
95-
bool use_big_ints);
87+
static v8::MaybeLocal<v8::Value> All(Environment* env,
88+
DatabaseSync* db,
89+
sqlite3_stmt* stmt,
90+
bool return_arrays,
91+
bool use_big_ints);
92+
static v8::MaybeLocal<v8::Object> Run(Environment* env,
93+
DatabaseSync* db,
94+
sqlite3_stmt* stmt,
95+
bool use_big_ints);
9696
static BaseObjectPtr<StatementSyncIterator> Iterate(
9797
Environment* env, BaseObjectPtr<StatementSync> stmt);
9898
static v8::MaybeLocal<v8::Value> ColumnToValue(Environment* env,
@@ -102,11 +102,11 @@ class StatementExecutionHelper {
102102
static v8::MaybeLocal<v8::Name> ColumnNameToName(Environment* env,
103103
sqlite3_stmt* stmt,
104104
const int column);
105-
static v8::Local<v8::Value> Get(Environment* env,
106-
DatabaseSync* db,
107-
sqlite3_stmt* stmt,
108-
bool return_arrays,
109-
bool use_big_ints);
105+
static v8::MaybeLocal<v8::Value> Get(Environment* env,
106+
DatabaseSync* db,
107+
sqlite3_stmt* stmt,
108+
bool return_arrays,
109+
bool use_big_ints);
110110
};
111111

112112
class DatabaseSync : public BaseObject {

0 commit comments

Comments
 (0)