Skip to content

Commit ca54aac

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

File tree

2 files changed

+101
-66
lines changed

2 files changed

+101
-66
lines changed

‎src/node_sqlite.cc‎

Lines changed: 87 additions & 52 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,11 +2096,11 @@ 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();
21042105
int r;
21052106
int num_cols = sqlite3_column_count(stmt);
@@ -2110,7 +2111,7 @@ Local<Value> StatementExecutionHelper::All(Environment* env,
21102111
while ((r = sqlite3_step(stmt)) == SQLITE_ROW) {
21112112
if (ExtractRowValues(env, stmt, num_cols, use_big_ints, &row_values)
21122113
.IsNothing()) {
2113-
return Undefined(isolate);
2114+
return MaybeLocal<Value>();
21142115
}
21152116

21162117
if (return_arrays) {
@@ -2122,8 +2123,9 @@ Local<Value> StatementExecutionHelper::All(Environment* env,
21222123
row_keys.reserve(num_cols);
21232124
for (int i = 0; i < num_cols; ++i) {
21242125
Local<Name> key;
2125-
if (!ColumnNameToName(env, stmt, i).ToLocal(&key))
2126-
return Undefined(isolate);
2126+
if (!ColumnNameToName(env, stmt, i).ToLocal(&key)) {
2127+
return MaybeLocal<Value>();
2128+
}
21272129
row_keys.emplace_back(key);
21282130
}
21292131
}
@@ -2134,18 +2136,19 @@ Local<Value> StatementExecutionHelper::All(Environment* env,
21342136
}
21352137
}
21362138

2137-
CHECK_ERROR_OR_THROW(isolate, db, r, SQLITE_DONE, Undefined(isolate));
2139+
CHECK_ERROR_OR_THROW(isolate, db, r, SQLITE_DONE, MaybeLocal<Value>());
21382140
return Array::New(isolate, rows.data(), rows.size());
21392141
}
21402142

2141-
Local<Object> StatementExecutionHelper::Run(Environment* env,
2142-
DatabaseSync* db,
2143-
sqlite3_stmt* stmt,
2144-
bool use_big_ints) {
2143+
MaybeLocal<Object> StatementExecutionHelper::Run(Environment* env,
2144+
DatabaseSync* db,
2145+
sqlite3_stmt* stmt,
2146+
bool use_big_ints) {
21452147
Isolate* isolate = env->isolate();
2148+
EscapableHandleScope scope(isolate);
21462149
sqlite3_step(stmt);
21472150
int r = sqlite3_reset(stmt);
2148-
CHECK_ERROR_OR_THROW(isolate, db, r, SQLITE_OK, Object::New(isolate));
2151+
CHECK_ERROR_OR_THROW(isolate, db, r, SQLITE_OK, MaybeLocal<Object>());
21492152
Local<Object> result = Object::New(isolate);
21502153
sqlite3_int64 last_insert_rowid = sqlite3_last_insert_rowid(db->Connection());
21512154
sqlite3_int64 changes = sqlite3_changes64(db->Connection());
@@ -2167,10 +2170,10 @@ Local<Object> StatementExecutionHelper::Run(Environment* env,
21672170
.IsNothing() ||
21682171
result->Set(env->context(), env->changes_string(), changes_val)
21692172
.IsNothing()) {
2170-
return Object::New(isolate);
2173+
return MaybeLocal<Object>();
21712174
}
21722175

2173-
return result;
2176+
return scope.Escape(result);
21742177
}
21752178

21762179
BaseObjectPtr<StatementSyncIterator> StatementExecutionHelper::Iterate(
@@ -2207,19 +2210,19 @@ BaseObjectPtr<StatementSyncIterator> StatementExecutionHelper::Iterate(
22072210
return iter;
22082211
}
22092212

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

22182221
int r = sqlite3_step(stmt);
22192222
if (r == SQLITE_DONE) return Undefined(isolate);
22202223
if (r != SQLITE_ROW) {
22212224
THROW_ERR_SQLITE_ERROR(isolate, db);
2222-
return Undefined(isolate);
2225+
return MaybeLocal<Value>();
22232226
}
22242227

22252228
int num_cols = sqlite3_column_count(stmt);
@@ -2230,7 +2233,7 @@ Local<Value> StatementExecutionHelper::Get(Environment* env,
22302233
LocalVector<Value> row_values(isolate);
22312234
if (ExtractRowValues(env, stmt, num_cols, use_big_ints, &row_values)
22322235
.IsNothing()) {
2233-
return Undefined(isolate);
2236+
return MaybeLocal<Value>();
22342237
}
22352238

22362239
if (return_arrays) {
@@ -2239,9 +2242,11 @@ Local<Value> StatementExecutionHelper::Get(Environment* env,
22392242
LocalVector<Name> keys(isolate);
22402243
keys.reserve(num_cols);
22412244
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());
2245+
Local<Name> key;
2246+
if (!ColumnNameToName(env, stmt, i).ToLocal(&key)) {
2247+
return MaybeLocal<Value>();
2248+
}
2249+
keys.emplace_back(key);
22452250
}
22462251

22472252
DCHECK_EQ(keys.size(), row_values.size());
@@ -2265,11 +2270,16 @@ void StatementSync::All(const FunctionCallbackInfo<Value>& args) {
22652270
}
22662271

22672272
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_));
2273+
2274+
Local<Value> result;
2275+
if (StatementExecutionHelper::All(env,
2276+
stmt->db_.get(),
2277+
stmt->statement_,
2278+
stmt->return_arrays_,
2279+
stmt->use_big_ints_)
2280+
.ToLocal(&result)) {
2281+
args.GetReturnValue().Set(result);
2282+
}
22732283
}
22742284

22752285
void StatementSync::Iterate(const FunctionCallbackInfo<Value>& args) {
@@ -2308,11 +2318,15 @@ void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
23082318
return;
23092319
}
23102320

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

23182332
void StatementSync::Run(const FunctionCallbackInfo<Value>& args) {
@@ -2328,8 +2342,12 @@ void StatementSync::Run(const FunctionCallbackInfo<Value>& args) {
23282342
return;
23292343
}
23302344

2331-
args.GetReturnValue().Set(StatementExecutionHelper::Run(
2332-
env, stmt->db_.get(), stmt->statement_, stmt->use_big_ints_));
2345+
Local<Object> result;
2346+
if (StatementExecutionHelper::Run(
2347+
env, stmt->db_.get(), stmt->statement_, stmt->use_big_ints_)
2348+
.ToLocal(&result)) {
2349+
args.GetReturnValue().Set(result);
2350+
}
23332351
}
23342352

23352353
void StatementSync::Columns(const FunctionCallbackInfo<Value>& args) {
@@ -2575,8 +2593,12 @@ void SQLTagStore::Run(const FunctionCallbackInfo<Value>& info) {
25752593
}
25762594
}
25772595

2578-
info.GetReturnValue().Set(StatementExecutionHelper::Run(
2579-
env, stmt->db_.get(), stmt->statement_, stmt->use_big_ints_));
2596+
Local<Object> result;
2597+
if (StatementExecutionHelper::Run(
2598+
env, stmt->db_.get(), stmt->statement_, stmt->use_big_ints_)
2599+
.ToLocal(&result)) {
2600+
info.GetReturnValue().Set(result);
2601+
}
25802602
}
25812603

25822604
void SQLTagStore::Iterate(const FunctionCallbackInfo<Value>& args) {
@@ -2642,11 +2664,15 @@ void SQLTagStore::Get(const FunctionCallbackInfo<Value>& args) {
26422664
}
26432665
}
26442666

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

26522678
void SQLTagStore::All(const FunctionCallbackInfo<Value>& args) {
@@ -2678,11 +2704,15 @@ void SQLTagStore::All(const FunctionCallbackInfo<Value>& args) {
26782704
}
26792705

26802706
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_));
2707+
Local<Value> result;
2708+
if (StatementExecutionHelper::All(env,
2709+
stmt->db_.get(),
2710+
stmt->statement_,
2711+
stmt->return_arrays_,
2712+
stmt->use_big_ints_)
2713+
.ToLocal(&result)) {
2714+
args.GetReturnValue().Set(result);
2715+
}
26862716
}
26872717

26882718
void SQLTagStore::Size(const FunctionCallbackInfo<Value>& info) {
@@ -2755,8 +2785,13 @@ BaseObjectPtr<StatementSync> SQLTagStore::PrepareStatement(
27552785

27562786
if (stmt == nullptr) {
27572787
sqlite3_stmt* s = nullptr;
2758-
Local<String> sql_str =
2759-
String::NewFromUtf8(isolate, sql.c_str()).ToLocalChecked();
2788+
// TODO(@jasnell): It's not entirely clear why we're doing this conversion
2789+
// just to convert it back again with the Utf8Value. It may be possible to
2790+
// optimize this.
2791+
Local<String> sql_str;
2792+
if (!ToV8Value(env->context(), sql).ToLocal(&sql_str)) {
2793+
return BaseObjectPtr<StatementSync>();
2794+
}
27602795
Utf8Value sql_utf8(isolate, sql_str);
27612796

27622797
int r = sqlite3_prepare_v2(

‎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)