Skip to content

Commit

Permalink
src,test: disallow unsafe integer coercion in SQLite
Browse files Browse the repository at this point in the history
Currently, by default (i.e., when use_big_ints_ has not explicitly been
set to true), reading a SQLite integer value that is not a safe integer
in JavaScript is likely to yield an incorrect number.

Instead, err on the side of caution and throw if the stored integer is
not a safe integer in JavaScript and if use_big_ints_ has not been set
to true.

PR-URL: #53851
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
tniessen authored and targos committed Jul 28, 2024
1 parent 68b8412 commit 418d5b9
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 4 deletions.
17 changes: 13 additions & 4 deletions src/node_sqlite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -370,12 +370,21 @@ bool StatementSync::BindValue(const Local<Value>& value, const int index) {

Local<Value> StatementSync::ColumnToValue(const int column) {
switch (sqlite3_column_type(statement_, column)) {
case SQLITE_INTEGER:
case SQLITE_INTEGER: {
sqlite3_int64 value = sqlite3_column_int64(statement_, column);
if (use_big_ints_) {
return BigInt::New(env()->isolate(),
sqlite3_column_int64(statement_, column));
return BigInt::New(env()->isolate(), value);
} else if (std::abs(value) <= kMaxSafeJsInteger) {
return Number::New(env()->isolate(), value);
} else {
THROW_ERR_OUT_OF_RANGE(env()->isolate(),
"The value of column %d is too large to be "
"represented as a JavaScript number: %" PRId64,
column,
value);
return Local<Value>();
}
// Fall through.
}
case SQLITE_FLOAT:
return Number::New(env()->isolate(),
sqlite3_column_double(statement_, column));
Expand Down
16 changes: 16 additions & 0 deletions test/parallel/test-sqlite.js
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,22 @@ suite('StatementSync.prototype.setReadBigInts()', () => {
message: /The "readBigInts" argument must be a boolean/,
});
});

test('BigInt is required for reading large integers', (t) => {
const db = new DatabaseSync(nextDb());
const bad = db.prepare(`SELECT ${Number.MAX_SAFE_INTEGER} + 1`);
t.assert.throws(() => {
bad.get();
}, {
code: 'ERR_OUT_OF_RANGE',
message: /^The value of column 0 is too large.*: 9007199254740992$/,
});
const good = db.prepare(`SELECT ${Number.MAX_SAFE_INTEGER} + 1`);
good.setReadBigInts(true);
t.assert.deepStrictEqual(good.get(), {
[`${Number.MAX_SAFE_INTEGER} + 1`]: 2n ** 53n,
});
});
});

suite('StatementSync.prototype.setAllowBareNamedParameters()', () => {
Expand Down

0 comments on commit 418d5b9

Please sign in to comment.