-
-
Notifications
You must be signed in to change notification settings - Fork 31.7k
sqlite,test,doc: accept Buffer and URL as paths #56991
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
nodejs-github-bot
merged 12 commits into
nodejs:main
from
geeksilva97:sqlite-accept-buffer
Feb 27, 2025
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
53a0251
sqlite,test,doc: allow Buffer and URL as database location
geeksilva97 2533cf1
test: remove unneeded comments
geeksilva97 3eb3a7e
fixup: add env properties
geeksilva97 abf3fa7
sqlite,test: test url scheme
geeksilva97 9ab8acb
sqlite: refactor database path validation
geeksilva97 9af5f05
test: improve test cleanup
geeksilva97 ada95c1
sqlite,doc: add changelog
geeksilva97 95495b0
fixup: use std::optional instead of boolean
geeksilva97 bf17da6
test: test null bytes for String and Buffer
geeksilva97 79c1ca1
sqlite: ensure query params are used
geeksilva97 8508790
sqlite,doc: make path argument consistent in all interfaces
geeksilva97 7d1111f
sqlite,test: test URI passed as string or Buffer
geeksilva97 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ | |
#include "node.h" | ||
#include "node_errors.h" | ||
#include "node_mem-inl.h" | ||
#include "node_url.h" | ||
#include "sqlite3.h" | ||
#include "threadpoolwork-inl.h" | ||
#include "util-inl.h" | ||
|
@@ -181,10 +182,11 @@ class BackupJob : public ThreadPoolWork { | |
void ScheduleBackup() { | ||
Isolate* isolate = env()->isolate(); | ||
HandleScope handle_scope(isolate); | ||
backup_status_ = sqlite3_open_v2(destination_name_.c_str(), | ||
&dest_, | ||
SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, | ||
nullptr); | ||
backup_status_ = sqlite3_open_v2( | ||
destination_name_.c_str(), | ||
&dest_, | ||
SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_URI, | ||
nullptr); | ||
Local<Promise::Resolver> resolver = | ||
Local<Promise::Resolver>::New(env()->isolate(), resolver_); | ||
if (backup_status_ != SQLITE_OK) { | ||
|
@@ -503,11 +505,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(), this, r, SQLITE_OK, false); | ||
|
||
r = sqlite3_db_config(connection_, | ||
|
@@ -585,27 +590,85 @@ bool DatabaseSync::ShouldIgnoreSQLiteError() { | |
return ignore_next_sqlite_error_; | ||
} | ||
|
||
std::optional<std::string> ValidateDatabasePath(Environment* env, | ||
geeksilva97 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Local<Value> 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<String>()).ToString(); | ||
if (!has_null_bytes(location)) { | ||
return location; | ||
} | ||
} | ||
|
||
if (path->IsUint8Array()) { | ||
Local<Uint8Array> buffer = path.As<Uint8Array>(); | ||
size_t byteOffset = buffer->ByteOffset(); | ||
size_t byteLength = buffer->ByteLength(); | ||
auto data = | ||
static_cast<const uint8_t*>(buffer->Buffer()->Data()) + byteOffset; | ||
if (!(std::find(data, data + byteLength, 0) != data + byteLength)) { | ||
geeksilva97 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Local<Value> out; | ||
if (String::NewFromUtf8(env->isolate(), | ||
reinterpret_cast<const char*>(data), | ||
NewStringType::kNormal, | ||
static_cast<int>(byteLength)) | ||
.ToLocal(&out)) { | ||
return Utf8Value(env->isolate(), out.As<String>()).ToString(); | ||
} | ||
} | ||
} | ||
|
||
// When is URL | ||
if (path->IsObject()) { | ||
Local<Object> url = path.As<Object>(); | ||
Local<Value> href; | ||
Local<Value> 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<String>()).ToString(); | ||
geeksilva97 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (!has_null_bytes(location)) { | ||
geeksilva97 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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; | ||
} | ||
Comment on lines
+638
to
+641
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed the Guaranteeing that a valid URL is given we can return the URL href it is. |
||
|
||
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<Value>& 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<std::string> location = | ||
ValidateDatabasePath(env, args[0], "path"); | ||
if (!location.has_value()) { | ||
return; | ||
} | ||
|
||
std::string location = | ||
Utf8Value(env->isolate(), args[0].As<String>()).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(), | ||
|
@@ -984,17 +1047,15 @@ void Backup(const FunctionCallbackInfo<Value>& args) { | |
DatabaseSync* db; | ||
ASSIGN_OR_RETURN_UNWRAP(&db, args[0].As<Object>()); | ||
THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open"); | ||
if (!args[1]->IsString()) { | ||
THROW_ERR_INVALID_ARG_TYPE( | ||
env->isolate(), "The \"destination\" argument must be a string."); | ||
std::optional<std::string> dest_path = | ||
ValidateDatabasePath(env, args[1], "path"); | ||
if (!dest_path.has_value()) { | ||
return; | ||
} | ||
|
||
int rate = 100; | ||
std::string source_db = "main"; | ||
std::string dest_db = "main"; | ||
|
||
Utf8Value dest_path(env->isolate(), args[1].As<String>()); | ||
Local<Function> progressFunc = Local<Function>(); | ||
|
||
if (args.Length() > 2) { | ||
|
@@ -1077,12 +1138,11 @@ void Backup(const FunctionCallbackInfo<Value>& args) { | |
} | ||
|
||
args.GetReturnValue().Set(resolver->GetPromise()); | ||
|
||
BackupJob* job = new BackupJob(env, | ||
db, | ||
resolver, | ||
std::move(source_db), | ||
*dest_path, | ||
dest_path.value(), | ||
std::move(dest_db), | ||
rate, | ||
progressFunc); | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.