fix(ext/node): rewrite SQLite named parameter handing (#28197)

Allow bare named params and handle invalid param name. Also adds
`StatementSync#setAllowBareNamedParameters`

Fixes https://github.com/denoland/deno/issues/28183
This commit is contained in:
Divy Srivastava 2025-02-20 18:34:34 +05:30 committed by GitHub
parent 664d50fab8
commit c1276d8fa0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 121 additions and 130 deletions

View file

@ -205,6 +205,7 @@ impl DatabaseSync {
inner: raw_stmt,
db: self.conn.clone(),
use_big_ints: Cell::new(false),
allow_bare_named_params: Cell::new(true),
is_iter_finished: false,
})
}

View file

@ -29,6 +29,8 @@ pub struct StatementSync {
pub db: Rc<RefCell<Option<rusqlite::Connection>>>,
pub use_big_ints: Cell<bool>,
pub allow_bare_named_params: Cell<bool>,
pub is_iter_finished: bool,
}
@ -208,153 +210,32 @@ impl StatementSync {
Ok(Some(result))
}
// Bind the parameters to the prepared statement.
fn bind_params(
fn bind_value(
&self,
scope: &mut v8::HandleScope,
params: Option<&v8::FunctionCallbackArguments>,
) -> Result<(), SqliteError> {
let raw = self.inner;
if let Some(params) = params {
let len = params.length();
let mut param_count = 0;
for i in 0..len {
let value = params.get(i);
if value.is_number() {
let value = value.number_value(scope).unwrap();
// SAFETY: `self.inner` is a valid pointer to a sqlite3_stmt
// as it lives as long as the StatementSync instance.
unsafe {
ffi::sqlite3_bind_double(raw, param_count + 1, value);
}
param_count += 1
} else if value.is_string() {
let value = value.to_rust_string_lossy(scope);
// SAFETY: `self.inner` is a valid pointer to a sqlite3_stmt
// as it lives as long as the StatementSync instance.
//
// SQLITE_TRANSIENT is used to indicate that SQLite should make a copy of the data.
unsafe {
ffi::sqlite3_bind_text(
raw,
param_count + 1,
value.as_ptr() as *const _,
value.len() as i32,
ffi::SQLITE_TRANSIENT(),
);
}
param_count += 1;
} else if value.is_null() {
// SAFETY: `self.inner` is a valid pointer to a sqlite3_stmt
// as it lives as long as the StatementSync instance.
unsafe {
ffi::sqlite3_bind_null(raw, param_count + 1);
}
param_count += 1;
} else if value.is_array_buffer_view() {
let value: v8::Local<v8::ArrayBufferView> = value.try_into().unwrap();
let data = value.data();
let size = value.byte_length();
// SAFETY: `self.inner` is a valid pointer to a sqlite3_stmt
// as it lives as long as the StatementSync instance.
//
// SQLITE_TRANSIENT is used to indicate that SQLite should make a copy of the data.
unsafe {
ffi::sqlite3_bind_blob(
raw,
param_count + 1,
data,
size as i32,
ffi::SQLITE_TRANSIENT(),
);
}
param_count += 1
} else if value.is_big_int() {
let value: v8::Local<v8::BigInt> = value.try_into().unwrap();
let (as_int, lossless) = value.i64_value();
if !lossless {
return Err(SqliteError::FailedBind(
"BigInt value is too large to bind",
));
}
// SAFETY: `self.inner` is a valid pointer to a sqlite3_stmt
// as it lives as long as the StatementSync instance.
unsafe {
ffi::sqlite3_bind_int64(raw, param_count + 1, as_int);
}
param_count += 1;
} else if value.is_object() {
let value: v8::Local<v8::Object> = value.try_into().unwrap();
let maybe_keys =
value.get_property_names(scope, GetPropertyNamesArgs::default());
if let Some(keys) = maybe_keys {
let length = keys.length();
for i in 0..length {
if let Some(key) = keys.get_index(scope, i) {
if let Some(key_str) = key.to_string(scope) {
let key_str = key_str.to_rust_string_lossy(scope);
if let Some(value) = value.get(scope, key) {
self.bind_params_object(
scope,
key_str,
value,
param_count,
)?;
param_count += 1;
}
}
}
}
}
} else {
return Err(SqliteError::FailedBind("Unsupported type"));
}
}
}
Ok(())
}
//helper function that binds the object attribute to named param
fn bind_params_object(
&self,
scope: &mut v8::HandleScope,
key: String,
value: v8::Local<v8::Value>,
count: i32,
index: i32,
) -> Result<(), SqliteError> {
let raw = self.inner;
// SAFETY: `self.inner` is a valid pointer to a sqlite3_stmt
// as it lives as long as the StatementSync instance.
unsafe {
ffi::sqlite3_bind_parameter_index(raw, key.as_ptr() as *const _);
}
if value.is_number() {
let value = value.number_value(scope).unwrap();
// SAFETY: `self.inner` is a valid pointer to a sqlite3_stmt
// as it lives as long as the StatementSync instance.
unsafe {
ffi::sqlite3_bind_double(raw, count + 1, value);
ffi::sqlite3_bind_double(raw, index, value);
}
} else if value.is_string() {
let value = value.to_rust_string_lossy(scope);
// SAFETY: `self.inner` is a valid pointer to a sqlite3_stmt
// as it lives as long as the StatementSync instance.
//
// SQLITE_TRANSIENT is used to indicate that SQLite should make a copy of the data.
unsafe {
ffi::sqlite3_bind_text(
raw,
count + 1,
index,
value.as_ptr() as *const _,
value.len() as i32,
ffi::SQLITE_TRANSIENT(),
@ -364,7 +245,7 @@ impl StatementSync {
// SAFETY: `self.inner` is a valid pointer to a sqlite3_stmt
// as it lives as long as the StatementSync instance.
unsafe {
ffi::sqlite3_bind_null(raw, count + 1);
ffi::sqlite3_bind_null(raw, index);
}
} else if value.is_array_buffer_view() {
let value: v8::Local<v8::ArrayBufferView> = value.try_into().unwrap();
@ -378,7 +259,7 @@ impl StatementSync {
unsafe {
ffi::sqlite3_bind_blob(
raw,
count + 1,
index,
data,
size as i32,
ffi::SQLITE_TRANSIENT(),
@ -396,11 +277,101 @@ impl StatementSync {
// SAFETY: `self.inner` is a valid pointer to a sqlite3_stmt
// as it lives as long as the StatementSync instance.
unsafe {
ffi::sqlite3_bind_int64(raw, count + 1, as_int);
ffi::sqlite3_bind_int64(raw, index, as_int);
}
} else {
return Err(SqliteError::FailedBind("Unsupported type"));
}
Ok(())
}
// Bind the parameters to the prepared statement.
fn bind_params(
&self,
scope: &mut v8::HandleScope,
params: Option<&v8::FunctionCallbackArguments>,
) -> Result<(), SqliteError> {
let raw = self.inner;
let mut anon_start = 0;
if let Some(params) = params {
let param0 = params.get(0);
if param0.is_object() && !param0.is_array_buffer_view() {
let obj = v8::Local::<v8::Object>::try_from(param0).unwrap();
let keys = obj
.get_property_names(scope, GetPropertyNamesArgs::default())
.unwrap();
// Allow specifying named parameters without the SQLite prefix character to improve
// ergonomics. This can be disabled with `StatementSync#setAllowBareNamedParams`.
let mut bare_named_params = std::collections::HashMap::new();
if self.allow_bare_named_params.get() {
// SAFETY: `raw` is a valid pointer to a sqlite3_stmt.
let param_count = unsafe { ffi::sqlite3_bind_parameter_count(raw) };
for i in 1..=param_count {
// SAFETY: `raw` is a valid pointer to a sqlite3_stmt.
let bare_name = unsafe {
let name = ffi::sqlite3_bind_parameter_name(raw, i);
if name.is_null() {
continue;
}
std::ffi::CStr::from_ptr(name.offset(1)).to_bytes()
};
let e = bare_named_params.insert(bare_name, i);
if e.is_some() {
return Err(SqliteError::FailedBind("Duplicate named parameter"));
}
}
}
let len = keys.length();
for j in 0..len {
let key = keys.get_index(scope, j).unwrap();
let key_str = key.to_rust_string_lossy(scope);
let key_c = std::ffi::CString::new(key_str).unwrap();
// SAFETY: `raw` is a valid pointer to a sqlite3_stmt.
let mut r = unsafe {
ffi::sqlite3_bind_parameter_index(raw, key_c.as_ptr() as *const _)
};
if r == 0 {
let lookup = bare_named_params.get(key_c.as_bytes());
if let Some(index) = lookup {
r = *index;
}
if r == 0 {
return Err(SqliteError::FailedBind("Named parameter not found"));
}
}
let value = obj.get(scope, key).unwrap();
self.bind_value(scope, value, r)?;
}
anon_start += 1;
}
let mut anon_idx = 1;
for i in anon_start..params.length() {
// SAFETY: `raw` is a valid pointer to a sqlite3_stmt.
while !unsafe { ffi::sqlite3_bind_parameter_name(raw, anon_idx) }
.is_null()
{
anon_idx += 1;
}
let value = params.get(i);
self.bind_value(scope, value, anon_idx)?;
anon_idx += 1;
}
}
Ok(())
}
}
@ -621,6 +592,11 @@ impl StatementSync {
Ok(iterator)
}
#[fast]
fn set_allow_bare_named_parameters(&self, enabled: bool) {
self.allow_bare_named_params.set(enabled);
}
#[fast]
fn set_read_big_ints(&self, enabled: bool) {
self.use_big_ints.set(enabled);

View file

@ -214,6 +214,20 @@ Deno.test("[node/sqlite] query should handle mixed positional and named paramete
variable3: 2,
}]);
const result2 = db.prepare(query).all({ var2: 1, var1: "test" });
assertEquals(result2, [{
__proto__: null,
variable1: "test",
variable2: 1,
variable3: 2,
}]);
const stmt = db.prepare(query);
stmt.setAllowBareNamedParameters(false);
assertThrows(() => {
stmt.all({ var1: "test", var2: 1 });
});
db.close();
});