Skip to content

Commit 57a9d3f

Browse files
tniessenaduh95
tniessen
authored and
aduh95
committed
sqlite: disable DQS misfeature by default
Double-quoted string (DQS) literals are not allowed by the SQL standard, which defines that text enclosed in double quotes is to be interpreted as an identifier only and never as a string literal. Nevertheless, for historical reasons, SQLite allows double-quoted string literals in some cases, which leads to inconsistent behavior and subtle bugs. This commit changes the behavior of the built-in Node.js API for SQLite such that the DQS misfeature is disabled by default. This is recommended by the developers of SQLite. Users can explicitly enable DQS for compatibility with legacy database schemas if necessary. PR-URL: #55297 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Jake Yuesong Li <[email protected]>
1 parent d51e5a8 commit 57a9d3f

File tree

4 files changed

+75
-4
lines changed

4 files changed

+75
-4
lines changed

‎doc/api/sqlite.md

+5
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,10 @@ added: v22.5.0
112112
legacy database schemas. The enforcement of foreign key constraints can be
113113
enabled and disabled after opening the database using
114114
[`PRAGMA foreign_keys`][]. **Default:** `true`.
115+
* `enableDoubleQuotedStringLiterals` {boolean} If `true`, SQLite will accept
116+
[double-quoted string literals][]. This is not recommended but can be
117+
enabled for compatibility with legacy database schemas.
118+
**Default:** `false`.
115119

116120
Constructs a new `DatabaseSync` instance.
117121

@@ -332,6 +336,7 @@ exception.
332336
[`sqlite3_sql()`]: https://www.sqlite.org/c3ref/expanded_sql.html
333337
[connection]: https://www.sqlite.org/c3ref/sqlite3.html
334338
[data types]: https://www.sqlite.org/datatype3.html
339+
[double-quoted string literals]: https://www.sqlite.org/quirks.html#dblquote
335340
[in memory]: https://www.sqlite.org/inmemorydb.html
336341
[parameters are bound]: https://www.sqlite.org/c3ref/bind_blob.html
337342
[prepared statement]: https://www.sqlite.org/c3ref/stmt.html

‎src/node_sqlite.cc

+39-3
Original file line numberDiff line numberDiff line change
@@ -96,13 +96,15 @@ DatabaseSync::DatabaseSync(Environment* env,
9696
Local<Object> object,
9797
Local<String> location,
9898
bool open,
99-
bool enable_foreign_keys_on_open)
99+
bool enable_foreign_keys_on_open,
100+
bool enable_dqs_on_open)
100101
: BaseObject(env, object) {
101102
MakeWeak();
102103
node::Utf8Value utf8_location(env->isolate(), location);
103104
location_ = utf8_location.ToString();
104105
connection_ = nullptr;
105106
enable_foreign_keys_on_open_ = enable_foreign_keys_on_open;
107+
enable_dqs_on_open_ = enable_dqs_on_open;
106108

107109
if (open) {
108110
Open();
@@ -132,6 +134,17 @@ bool DatabaseSync::Open() {
132134
int r = sqlite3_open_v2(location_.c_str(), &connection_, flags, nullptr);
133135
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
134136

137+
r = sqlite3_db_config(connection_,
138+
SQLITE_DBCONFIG_DQS_DML,
139+
static_cast<int>(enable_dqs_on_open_),
140+
nullptr);
141+
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
142+
r = sqlite3_db_config(connection_,
143+
SQLITE_DBCONFIG_DQS_DDL,
144+
static_cast<int>(enable_dqs_on_open_),
145+
nullptr);
146+
CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false);
147+
135148
int foreign_keys_enabled;
136149
r = sqlite3_db_config(connection_,
137150
SQLITE_DBCONFIG_ENABLE_FKEY,
@@ -182,6 +195,7 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
182195

183196
bool open = true;
184197
bool enable_foreign_keys = true;
198+
bool enable_dqs = false;
185199

186200
if (args.Length() > 1) {
187201
if (!args[1]->IsObject()) {
@@ -222,10 +236,32 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
222236
}
223237
enable_foreign_keys = enable_foreign_keys_v.As<Boolean>()->Value();
224238
}
239+
240+
Local<String> enable_dqs_string = FIXED_ONE_BYTE_STRING(
241+
env->isolate(), "enableDoubleQuotedStringLiterals");
242+
Local<Value> enable_dqs_v;
243+
if (!options->Get(env->context(), enable_dqs_string)
244+
.ToLocal(&enable_dqs_v)) {
245+
return;
246+
}
247+
if (!enable_dqs_v->IsUndefined()) {
248+
if (!enable_dqs_v->IsBoolean()) {
249+
node::THROW_ERR_INVALID_ARG_TYPE(
250+
env->isolate(),
251+
"The \"options.enableDoubleQuotedStringLiterals\" argument must be "
252+
"a boolean.");
253+
return;
254+
}
255+
enable_dqs = enable_dqs_v.As<Boolean>()->Value();
256+
}
225257
}
226258

227-
new DatabaseSync(
228-
env, args.This(), args[0].As<String>(), open, enable_foreign_keys);
259+
new DatabaseSync(env,
260+
args.This(),
261+
args[0].As<String>(),
262+
open,
263+
enable_foreign_keys,
264+
enable_dqs);
229265
}
230266

231267
void DatabaseSync::Open(const FunctionCallbackInfo<Value>& args) {

‎src/node_sqlite.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ class DatabaseSync : public BaseObject {
2222
v8::Local<v8::Object> object,
2323
v8::Local<v8::String> location,
2424
bool open,
25-
bool enable_foreign_keys_on_open);
25+
bool enable_foreign_keys_on_open,
26+
bool enable_dqs_on_open);
2627
void MemoryInfo(MemoryTracker* tracker) const override;
2728
static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
2829
static void Open(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -45,6 +46,7 @@ class DatabaseSync : public BaseObject {
4546
sqlite3* connection_;
4647
std::unordered_set<StatementSync*> statements_;
4748
bool enable_foreign_keys_on_open_;
49+
bool enable_dqs_on_open_;
4850
};
4951

5052
class StatementSync : public BaseObject {

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

+28
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,34 @@ suite('DatabaseSync() constructor', () => {
8686
t.after(() => { db.close(); });
8787
db.exec('INSERT INTO bar (foo_id) VALUES (1)');
8888
});
89+
90+
test('throws if options.enableDoubleQuotedStringLiterals is provided but is not a boolean', (t) => {
91+
t.assert.throws(() => {
92+
new DatabaseSync('foo', { enableDoubleQuotedStringLiterals: 5 });
93+
}, {
94+
code: 'ERR_INVALID_ARG_TYPE',
95+
message: /The "options\.enableDoubleQuotedStringLiterals" argument must be a boolean/,
96+
});
97+
});
98+
99+
test('disables double-quoted string literals by default', (t) => {
100+
const dbPath = nextDb();
101+
const db = new DatabaseSync(dbPath);
102+
t.after(() => { db.close(); });
103+
t.assert.throws(() => {
104+
db.exec('SELECT "foo";');
105+
}, {
106+
code: 'ERR_SQLITE_ERROR',
107+
message: /no such column: "foo"/,
108+
});
109+
});
110+
111+
test('allows enabling double-quoted string literals', (t) => {
112+
const dbPath = nextDb();
113+
const db = new DatabaseSync(dbPath, { enableDoubleQuotedStringLiterals: true });
114+
t.after(() => { db.close(); });
115+
db.exec('SELECT "foo";');
116+
});
89117
});
90118

91119
suite('DatabaseSync.prototype.open()', () => {

0 commit comments

Comments
 (0)