Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 72c554a

Browse filesBrowse files
targosRafaelGSS
targos
authored and
RafaelGSS
committed
sqlite: return results with null prototype
These objects are dictionaries, and a query can return columns with special names like `__proto__` (which would be ignored without this change). Also construct the object by passing vectors of properties for better performance and improve error handling by using `MaybeLocal`. PR-URL: #54350 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 290f6ce commit 72c554a
Copy full SHA for 72c554a

File tree

Expand file treeCollapse file tree

8 files changed

+76
-61
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

8 files changed

+76
-61
lines changed
Open diff view settings
Collapse file

‎src/node_sqlite.cc‎

Copy file name to clipboardExpand all lines: src/node_sqlite.cc
+45-43Lines changed: 45 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ using v8::FunctionTemplate;
2525
using v8::Integer;
2626
using v8::Isolate;
2727
using v8::Local;
28+
using v8::LocalVector;
29+
using v8::MaybeLocal;
30+
using v8::Name;
31+
using v8::Null;
2832
using v8::Number;
2933
using v8::Object;
3034
using v8::String;
@@ -405,7 +409,7 @@ bool StatementSync::BindValue(const Local<Value>& value, const int index) {
405409
return true;
406410
}
407411

408-
Local<Value> StatementSync::ColumnToValue(const int column) {
412+
MaybeLocal<Value> StatementSync::ColumnToValue(const int column) {
409413
switch (sqlite3_column_type(statement_, column)) {
410414
case SQLITE_INTEGER: {
411415
sqlite3_int64 value = sqlite3_column_int64(statement_, column);
@@ -419,7 +423,7 @@ Local<Value> StatementSync::ColumnToValue(const int column) {
419423
"represented as a JavaScript number: %" PRId64,
420424
column,
421425
value);
422-
return Local<Value>();
426+
return MaybeLocal<Value>();
423427
}
424428
}
425429
case SQLITE_FLOAT:
@@ -428,14 +432,10 @@ Local<Value> StatementSync::ColumnToValue(const int column) {
428432
case SQLITE_TEXT: {
429433
const char* value = reinterpret_cast<const char*>(
430434
sqlite3_column_text(statement_, column));
431-
Local<Value> val;
432-
if (!String::NewFromUtf8(env()->isolate(), value).ToLocal(&val)) {
433-
return Local<Value>();
434-
}
435-
return val;
435+
return String::NewFromUtf8(env()->isolate(), value).As<Value>();
436436
}
437437
case SQLITE_NULL:
438-
return v8::Null(env()->isolate());
438+
return Null(env()->isolate());
439439
case SQLITE_BLOB: {
440440
size_t size =
441441
static_cast<size_t>(sqlite3_column_bytes(statement_, column));
@@ -451,19 +451,15 @@ Local<Value> StatementSync::ColumnToValue(const int column) {
451451
}
452452
}
453453

454-
Local<Value> StatementSync::ColumnNameToValue(const int column) {
454+
MaybeLocal<Name> StatementSync::ColumnNameToName(const int column) {
455455
const char* col_name = sqlite3_column_name(statement_, column);
456456
if (col_name == nullptr) {
457457
node::THROW_ERR_INVALID_STATE(
458458
env(), "Cannot get name of column %d", column);
459-
return Local<Value>();
459+
return MaybeLocal<Name>();
460460
}
461461

462-
Local<String> key;
463-
if (!String::NewFromUtf8(env()->isolate(), col_name).ToLocal(&key)) {
464-
return Local<Value>();
465-
}
466-
return key;
462+
return String::NewFromUtf8(env()->isolate(), col_name).As<Name>();
467463
}
468464

469465
void StatementSync::MemoryInfo(MemoryTracker* tracker) const {}
@@ -474,38 +470,40 @@ void StatementSync::All(const FunctionCallbackInfo<Value>& args) {
474470
Environment* env = Environment::GetCurrent(args);
475471
THROW_AND_RETURN_ON_BAD_STATE(
476472
env, stmt->IsFinalized(), "statement has been finalized");
473+
Isolate* isolate = env->isolate();
477474
int r = sqlite3_reset(stmt->statement_);
478-
CHECK_ERROR_OR_THROW(
479-
env->isolate(), stmt->db_->Connection(), r, SQLITE_OK, void());
475+
CHECK_ERROR_OR_THROW(isolate, stmt->db_->Connection(), r, SQLITE_OK, void());
480476

481477
if (!stmt->BindParams(args)) {
482478
return;
483479
}
484480

485481
auto reset = OnScopeLeave([&]() { sqlite3_reset(stmt->statement_); });
486482
int num_cols = sqlite3_column_count(stmt->statement_);
487-
std::vector<Local<Value>> rows;
483+
LocalVector<Value> rows(isolate);
488484
while ((r = sqlite3_step(stmt->statement_)) == SQLITE_ROW) {
489-
Local<Object> row = Object::New(env->isolate());
485+
LocalVector<Name> row_keys(isolate);
486+
row_keys.reserve(num_cols);
487+
LocalVector<Value> row_values(isolate);
488+
row_values.reserve(num_cols);
490489

491490
for (int i = 0; i < num_cols; ++i) {
492-
Local<Value> key = stmt->ColumnNameToValue(i);
493-
if (key.IsEmpty()) return;
494-
Local<Value> val = stmt->ColumnToValue(i);
495-
if (val.IsEmpty()) return;
496-
497-
if (row->Set(env->context(), key, val).IsNothing()) {
498-
return;
499-
}
491+
Local<Name> key;
492+
if (!stmt->ColumnNameToName(i).ToLocal(&key)) return;
493+
Local<Value> val;
494+
if (!stmt->ColumnToValue(i).ToLocal(&val)) return;
495+
row_keys.emplace_back(key);
496+
row_values.emplace_back(val);
500497
}
501498

499+
Local<Object> row = Object::New(
500+
isolate, Null(isolate), row_keys.data(), row_values.data(), num_cols);
502501
rows.emplace_back(row);
503502
}
504503

505504
CHECK_ERROR_OR_THROW(
506-
env->isolate(), stmt->db_->Connection(), r, SQLITE_DONE, void());
507-
args.GetReturnValue().Set(
508-
Array::New(env->isolate(), rows.data(), rows.size()));
505+
isolate, stmt->db_->Connection(), r, SQLITE_DONE, void());
506+
args.GetReturnValue().Set(Array::New(isolate, rows.data(), rows.size()));
509507
}
510508

511509
void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
@@ -514,9 +512,9 @@ void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
514512
Environment* env = Environment::GetCurrent(args);
515513
THROW_AND_RETURN_ON_BAD_STATE(
516514
env, stmt->IsFinalized(), "statement has been finalized");
515+
Isolate* isolate = env->isolate();
517516
int r = sqlite3_reset(stmt->statement_);
518-
CHECK_ERROR_OR_THROW(
519-
env->isolate(), stmt->db_->Connection(), r, SQLITE_OK, void());
517+
CHECK_ERROR_OR_THROW(isolate, stmt->db_->Connection(), r, SQLITE_OK, void());
520518

521519
if (!stmt->BindParams(args)) {
522520
return;
@@ -526,7 +524,7 @@ void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
526524
r = sqlite3_step(stmt->statement_);
527525
if (r == SQLITE_DONE) return;
528526
if (r != SQLITE_ROW) {
529-
THROW_ERR_SQLITE_ERROR(env->isolate(), stmt->db_->Connection());
527+
THROW_ERR_SQLITE_ERROR(isolate, stmt->db_->Connection());
530528
return;
531529
}
532530

@@ -535,19 +533,23 @@ void StatementSync::Get(const FunctionCallbackInfo<Value>& args) {
535533
return;
536534
}
537535

538-
Local<Object> result = Object::New(env->isolate());
536+
LocalVector<Name> keys(isolate);
537+
keys.reserve(num_cols);
538+
LocalVector<Value> values(isolate);
539+
values.reserve(num_cols);
539540

540541
for (int i = 0; i < num_cols; ++i) {
541-
Local<Value> key = stmt->ColumnNameToValue(i);
542-
if (key.IsEmpty()) return;
543-
Local<Value> val = stmt->ColumnToValue(i);
544-
if (val.IsEmpty()) return;
545-
546-
if (result->Set(env->context(), key, val).IsNothing()) {
547-
return;
548-
}
542+
Local<Name> key;
543+
if (!stmt->ColumnNameToName(i).ToLocal(&key)) return;
544+
Local<Value> val;
545+
if (!stmt->ColumnToValue(i).ToLocal(&val)) return;
546+
keys.emplace_back(key);
547+
values.emplace_back(val);
549548
}
550549

550+
Local<Object> result =
551+
Object::New(isolate, Null(isolate), keys.data(), values.data(), num_cols);
552+
551553
args.GetReturnValue().Set(result);
552554
}
553555

@@ -676,7 +678,7 @@ Local<FunctionTemplate> StatementSync::GetConstructorTemplate(
676678
if (tmpl.IsEmpty()) {
677679
Isolate* isolate = env->isolate();
678680
tmpl = NewFunctionTemplate(isolate, IllegalConstructor);
679-
tmpl->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "StatementSync"));
681+
tmpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "StatementSync"));
680682
tmpl->InstanceTemplate()->SetInternalFieldCount(
681683
StatementSync::kInternalFieldCount);
682684
SetProtoMethod(isolate, tmpl, "all", StatementSync::All);
Collapse file

‎src/node_sqlite.h‎

Copy file name to clipboardExpand all lines: src/node_sqlite.h
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ class StatementSync : public BaseObject {
8080
std::optional<std::map<std::string, std::string>> bare_named_params_;
8181
bool BindParams(const v8::FunctionCallbackInfo<v8::Value>& args);
8282
bool BindValue(const v8::Local<v8::Value>& value, const int index);
83-
v8::Local<v8::Value> ColumnToValue(const int column);
84-
v8::Local<v8::Value> ColumnNameToValue(const int column);
83+
v8::MaybeLocal<v8::Value> ColumnToValue(const int column);
84+
v8::MaybeLocal<v8::Name> ColumnNameToName(const int column);
8585
};
8686

8787
} // namespace sqlite
Collapse file

‎test/parallel/test-sqlite-data-types.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-sqlite-data-types.js
+5-1Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,27 +49,31 @@ suite('data binding and mapping', () => {
4949

5050
const query = db.prepare('SELECT * FROM types WHERE key = ?');
5151
t.assert.deepStrictEqual(query.get(1), {
52+
__proto__: null,
5253
key: 1,
5354
int: 42,
5455
double: 3.14159,
5556
text: 'foo',
5657
buf: u8a,
5758
});
5859
t.assert.deepStrictEqual(query.get(2), {
60+
__proto__: null,
5961
key: 2,
6062
int: null,
6163
double: null,
6264
text: null,
6365
buf: null,
6466
});
6567
t.assert.deepStrictEqual(query.get(3), {
68+
__proto__: null,
6669
key: 3,
6770
int: 8,
6871
double: 2.718,
6972
text: 'bar',
7073
buf: new TextEncoder().encode('x☃y☃'),
7174
});
7275
t.assert.deepStrictEqual(query.get(4), {
76+
__proto__: null,
7377
key: 4,
7478
int: 99,
7579
double: 0xf,
@@ -151,7 +155,7 @@ suite('data binding and mapping', () => {
151155
);
152156
t.assert.deepStrictEqual(
153157
db.prepare('SELECT * FROM data ORDER BY key').all(),
154-
[{ key: 1, val: 5 }, { key: 2, val: null }],
158+
[{ __proto__: null, key: 1, val: 5 }, { __proto__: null, key: 2, val: null }],
155159
);
156160
});
157161
});
Collapse file

‎test/parallel/test-sqlite-database-sync.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-sqlite-database-sync.js
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,8 +143,8 @@ suite('DatabaseSync.prototype.exec()', () => {
143143
t.assert.strictEqual(result, undefined);
144144
const stmt = db.prepare('SELECT * FROM data ORDER BY key');
145145
t.assert.deepStrictEqual(stmt.all(), [
146-
{ key: 1, val: 2 },
147-
{ key: 8, val: 9 },
146+
{ __proto__: null, key: 1, val: 2 },
147+
{ __proto__: null, key: 8, val: 9 },
148148
]);
149149
});
150150

Collapse file

‎test/parallel/test-sqlite-named-parameters.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-sqlite-named-parameters.js
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ suite('named parameters', () => {
4242
stmt.run({ k: 1, v: 9 });
4343
t.assert.deepStrictEqual(
4444
db.prepare('SELECT * FROM data').get(),
45-
{ key: 1, val: 9 },
45+
{ __proto__: null, key: 1, val: 9 },
4646
);
4747
});
4848

@@ -57,7 +57,7 @@ suite('named parameters', () => {
5757
stmt.run({ k: 1 });
5858
t.assert.deepStrictEqual(
5959
db.prepare('SELECT * FROM data').get(),
60-
{ key: 1, val: 1 },
60+
{ __proto__: null, key: 1, val: 1 },
6161
);
6262
});
6363

Collapse file

‎test/parallel/test-sqlite-statement-sync.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-sqlite-statement-sync.js
+15-6Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,15 @@ suite('StatementSync.prototype.get()', () => {
4343
t.assert.strictEqual(stmt.get('key1', 'val1'), undefined);
4444
t.assert.strictEqual(stmt.get('key2', 'val2'), undefined);
4545
stmt = db.prepare('SELECT * FROM storage ORDER BY key');
46-
t.assert.deepStrictEqual(stmt.get(), { key: 'key1', val: 'val1' });
46+
t.assert.deepStrictEqual(stmt.get(), { __proto__: null, key: 'key1', val: 'val1' });
47+
});
48+
49+
test('executes a query that returns special columns', (t) => {
50+
const db = new DatabaseSync(nextDb());
51+
t.after(() => { db.close(); });
52+
const stmt = db.prepare('SELECT 1 as __proto__, 2 as constructor, 3 as toString');
53+
// eslint-disable-next-line no-dupe-keys
54+
t.assert.deepStrictEqual(stmt.get(), { __proto__: null, ['__proto__']: 1, constructor: 2, toString: 3 });
4755
});
4856
});
4957

@@ -71,8 +79,8 @@ suite('StatementSync.prototype.all()', () => {
7179
);
7280
stmt = db.prepare('SELECT * FROM storage ORDER BY key');
7381
t.assert.deepStrictEqual(stmt.all(), [
74-
{ key: 'key1', val: 'val1' },
75-
{ key: 'key2', val: 'val2' },
82+
{ __proto__: null, key: 'key1', val: 'val1' },
83+
{ __proto__: null, key: 'key2', val: 'val2' },
7684
]);
7785
});
7886
});
@@ -171,11 +179,11 @@ suite('StatementSync.prototype.setReadBigInts()', () => {
171179
t.assert.strictEqual(setup, undefined);
172180

173181
const query = db.prepare('SELECT val FROM data');
174-
t.assert.deepStrictEqual(query.get(), { val: 42 });
182+
t.assert.deepStrictEqual(query.get(), { __proto__: null, val: 42 });
175183
t.assert.strictEqual(query.setReadBigInts(true), undefined);
176-
t.assert.deepStrictEqual(query.get(), { val: 42n });
184+
t.assert.deepStrictEqual(query.get(), { __proto__: null, val: 42n });
177185
t.assert.strictEqual(query.setReadBigInts(false), undefined);
178-
t.assert.deepStrictEqual(query.get(), { val: 42 });
186+
t.assert.deepStrictEqual(query.get(), { __proto__: null, val: 42 });
179187

180188
const insert = db.prepare('INSERT INTO data (key) VALUES (?)');
181189
t.assert.deepStrictEqual(
@@ -223,6 +231,7 @@ suite('StatementSync.prototype.setReadBigInts()', () => {
223231
const good = db.prepare(`SELECT ${Number.MAX_SAFE_INTEGER} + 1`);
224232
good.setReadBigInts(true);
225233
t.assert.deepStrictEqual(good.get(), {
234+
__proto__: null,
226235
[`${Number.MAX_SAFE_INTEGER} + 1`]: 2n ** 53n,
227236
});
228237
});
Collapse file

‎test/parallel/test-sqlite-transactions.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-sqlite-transactions.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ suite('manual transactions', () => {
3737
);
3838
t.assert.deepStrictEqual(
3939
db.prepare('SELECT * FROM data').all(),
40-
[{ key: 100 }],
40+
[{ __proto__: null, key: 100 }],
4141
);
4242
});
4343

Collapse file

‎test/parallel/test-sqlite.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-sqlite.js
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,11 +77,11 @@ test('in-memory databases are supported', (t) => {
7777
t.assert.strictEqual(setup2, undefined);
7878
t.assert.deepStrictEqual(
7979
db1.prepare('SELECT * FROM data').all(),
80-
[{ key: 1 }]
80+
[{ __proto__: null, key: 1 }]
8181
);
8282
t.assert.deepStrictEqual(
8383
db2.prepare('SELECT * FROM data').all(),
84-
[{ key: 1 }]
84+
[{ __proto__: null, key: 1 }]
8585
);
8686
});
8787

@@ -90,10 +90,10 @@ test('PRAGMAs are supported', (t) => {
9090
t.after(() => { db.close(); });
9191
t.assert.deepStrictEqual(
9292
db.prepare('PRAGMA journal_mode = WAL').get(),
93-
{ journal_mode: 'wal' },
93+
{ __proto__: null, journal_mode: 'wal' },
9494
);
9595
t.assert.deepStrictEqual(
9696
db.prepare('PRAGMA journal_mode').get(),
97-
{ journal_mode: 'wal' },
97+
{ __proto__: null, journal_mode: 'wal' },
9898
);
9999
});

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.