From 6bb9155eab972878dbbfcf5546630e8e6f0b2c6b Mon Sep 17 00:00:00 2001 From: Edy Silva Date: Tue, 18 Mar 2025 11:59:52 -0300 Subject: [PATCH 1/2] sqlite,test,doc: allow Buffer and URL as database location PR-URL: https://github.com/nodejs/node/pull/56991 Reviewed-By: Colin Ihrig --- doc/api/sqlite.md | 12 ++- src/env_properties.h | 3 + src/node_sqlite.cc | 84 +++++++++++++++--- test/parallel/test-sqlite-database-sync.js | 31 ++++++- test/parallel/test-sqlite.js | 99 ++++++++++++++++++++++ 5 files changed, 212 insertions(+), 17 deletions(-) diff --git a/doc/api/sqlite.md b/doc/api/sqlite.md index 11bc81e0e428f7..4e58f1da087d8b 100644 --- a/doc/api/sqlite.md +++ b/doc/api/sqlite.md @@ -77,20 +77,24 @@ console.log(query.all()); This class represents a single [connection][] to a SQLite database. All APIs exposed by this class execute synchronously. -### `new DatabaseSync(location[, options])` +### `new DatabaseSync(path[, options])` -* `location` {string} The location of the database. A SQLite database can be +* `path` {string | Buffer | URL} The path of the database. A SQLite database can be stored in a file or completely [in memory][]. To use a file-backed database, - the location should be a file path. To use an in-memory database, the location + the path should be a file path. To use an in-memory database, the path should be the special name `':memory:'`. * `options` {Object} Configuration options for the database connection. The following options are supported: @@ -191,7 +195,7 @@ wrapper around [`sqlite3_create_function_v2()`][]. added: v22.5.0 --> -Opens the database specified in the `location` argument of the `DatabaseSync` +Opens the database specified in the `path` argument of the `DatabaseSync` constructor. This method should only be used when the database is not opened via the constructor. An exception is thrown if the database is already open. diff --git a/src/env_properties.h b/src/env_properties.h index 9f898231707822..419acb911dab46 100644 --- a/src/env_properties.h +++ b/src/env_properties.h @@ -186,6 +186,8 @@ V(homedir_string, "homedir") \ V(host_string, "host") \ V(hostmaster_string, "hostmaster") \ + V(hostname_string, "hostname") \ + V(href_string, "href") \ V(http_1_1_string, "http/1.1") \ V(id_string, "id") \ V(identity_string, "identity") \ @@ -295,6 +297,7 @@ V(priority_string, "priority") \ V(process_string, "process") \ V(promise_string, "promise") \ + V(protocol_string, "protocol") \ V(prototype_string, "prototype") \ V(psk_string, "psk") \ V(pubkey_string, "pubkey") \ diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 0b8f7ef2a21763..c72817eb24672e 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -7,6 +7,7 @@ #include "node.h" #include "node_errors.h" #include "node_mem-inl.h" +#include "node_url.h" #include "sqlite3.h" #include "util-inl.h" @@ -292,11 +293,14 @@ bool DatabaseSync::Open() { } // TODO(cjihrig): Support additional flags. + int default_flags = SQLITE_OPEN_URI; int flags = open_config_.get_read_only() ? SQLITE_OPEN_READONLY : SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE; - int r = sqlite3_open_v2( - open_config_.location().c_str(), &connection_, flags, nullptr); + int r = sqlite3_open_v2(open_config_.location().c_str(), + &connection_, + flags | default_flags, + nullptr); CHECK_ERROR_OR_THROW(env()->isolate(), connection_, r, SQLITE_OK, false); r = sqlite3_db_config(connection_, @@ -358,27 +362,85 @@ inline sqlite3* DatabaseSync::Connection() { return connection_; } +std::optional ValidateDatabasePath(Environment* env, + Local path, + const std::string& field_name) { + auto has_null_bytes = [](const std::string& str) { + return str.find('\0') != std::string::npos; + }; + std::string location; + if (path->IsString()) { + location = Utf8Value(env->isolate(), path.As()).ToString(); + if (!has_null_bytes(location)) { + return location; + } + } + + if (path->IsUint8Array()) { + Local buffer = path.As(); + size_t byteOffset = buffer->ByteOffset(); + size_t byteLength = buffer->ByteLength(); + auto data = + static_cast(buffer->Buffer()->Data()) + byteOffset; + if (!(std::find(data, data + byteLength, 0) != data + byteLength)) { + Local out; + if (String::NewFromUtf8(env->isolate(), + reinterpret_cast(data), + NewStringType::kNormal, + static_cast(byteLength)) + .ToLocal(&out)) { + return Utf8Value(env->isolate(), out.As()).ToString(); + } + } + } + + // When is URL + if (path->IsObject()) { + Local url = path.As(); + Local href; + Local protocol; + if (url->Get(env->context(), env->href_string()).ToLocal(&href) && + href->IsString() && + url->Get(env->context(), env->protocol_string()).ToLocal(&protocol) && + protocol->IsString()) { + location = Utf8Value(env->isolate(), href.As()).ToString(); + if (!has_null_bytes(location)) { + auto file_url = ada::parse(location); + CHECK(file_url); + if (file_url->type != ada::scheme::FILE) { + THROW_ERR_INVALID_URL_SCHEME(env->isolate()); + return std::nullopt; + } + + return location; + } + } + } + + THROW_ERR_INVALID_ARG_TYPE(env->isolate(), + "The \"%s\" argument must be a string, " + "Uint8Array, or URL without null bytes.", + field_name.c_str()); + + return std::nullopt; +} + void DatabaseSync::New(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - if (!args.IsConstructCall()) { THROW_ERR_CONSTRUCT_CALL_REQUIRED(env); return; } - if (!args[0]->IsString()) { - THROW_ERR_INVALID_ARG_TYPE(env->isolate(), - "The \"path\" argument must be a string."); + std::optional location = + ValidateDatabasePath(env, args[0], "path"); + if (!location.has_value()) { return; } - std::string location = - Utf8Value(env->isolate(), args[0].As()).ToString(); - DatabaseOpenConfiguration open_config(std::move(location)); - + DatabaseOpenConfiguration open_config(std::move(location.value())); bool open = true; bool allow_load_extension = false; - if (args.Length() > 1) { if (!args[1]->IsObject()) { THROW_ERR_INVALID_ARG_TYPE(env->isolate(), diff --git a/test/parallel/test-sqlite-database-sync.js b/test/parallel/test-sqlite-database-sync.js index b9ce0b8ad2aefc..7734ead74a1412 100644 --- a/test/parallel/test-sqlite-database-sync.js +++ b/test/parallel/test-sqlite-database-sync.js @@ -23,12 +23,30 @@ suite('DatabaseSync() constructor', () => { }); }); - test('throws if database path is not a string', (t) => { + test('throws if database path is not a string, Uint8Array, or URL', (t) => { t.assert.throws(() => { new DatabaseSync(); }, { code: 'ERR_INVALID_ARG_TYPE', - message: /The "path" argument must be a string/, + message: /The "path" argument must be a string, Uint8Array, or URL without null bytes/, + }); + }); + + test('throws if the database location as Buffer contains null bytes', (t) => { + t.assert.throws(() => { + new DatabaseSync(Buffer.from('l\0cation')); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: 'The "path" argument must be a string, Uint8Array, or URL without null bytes.', + }); + }); + + test('throws if the database location as string contains null bytes', (t) => { + t.assert.throws(() => { + new DatabaseSync('l\0cation'); + }, { + code: 'ERR_INVALID_ARG_TYPE', + message: 'The "path" argument must be a string, Uint8Array, or URL without null bytes.', }); }); @@ -256,6 +274,15 @@ suite('DatabaseSync.prototype.exec()', () => { }); }); + test('throws if the URL does not have the file: scheme', (t) => { + t.assert.throws(() => { + new DatabaseSync(new URL('http://example.com')); + }, { + code: 'ERR_INVALID_URL_SCHEME', + message: 'The URL must be of scheme file:', + }); + }); + test('throws if database is not open', (t) => { const db = new DatabaseSync(nextDb(), { open: false }); diff --git a/test/parallel/test-sqlite.js b/test/parallel/test-sqlite.js index c9a45fa22aecc2..ec04e425e30aaa 100644 --- a/test/parallel/test-sqlite.js +++ b/test/parallel/test-sqlite.js @@ -4,6 +4,7 @@ const tmpdir = require('../common/tmpdir'); const { join } = require('node:path'); const { DatabaseSync, constants } = require('node:sqlite'); const { suite, test } = require('node:test'); +const { pathToFileURL } = require('node:url'); let cnt = 0; tmpdir.refresh(); @@ -111,3 +112,101 @@ test('math functions are enabled', (t) => { { __proto__: null, pi: 3.141592653589793 }, ); }); + +test('Buffer is supported as the database path', (t) => { + const db = new DatabaseSync(Buffer.from(nextDb())); + t.after(() => { db.close(); }); + db.exec(` + CREATE TABLE data(key INTEGER PRIMARY KEY); + INSERT INTO data (key) VALUES (1); + `); + + t.assert.deepStrictEqual( + db.prepare('SELECT * FROM data').all(), + [{ __proto__: null, key: 1 }] + ); +}); + +test('URL is supported as the database path', (t) => { + const url = pathToFileURL(nextDb()); + const db = new DatabaseSync(url); + t.after(() => { db.close(); }); + db.exec(` + CREATE TABLE data(key INTEGER PRIMARY KEY); + INSERT INTO data (key) VALUES (1); + `); + + t.assert.deepStrictEqual( + db.prepare('SELECT * FROM data').all(), + [{ __proto__: null, key: 1 }] + ); +}); + + +suite('URI query params', () => { + const baseDbPath = nextDb(); + const baseDb = new DatabaseSync(baseDbPath); + baseDb.exec(` + CREATE TABLE data(key INTEGER PRIMARY KEY); + INSERT INTO data (key) VALUES (1); + `); + baseDb.close(); + + test('query params are supported with URL objects', (t) => { + const url = pathToFileURL(baseDbPath); + url.searchParams.set('mode', 'ro'); + const readOnlyDB = new DatabaseSync(url); + t.after(() => { readOnlyDB.close(); }); + + t.assert.deepStrictEqual( + readOnlyDB.prepare('SELECT * FROM data').all(), + [{ __proto__: null, key: 1 }] + ); + t.assert.throws(() => { + readOnlyDB.exec('INSERT INTO data (key) VALUES (1);'); + }, { + code: 'ERR_SQLITE_ERROR', + message: 'attempt to write a readonly database', + }); + }); + + test('query params are supported with string', (t) => { + const url = pathToFileURL(baseDbPath); + url.searchParams.set('mode', 'ro'); + + // Ensures a valid URI passed as a string is supported + const readOnlyDB = new DatabaseSync(url.toString()); + t.after(() => { readOnlyDB.close(); }); + + t.assert.deepStrictEqual( + readOnlyDB.prepare('SELECT * FROM data').all(), + [{ __proto__: null, key: 1 }] + ); + t.assert.throws(() => { + readOnlyDB.exec('INSERT INTO data (key) VALUES (1);'); + }, { + code: 'ERR_SQLITE_ERROR', + message: 'attempt to write a readonly database', + }); + }); + + test('query params are supported with Buffer', (t) => { + const url = pathToFileURL(baseDbPath); + url.searchParams.set('mode', 'ro'); + + // Ensures a valid URI passed as a Buffer is supported + const readOnlyDB = new DatabaseSync(Buffer.from(url.toString())); + t.after(() => { readOnlyDB.close(); }); + + t.assert.deepStrictEqual( + readOnlyDB.prepare('SELECT * FROM data').all(), + [{ __proto__: null, key: 1 }] + ); + t.assert.throws(() => { + readOnlyDB.exec('INSERT INTO data (key) VALUES (1);'); + }, { + code: 'ERR_SQLITE_ERROR', + message: 'attempt to write a readonly database', + }); + }); +}); From 265e6fb9472193a9a6148a0e7f479a91d23cfcab Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli Date: Sat, 1 Mar 2025 16:00:17 -0500 Subject: [PATCH 2/2] src: reduce string allocations on sqlite PR-URL: https://github.com/nodejs/node/pull/57227 Reviewed-By: Colin Ihrig Reviewed-By: Edy Silva Reviewed-By: James M Snell Reviewed-By: Zeyu "Alex" Yang --- src/node_sqlite.cc | 48 ++++++++++++++++------------------------------ 1 file changed, 16 insertions(+), 32 deletions(-) diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index c72817eb24672e..0e24e634826645 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -365,54 +365,38 @@ inline sqlite3* DatabaseSync::Connection() { std::optional ValidateDatabasePath(Environment* env, Local path, const std::string& field_name) { - auto has_null_bytes = [](const std::string& str) { - return str.find('\0') != std::string::npos; + constexpr auto has_null_bytes = [](std::string_view str) { + return str.find('\0') != std::string_view::npos; }; - std::string location; if (path->IsString()) { - location = Utf8Value(env->isolate(), path.As()).ToString(); - if (!has_null_bytes(location)) { - return location; + Utf8Value location(env->isolate(), path.As()); + if (!has_null_bytes(location.ToStringView())) { + return location.ToString(); } - } - - if (path->IsUint8Array()) { + } else if (path->IsUint8Array()) { Local buffer = path.As(); size_t byteOffset = buffer->ByteOffset(); size_t byteLength = buffer->ByteLength(); auto data = static_cast(buffer->Buffer()->Data()) + byteOffset; - if (!(std::find(data, data + byteLength, 0) != data + byteLength)) { - Local out; - if (String::NewFromUtf8(env->isolate(), - reinterpret_cast(data), - NewStringType::kNormal, - static_cast(byteLength)) - .ToLocal(&out)) { - return Utf8Value(env->isolate(), out.As()).ToString(); - } + if (std::find(data, data + byteLength, 0) == data + byteLength) { + return std::string(reinterpret_cast(data), byteLength); } - } - - // When is URL - if (path->IsObject()) { - Local url = path.As(); + } else if (path->IsObject()) { // When is URL + auto url = path.As(); Local href; - Local protocol; if (url->Get(env->context(), env->href_string()).ToLocal(&href) && - href->IsString() && - url->Get(env->context(), env->protocol_string()).ToLocal(&protocol) && - protocol->IsString()) { - location = Utf8Value(env->isolate(), href.As()).ToString(); + href->IsString()) { + Utf8Value location_value(env->isolate(), href.As()); + auto location = location_value.ToStringView(); if (!has_null_bytes(location)) { - auto file_url = ada::parse(location); - CHECK(file_url); - if (file_url->type != ada::scheme::FILE) { + CHECK(ada::can_parse(location)); + if (!location.starts_with("file:")) { THROW_ERR_INVALID_URL_SCHEME(env->isolate()); return std::nullopt; } - return location; + return location_value.ToString(); } } }