Skip to content

Commit 293e69d

Browse files
araujoguiaduh95
authored andcommitted
sqlite: throw on invalid URL path instead of abort
ValidateDatabasePath() treats any object exposing a string `href` as a URL, then asserts the parse result with CHECK(ada::can_parse(location)). Whether that string parses depends on user input rather than on an invariant the code guarantees, so a value such as { href: 'not a url' } aborted the process instead of throwing. Replace the CHECK with an ERR_INVALID_URL exception, matching how node_file.cc reports an unparsable URL. Both DatabaseSync() and backup() validate their path through this function, so both paths are covered. Signed-off-by: Guilherme Araújo <arauujogui@gmail.com> Assisted-by: Claude Code PR-URL: #66026 Reviewed-By: Xuguang Mei <meixuguang@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
1 parent b7eef15 commit 293e69d

3 files changed

Lines changed: 21 additions & 1 deletion

File tree

src/node_sqlite.cc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1296,7 +1296,11 @@ std::optional<std::string> ValidateDatabasePath(Environment* env,
12961296
Utf8Value location_value(env->isolate(), href.As<String>());
12971297
auto location = location_value.ToStringView();
12981298
if (!has_null_bytes(location)) {
1299-
CHECK(ada::can_parse(location));
1299+
if (!ada::can_parse(location)) {
1300+
THROW_ERR_INVALID_URL(env->isolate(), "Invalid URL");
1301+
return std::nullopt;
1302+
}
1303+
13001304
if (!location.starts_with("file:")) {
13011305
THROW_ERR_INVALID_URL_SCHEME(env->isolate());
13021306
return std::nullopt;

test/parallel/test-sqlite-backup.mjs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,16 @@ test('throws if URL is not file: scheme', (t) => {
269269
});
270270
});
271271

272+
test('throws if the URL-like path has an unparsable href', (t) => {
273+
const database = new DatabaseSync(':memory:');
274+
275+
t.after(() => { database.close(); });
276+
277+
t.assert.throws(() => {
278+
backup(database, { href: 'not a url' });
279+
}, { code: 'ERR_INVALID_URL' });
280+
});
281+
272282
test('database backup fails when dest file is not writable', { skip: isRoot }, async (t) => {
273283
const readonlyDestDb = nextDb();
274284
writeFileSync(readonlyDestDb, '', { mode: 0o444 });

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,12 @@ suite('DatabaseSync() constructor', () => {
5151
});
5252
});
5353

54+
test('throws if the URL-like path has an unparsable href', (t) => {
55+
t.assert.throws(() => {
56+
new DatabaseSync({ href: 'not a url' });
57+
}, { code: 'ERR_INVALID_URL' });
58+
});
59+
5460
test('throws if options is provided but is not an object', (t) => {
5561
t.assert.throws(() => {
5662
new DatabaseSync('foo', null);

0 commit comments

Comments
 (0)