Skip to content

Commit 7b69c4f

Browse files
jasnelljasnell
committed
src: correct the error handling in StatementExecutionHelper
Error handling logic was flawed in StatementExecutionHelper methods.
1 parent cff138c commit 7b69c4f

File tree

2 files changed

+104
-74
lines changed

2 files changed

+104
-74
lines changed

‎src/node_sqlite.cc‎

Lines changed: 90 additions & 60 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;
@@ -2095,12 +2096,13 @@ Maybe<void> ExtractRowValues(Environment* env,
20952096
return JustVoid();
20962097
}
20972098

2098-
Local<Value> StatementExecutionHelper::All(Environment* env,
2099-
DatabaseSync* db,
2100-
sqlite3_stmt* stmt,
2101-
bool return_arrays,
2102-
bool use_big_ints) {
2099+
MaybeLocal<Value> StatementExecutionHelper::All(Environment* env,
2100+
DatabaseSync* db,
2101+
sqlite3_stmt* stmt,
2102+
bool return_arrays,
2103+
bool use_big_ints) {
21032104
Isolate* isolate = env->isolate();
2105+
EscapableHandleScope scope(isolate);
21042106
int r;
21052107
int num_cols = sqlite3_column_count(stmt);
21062108
LocalVector<Value> rows(isolate);
@@ -2110,7 +2112,7 @@ Local<Value> StatementExecutionHelper::All(Environment* env,
21102112
while ((r = sqlite3_step(stmt)) == SQLITE_ROW) {
21112113
if (ExtractRowValues(env, stmt, num_cols, use_big_ints, &row_values)
21122114
.IsNothing()) {
2113-
return Undefined(isolate);
2115+
return MaybeLocal<Value>();
21142116
}
21152117

21162118
if (return_arrays) {
@@ -2122,8 +2124,9 @@ Local<Value> StatementExecutionHelper::All(Environment* env,
21222124
row_keys.reserve(num_cols);
21232125
for (int i = 0; i < num_cols; ++i) {
21242126
Local<Name> key;
2125-
if (!ColumnNameToName(env, stmt, i).ToLocal(&key))
2126-
return Undefined(isolate);
2127+
if (!ColumnNameToName(env, stmt, i).ToLocal(&key)) {
2128+
return MaybeLocal<Value>();
2129+
}
21272130
row_keys.emplace_back(key);
21282131
}
21292132
}
@@ -2134,18 +2137,19 @@ Local<Value> StatementExecutionHelper::All(Environment* env,
21342137
}
21352138
}
21362139

2137-
CHECK_ERROR_OR_THROW(isolate, db, r, SQLITE_DONE, Undefined(isolate));
2138-
return Array::New(isolate, rows.data(), rows.size());
2140+
CHECK_ERROR_OR_THROW(isolate, db, r, SQLITE_DONE, MaybeLocal<Value>());
2141+
return scope.Escape(Array::New(isolate, rows.data(), rows.size()));
21392142
}
21402143

2141-
Local<Object> StatementExecutionHelper::Run(Environment* env,
2142-
DatabaseSync* db,
2143-
sqlite3_stmt* stmt,
2144-
bool use_big_ints) {
2144+
MaybeLocal<Object> StatementExecutionHelper::Run(Environment* env,
2145+
DatabaseSync* db,
2146+
sqlite3_stmt* stmt,
2147+
bool use_big_ints) {
21452148
Isolate* isolate = env->isolate();
2149+
EscapableHandleScope scope(isolate);
21462150
sqlite3_step(stmt);
21472151
int r = sqlite3_reset(stmt);
2148-
CHECK_ERROR_OR_THROW(isolate, db, r, SQLITE_OK, Object::New(isolate));
2152+
CHECK_ERROR_OR_THROW(isolate, db, r, SQLITE_OK, MaybeLocal<Object>());
21492153
Local<Object> result = Object::New(isolate);
21502154
sqlite3_int64 last_insert_rowid = sqlite3_last_insert_rowid(db->Connection());
21512155
sqlite3_int64 changes = sqlite3_changes64(db->Connection());
@@ -2167,10 +2171,10 @@ Local<Object> StatementExecutionHelper::Run(Environment* env,
21672171
.IsNothing() ||
21682172
result->Set(env->context(), env->changes_string(), changes_val)
21692173
.IsNothing()) {
2170-
return Object::New(isolate);
2174+
return MaybeLocal<Object>();
21712175
}
21722176

2173-
return result;
2177+
return scope.Escape(result);
21742178
}
21752179

21762180
BaseObjectPtr<StatementSyncIterator> StatementExecutionHelper::Iterate(
@@ -2207,19 +2211,20 @@ BaseObjectPtr<StatementSyncIterator> StatementExecutionHelper::Iterate(
22072211
return iter;
22082212
}
22092213

2210-
Local<Value> StatementExecutionHelper::Get(Environment* env,
2211-
DatabaseSync* db,
2212-
sqlite3_stmt* stmt,
2213-
bool return_arrays,
2214-
bool use_big_ints) {
2214+
MaybeLocal<Value> StatementExecutionHelper::Get(Environment* env,
2215+
DatabaseSync* db,
2216+
sqlite3_stmt* stmt,
2217+
bool return_arrays,
2218+
bool use_big_ints) {
22152219
Isolate* isolate = env->isolate();
2220+
EscapableHandleScope scope(isolate);
22162221
auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt); });
22172222

22182223
int r = sqlite3_step(stmt);
2219-
if (r == SQLITE_DONE) return Undefined(isolate);
2224+
if (r == SQLITE_DONE) return scope.Escape(Undefined(isolate));
22202225
if (r != SQLITE_ROW) {
22212226
THROW_ERR_SQLITE_ERROR(isolate, db);
2222-
return Undefined(isolate);
2227+
return MaybeLocal<Value>();
22232228
}
22242229

22252230
int num_cols = sqlite3_column_count(stmt);
@@ -2230,23 +2235,26 @@ Local<Value> StatementExecutionHelper::Get(Environment* env,
22302235
LocalVector<Value> row_values(isolate);
22312236
if (ExtractRowValues(env, stmt, num_cols, use_big_ints, &row_values)
22322237
.IsNothing()) {
2233-
return Undefined(isolate);
2238+
return MaybeLocal<Value>();
22342239
}
22352240

22362241
if (return_arrays) {
2237-
return Array::New(isolate, row_values.data(), row_values.size());
2242+
return scope.Escape(
2243+
Array::New(isolate, row_values.data(), row_values.size()));
22382244
} else {
22392245
LocalVector<Name> keys(isolate);
22402246
keys.reserve(num_cols);
22412247
for (int i = 0; i < num_cols; ++i) {
2242-
MaybeLocal<Name> key = ColumnNameToName(env, stmt, i);
2243-
if (key.IsEmpty()) return Undefined(isolate);
2244-
keys.emplace_back(key.ToLocalChecked());
2248+
Local<Name> key;
2249+
if (!ColumnNameToName(env, stmt, i).ToLocal(&key)) {
2250+
return MaybeLocal<Value>();
2251+
}
2252+
keys.emplace_back(key);
22452253
}
22462254

22472255
DCHECK_EQ(keys.size(), row_values.size());
2248-
return Object::New(
2249-
isolate, Null(isolate), keys.data(), row_values.data(), num_cols);
2256+
return scope.Escape(Object::New(
2257+
isolate, Null(isolate), keys.data(), row_values.data(), num_cols));
22502258
}
22512259
}
22522260

@@ -2265,11 +2273,16 @@ void StatementSync::All(const FunctionCallbackInfo<Value>& args) {
22652273
}
22662274

22672275
auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); });
2268-
args.GetReturnValue().Set(StatementExecutionHelper::All(env,
2269-
stmt->db_.get(),
2270-
stmt->statement_,
2271-
stmt->return_arrays_,
2272-
stmt->use_big_ints_));
2276+
2277+
Local<Value> result;
2278+
if (StatementExecutionHelper::All(env,
2279+
stmt->db_.get(),
2280+
stmt->statement_,
2281+
stmt->return_arrays_,
2282+
stmt->use_big_ints_)
2283+
.ToLocal(&result)) {
2284+
args.GetReturnValue().Set(result);
2285+
}
22732286
}
22742287

22752288
void StatementSync::Iterate(const FunctionCallbackInfo<Value>& args) {
@@ -2308,11 +2321,15 @@ void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
23082321
return;
23092322
}
23102323

2311-
args.GetReturnValue().Set(StatementExecutionHelper::Get(env,
2312-
stmt->db_.get(),
2313-
stmt->statement_,
2314-
stmt->return_arrays_,
2315-
stmt->use_big_ints_));
2324+
Local<Value> result;
2325+
if (StatementExecutionHelper::Get(env,
2326+
stmt->db_.get(),
2327+
stmt->statement_,
2328+
stmt->return_arrays_,
2329+
stmt->use_big_ints_)
2330+
.ToLocal(&result)) {
2331+
args.GetReturnValue().Set(result);
2332+
}
23162333
}
23172334

23182335
void StatementSync::Run(const FunctionCallbackInfo<Value>& args) {
@@ -2328,8 +2345,12 @@ void StatementSync::Run(const FunctionCallbackInfo<Value>& args) {
23282345
return;
23292346
}
23302347

2331-
args.GetReturnValue().Set(StatementExecutionHelper::Run(
2332-
env, stmt->db_.get(), stmt->statement_, stmt->use_big_ints_));
2348+
Local<Object> result;
2349+
if (StatementExecutionHelper::Run(
2350+
env, stmt->db_.get(), stmt->statement_, stmt->use_big_ints_)
2351+
.ToLocal(&result)) {
2352+
args.GetReturnValue().Set(result);
2353+
}
23332354
}
23342355

23352356
void StatementSync::Columns(const FunctionCallbackInfo<Value>& args) {
@@ -2575,8 +2596,12 @@ void SQLTagStore::Run(const FunctionCallbackInfo<Value>& info) {
25752596
}
25762597
}
25772598

2578-
info.GetReturnValue().Set(StatementExecutionHelper::Run(
2579-
env, stmt->db_.get(), stmt->statement_, stmt->use_big_ints_));
2599+
Local<Object> result;
2600+
if (StatementExecutionHelper::Run(
2601+
env, stmt->db_.get(), stmt->statement_, stmt->use_big_ints_)
2602+
.ToLocal(&result)) {
2603+
info.GetReturnValue().Set(result);
2604+
}
25802605
}
25812606

25822607
void SQLTagStore::Iterate(const FunctionCallbackInfo<Value>& args) {
@@ -2642,11 +2667,15 @@ void SQLTagStore::Get(const FunctionCallbackInfo<Value>& args) {
26422667
}
26432668
}
26442669

2645-
args.GetReturnValue().Set(StatementExecutionHelper::Get(env,
2646-
stmt->db_.get(),
2647-
stmt->statement_,
2648-
stmt->return_arrays_,
2649-
stmt->use_big_ints_));
2670+
Local<Value> result;
2671+
if (StatementExecutionHelper::Get(env,
2672+
stmt->db_.get(),
2673+
stmt->statement_,
2674+
stmt->return_arrays_,
2675+
stmt->use_big_ints_)
2676+
.ToLocal(&result)) {
2677+
args.GetReturnValue().Set(result);
2678+
}
26502679
}
26512680

26522681
void SQLTagStore::All(const FunctionCallbackInfo<Value>& args) {
@@ -2678,11 +2707,15 @@ void SQLTagStore::All(const FunctionCallbackInfo<Value>& args) {
26782707
}
26792708

26802709
auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); });
2681-
args.GetReturnValue().Set(StatementExecutionHelper::All(env,
2682-
stmt->db_.get(),
2683-
stmt->statement_,
2684-
stmt->return_arrays_,
2685-
stmt->use_big_ints_));
2710+
Local<Value> result;
2711+
if (StatementExecutionHelper::All(env,
2712+
stmt->db_.get(),
2713+
stmt->statement_,
2714+
stmt->return_arrays_,
2715+
stmt->use_big_ints_)
2716+
.ToLocal(&result)) {
2717+
args.GetReturnValue().Set(result);
2718+
}
26862719
}
26872720

26882721
void SQLTagStore::Size(const FunctionCallbackInfo<Value>& info) {
@@ -2738,7 +2771,7 @@ BaseObjectPtr<StatementSync> SQLTagStore::PrepareStatement(
27382771
return BaseObjectPtr<StatementSync>();
27392772
}
27402773
Utf8Value part(isolate, str_val);
2741-
sql += *part;
2774+
sql += part.ToStringView();
27422775
if (i < n_params) {
27432776
sql += "?";
27442777
}
@@ -2755,12 +2788,9 @@ BaseObjectPtr<StatementSync> SQLTagStore::PrepareStatement(
27552788

27562789
if (stmt == nullptr) {
27572790
sqlite3_stmt* s = nullptr;
2758-
Local<String> sql_str =
2759-
String::NewFromUtf8(isolate, sql.c_str()).ToLocalChecked();
2760-
Utf8Value sql_utf8(isolate, sql_str);
27612791

27622792
int r = sqlite3_prepare_v2(
2763-
session->database_->connection_, *sql_utf8, -1, &s, 0);
2793+
session->database_->connection_, sql.data(), sql.size(), &s, 0);
27642794

27652795
if (r != SQLITE_OK) {
27662796
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)