diff --git a/changelog.d/9310-mysql2-param-binding.md b/changelog.d/9310-mysql2-param-binding.md new file mode 100644 index 0000000000..e996d49205 --- /dev/null +++ b/changelog.d/9310-mysql2-param-binding.md @@ -0,0 +1,21 @@ +Fix silent data loss in the bundled `mysql2` prepared-statement bridge. Perry +stores strings of at most five bytes in its inline SSO representation, but the +wrapper tested only for heap strings; every short string therefore fell into a +catch-all branch that deliberately emitted SQL NULL. A parameter list made of +short values and nulls arrived at MySQL as all nulls, while longer strings and +primitive numbers took their intended branches. + +The bridge now accepts both Perry string representations without allocating in +the runtime heap and copies every parameter into an owned Rust value before +scheduling database work. Strings, integer and floating-point numbers, +booleans, null, Date, Buffer, and Uint8Array bind with their real values. An +undefined or unsupported parameter rejects the operation with an Error instead +of being rewritten to SQL NULL. Integer results are decoded with their actual +MySQL width and signedness, so a bound boolean no longer comes back as null. +Async `createConnection` and `getConnection` results preserve Perry's registry- +handle tag rather than returning an unusable ordinary number. + +Coverage includes exact extraction assertions for every supported type and a +live-MySQL regression that checks the server-observed values at parameter counts +3, 8, 12, and 17, plus Date and Buffer contents. The checks compare the full +values; a constant non-null substitute cannot pass them. diff --git a/crates/perry-ext-mysql2/src/lib.rs b/crates/perry-ext-mysql2/src/lib.rs index 7d4ae1fcd5..ce6c5d020a 100644 --- a/crates/perry-ext-mysql2/src/lib.rs +++ b/crates/perry-ext-mysql2/src/lib.rs @@ -15,10 +15,10 @@ //! adapter; followup once a wrapper actually demands it). use perry_ffi::{ - alloc_string, build_object_shape, js_array_alloc, js_array_get, js_array_push, + alloc_string, build_object_shape, js_array_alloc, js_array_get, js_array_length, js_array_push, js_object_alloc_with_shape, js_object_get_field, js_object_set_field, register_handle, - spawn_blocking, take_handle, with_handle, ArrayHeader, Handle, JsPromise, JsValue, - ObjectHeader, Promise, StringHeader, + spawn_blocking, take_handle, value_byte_slice, with_handle, ArrayHeader, Handle, JsPromise, + JsValue, ObjectHeader, Promise, StringHeader, SHORT_STRING_MAX_LEN, }; use sqlx::mysql::{MySqlConnection, MySqlDatabaseError, MySqlPool, MySqlPoolOptions, MySqlRow}; use sqlx::pool::PoolConnection; @@ -34,6 +34,12 @@ const DEFAULT_CONNECT_TIMEOUT_SECS: u64 = 10; const DEFAULT_QUERY_TIMEOUT_SECS: u64 = 30; const DEFAULT_ACQUIRE_TIMEOUT_SECS: u64 = 10; +extern "C" { + fn js_array_is_array(value: f64) -> f64; + fn js_date_get_time(value: f64) -> f64; + fn js_util_types_is_date(value: f64) -> f64; +} + /// Connection config — matches perry-stdlib's `MySqlConfig` shape. #[derive(Debug, Clone)] pub struct MySqlConfig { @@ -80,16 +86,22 @@ impl MySqlConfig { } unsafe fn jsvalue_to_string(value: JsValue) -> Option { - if value.is_string() { - let ptr = value.as_string_ptr(); - if !ptr.is_null() { - let len = (*ptr).byte_len as usize; - let data = (ptr as *const u8).add(std::mem::size_of::()); - let bytes = std::slice::from_raw_parts(data, len); - return std::str::from_utf8(bytes).ok().map(String::from); - } + if value.is_short_string() { + let mut bytes = [0; SHORT_STRING_MAX_LEN]; + let len = value.short_string_to_buf(&mut bytes)?; + return std::str::from_utf8(&bytes[..len]).ok().map(String::from); } - None + if !value.is_string() { + return None; + } + let ptr = value.as_string_ptr(); + if ptr.is_null() { + return None; + } + let len = (*ptr).byte_len as usize; + let data = (ptr as *const u8).add(std::mem::size_of::()); + let bytes = std::slice::from_raw_parts(data, len); + std::str::from_utf8(bytes).ok().map(String::from) } /// Percent-decode a URI component (`%25` → `%`, `%40` → `@`, …). A lone `%` @@ -245,15 +257,38 @@ enum QueryOutcome { fn extract_raw_value(row: &MySqlRow, index: usize, type_name: &str) -> RawValue { match type_name { - "INT" | "TINYINT" | "SMALLINT" | "MEDIUMINT" | "INT UNSIGNED" | "TINYINT UNSIGNED" - | "SMALLINT UNSIGNED" | "MEDIUMINT UNSIGNED" => row + "TINYINT" => row + .try_get::(index) + .map(|n| RawValue::Float64(n as f64)) + .unwrap_or(RawValue::Null), + "TINYINT UNSIGNED" => row + .try_get::(index) + .map(|n| RawValue::Float64(n as f64)) + .unwrap_or(RawValue::Null), + "SMALLINT" => row + .try_get::(index) + .map(|n| RawValue::Float64(n as f64)) + .unwrap_or(RawValue::Null), + "SMALLINT UNSIGNED" => row + .try_get::(index) + .map(|n| RawValue::Float64(n as f64)) + .unwrap_or(RawValue::Null), + "MEDIUMINT" | "INT" => row .try_get::(index) .map(|n| RawValue::Float64(n as f64)) .unwrap_or(RawValue::Null), - "BIGINT" | "BIGINT UNSIGNED" => row + "MEDIUMINT UNSIGNED" | "INT UNSIGNED" => row + .try_get::(index) + .map(|n| RawValue::Float64(n as f64)) + .unwrap_or(RawValue::Null), + "BIGINT" => row .try_get::(index) .map(|n| RawValue::Float64(n as f64)) .unwrap_or(RawValue::Null), + "BIGINT UNSIGNED" => row + .try_get::(index) + .map(|n| RawValue::Float64(n as f64)) + .unwrap_or(RawValue::Null), "FLOAT" | "DOUBLE" | "DECIMAL" => row .try_get::(index) .map(RawValue::Float64) @@ -479,6 +514,8 @@ fn is_row_returning_query(sql: &str) -> bool { enum ParamValue { Null, String(String), + Bytes(Vec), + DateTime(chrono::NaiveDateTime), Number(f64), Int(i64), Bool(bool), @@ -522,21 +559,33 @@ impl QueryRequest { } } -unsafe fn extract_params_from_jsvalue(params: JsValue) -> Vec { +unsafe fn extract_params_from_jsvalue(params: JsValue) -> Result, String> { + if params.is_undefined() || params.is_null() { + return Ok(Vec::new()); + } + + let params_f = f64::from_bits(params.bits()); + let is_array = JsValue::from_bits(js_array_is_array(params_f).to_bits()).to_bool(); + if !is_array { + return Err("Bind parameters must be an array".to_string()); + } + let arr_ptr = params.as_pointer::(); if arr_ptr.is_null() { - return Vec::new(); + return Err("Bind parameters array has no valid runtime pointer".to_string()); } - let length = (*arr_ptr).length; + let length = js_array_length(arr_ptr); let mut result = Vec::with_capacity(length as usize); for i in 0..length { let element = js_array_get(arr_ptr, i); - let p = if element.is_null() || element.is_undefined() { + let p = if element.is_null() { ParamValue::Null - } else if element.is_string() { + } else if element.is_undefined() { + return Err(format!("Bind parameter at index {i} is undefined")); + } else if element.is_any_string() { jsvalue_to_string(element) .map(ParamValue::String) - .unwrap_or(ParamValue::Null) + .ok_or_else(|| format!("Could not read string bind parameter at index {i}"))? } else if element.is_int32() { ParamValue::Int(element.to_int32() as i64) } else if element.is_bool() { @@ -548,12 +597,40 @@ unsafe fn extract_params_from_jsvalue(params: JsValue) -> Vec { } else { ParamValue::Number(n) } + } else if let Some(bytes) = value_byte_slice(element) { + // Copy off the Perry heap before the async query is scheduled. + // This covers Buffer and Uint8Array without retaining a raw pointer + // into movable/runtime-owned storage on the worker thread. + ParamValue::Bytes(bytes.to_vec()) } else { - ParamValue::Null + let value_f = f64::from_bits(element.bits()); + let is_date = JsValue::from_bits(js_util_types_is_date(value_f).to_bits()).to_bool(); + if is_date { + let millis = js_date_get_time(value_f); + if !millis.is_finite() { + return Err(format!("Bind parameter at index {i} is an invalid Date")); + } + let millis = millis as i64; + let date = chrono::DateTime::::from_timestamp_millis(millis) + .ok_or_else(|| { + format!("Bind parameter at index {i} is outside MySQL's Date range") + })? + .naive_utc(); + ParamValue::DateTime(date) + } else { + return Err(format!("Unsupported bind parameter at index {i}")); + } }; result.push(p); } - result + Ok(result) +} + +fn rejected_params_promise(message: String) -> *mut Promise { + let promise = JsPromise::new(); + let raw = promise.as_raw(); + promise.reject_string(&message); + raw } unsafe fn read_sql(sql_ptr: *const u8) -> String { @@ -731,6 +808,8 @@ async fn execute_query_on_connection( query = match param { ParamValue::Null => query.bind(Option::::None), ParamValue::String(s) => query.bind(s.clone()), + ParamValue::Bytes(bytes) => query.bind(bytes.clone()), + ParamValue::DateTime(date) => query.bind(*date), ParamValue::Number(n) => query.bind(*n), ParamValue::Int(i) => query.bind(*i), ParamValue::Bool(b) => query.bind(*b), @@ -789,6 +868,7 @@ async fn execute_query_on_target( /// `config_f` is a NaN-boxed JsValue. #[no_mangle] pub unsafe extern "C" fn js_mysql2_create_connection(config_f: f64) -> *mut Promise { + ensure_dispatch_registered(); let config = JsValue::from_bits(config_f.to_bits()); let mysql_config = parse_mysql_config(config); let promise = JsPromise::new(); @@ -807,7 +887,10 @@ pub unsafe extern "C" fn js_mysql2_create_connection(config_f: f64) -> *mut Prom match result { Ok(conn) => { let handle = register_handle(MysqlConnectionHandle::new(conn)); - promise.resolve(JsValue::from_number(handle as f64)); + // Registry handles are pointer-tagged small integers. Returning a + // normal JS number loses that identity, so the first method call + // cannot find the connection and rejects "Invalid connection handle". + promise.resolve(JsValue::from_object_ptr(handle as *mut ())); } Err(error) => error.reject(promise), } @@ -851,7 +934,10 @@ unsafe fn run_connection_query( ) -> *mut Promise { let sql = read_sql(sql_ptr); let params = JsValue::from_bits(params_f.to_bits()); - let param_values = extract_params_from_jsvalue(params); + let param_values = match extract_params_from_jsvalue(params) { + Ok(values) => values, + Err(message) => return rejected_params_promise(message), + }; let request = QueryRequest::new(sql, param_values, rows_as_array, force_prepared); let target = connection_target(conn_handle); @@ -1218,7 +1304,10 @@ unsafe fn run_pool_query( ) -> *mut Promise { let sql = read_sql(sql_ptr); let params = JsValue::from_bits(params_f.to_bits()); - let param_values = extract_params_from_jsvalue(params); + let param_values = match extract_params_from_jsvalue(params) { + Ok(values) => values, + Err(message) => return rejected_params_promise(message), + }; let request = QueryRequest::new(sql, param_values, rows_as_array, force_prepared); let pool = with_handle::(pool_handle, |wrapper| wrapper.pool.clone()); let promise = JsPromise::new(); @@ -1294,7 +1383,7 @@ pub extern "C" fn js_mysql2_pool_get_connection(pool_handle: Handle) -> *mut Pro match result { Ok(conn) => { let h = register_handle(MysqlPoolConnectionHandle::new(conn)); - promise.resolve(JsValue::from_number(h as f64)); + promise.resolve(JsValue::from_object_ptr(h as *mut ())); } Err(error) => error.reject(promise), } @@ -1326,7 +1415,10 @@ unsafe fn run_pool_conn_query( ) -> *mut Promise { let sql = read_sql(sql_ptr); let params = JsValue::from_bits(params_f.to_bits()); - let param_values = extract_params_from_jsvalue(params); + let param_values = match extract_params_from_jsvalue(params) { + Ok(values) => values, + Err(message) => return rejected_params_promise(message), + }; let request = QueryRequest::new(sql, param_values, rows_as_array, force_prepared); let connection = with_handle::(conn_handle, |wrapper| { Arc::clone(&wrapper.connection) @@ -1539,6 +1631,73 @@ mod tests { assert_eq!(transaction_sql_for_method("release"), None); } + #[test] + fn parameter_extraction_preserves_every_supported_value() { + unsafe { + let short = perry_runtime::JSValue::try_short_string(b"hi") + .expect("two-byte string uses the SSO representation"); + let long = alloc_string("long-string"); + let date = perry_runtime::date::js_date_new_from_timestamp(1_706_933_106_789.0); + let buffer = perry_ffi::alloc_buffer(&[0, 1, 127, 128, 255]); + + let mut array = js_array_alloc(8); + for value in [ + JsValue::from_bits(short.bits()), + JsValue::from_string_ptr(long.as_raw()), + JsValue::from_int32(42), + JsValue::from_number(3.25), + JsValue::TRUE, + JsValue::NULL, + JsValue::from_bits(date.to_bits()), + JsValue::from_object_ptr(buffer), + ] { + array = js_array_push(array, value); + } + + let expected_date = chrono::NaiveDate::from_ymd_opt(2024, 2, 3) + .unwrap() + .and_hms_milli_opt(4, 5, 6, 789) + .unwrap(); + let actual = extract_params_from_jsvalue(JsValue::from_object_ptr(array)) + .expect("all supported parameter values must marshal"); + assert_eq!( + actual, + vec![ + ParamValue::String("hi".to_string()), + ParamValue::String("long-string".to_string()), + ParamValue::Int(42), + ParamValue::Number(3.25), + ParamValue::Bool(true), + ParamValue::Null, + ParamValue::DateTime(expected_date), + ParamValue::Bytes(vec![0, 1, 127, 128, 255]), + ] + ); + } + } + + #[test] + fn parameter_extraction_rejects_values_instead_of_substituting_null() { + unsafe { + let mut undefined_array = js_array_alloc(1); + undefined_array = js_array_push(undefined_array, JsValue::UNDEFINED); + let error = extract_params_from_jsvalue(JsValue::from_object_ptr(undefined_array)) + .expect_err("undefined must never become SQL NULL"); + assert_eq!(error, "Bind parameter at index 0 is undefined"); + + let object = perry_ffi::alloc_object(); + let error = extract_params_from_jsvalue(object) + .expect_err("a non-array params container must be rejected"); + assert_eq!(error, "Bind parameters must be an array"); + + let mut object_array = js_array_alloc(1); + object_array = js_array_push(object_array, object); + let error = extract_params_from_jsvalue(JsValue::from_object_ptr(object_array)) + .expect_err("an unsupported parameter must never become SQL NULL"); + assert_eq!(error, "Unsupported bind parameter at index 0"); + } + } + #[test] fn mysql_server_error_metadata_matches_mysql2_shape() { assert_eq!(mysql2_error_code(1062), Some("ER_DUP_ENTRY")); diff --git a/crates/perry-ffi/src/jsvalue.rs b/crates/perry-ffi/src/jsvalue.rs index a0ae4e1303..4198111fbe 100644 --- a/crates/perry-ffi/src/jsvalue.rs +++ b/crates/perry-ffi/src/jsvalue.rs @@ -40,11 +40,17 @@ const TAG_FALSE: u64 = 0x7FFC_0000_0000_0003; const TAG_TRUE: u64 = 0x7FFC_0000_0000_0004; const STRING_TAG: u64 = 0x7FFF_0000_0000_0000; const SHORT_STRING_TAG: u64 = 0x7FF9_0000_0000_0000; +const SHORT_STRING_LEN_SHIFT: u64 = 40; +const SHORT_STRING_LEN_MASK: u64 = 0x0000_FF00_0000_0000; +const SHORT_STRING_DATA_MASK: u64 = 0x0000_00FF_FFFF_FFFF; const POINTER_TAG: u64 = 0x7FFD_0000_0000_0000; const INT32_TAG: u64 = 0x7FFE_0000_0000_0000; const TAG_MASK: u64 = 0xFFFF_0000_0000_0000; const POINTER_MASK: u64 = 0x0000_FFFF_FFFF_FFFF; +/// Maximum UTF-8 byte length stored directly inside an SSO [`JsValue`]. +pub const SHORT_STRING_MAX_LEN: usize = 5; + /// A NaN-boxed JavaScript value, as it crosses the FFI boundary. /// /// `JsValue` is `#[repr(transparent)]` over `u64`, so functions @@ -174,6 +180,24 @@ impl JsValue { (self.0 & TAG_MASK) == SHORT_STRING_TAG } + /// Copy an inline SSO string into `buffer` without allocating in the Perry + /// heap, returning its UTF-8 byte length. Returns `None` when this value is + /// not an inline string. + /// + /// This is preferable to materializing a `StringHeader` while a native + /// wrapper is walking raw runtime pointers: materialization may collect and + /// move the surrounding object or array before the wrapper finishes. + #[inline] + pub fn short_string_to_buf(self, buffer: &mut [u8; SHORT_STRING_MAX_LEN]) -> Option { + if !self.is_short_string() { + return None; + } + let len = ((self.0 & SHORT_STRING_LEN_MASK) >> SHORT_STRING_LEN_SHIFT) as usize; + let bytes = (self.0 & SHORT_STRING_DATA_MASK).to_le_bytes(); + buffer[..len].copy_from_slice(&bytes[..len]); + Some(len) + } + /// True if the value is a heap object pointer (`POINTER_TAG` — /// covers ObjectHeader, ArrayHeader, ClosureHeader, etc). #[inline] @@ -517,6 +541,9 @@ mod tests { let sso = JsValue::from_bits(sso_bits); assert!(sso.is_any_string(), "SSO short string is a string"); assert!(sso.is_short_string(), "SSO short string is the short repr"); + let mut decoded = [0; SHORT_STRING_MAX_LEN]; + assert_eq!(sso.short_string_to_buf(&mut decoded), Some(2)); + assert_eq!(&decoded[..2], b"hi"); assert!( !sso.is_string(), "strict is_string() must NOT match SSO — this is the footgun" @@ -532,6 +559,7 @@ mod tests { !heap.is_short_string(), "a heap STRING_TAG value is not the SSO repr" ); + assert_eq!(heap.short_string_to_buf(&mut decoded), None); // Non-strings: neither predicate fires. for (val, kind) in [ diff --git a/crates/perry-ffi/src/lib.rs b/crates/perry-ffi/src/lib.rs index 533c81fb8e..82fc5b5169 100644 --- a/crates/perry-ffi/src/lib.rs +++ b/crates/perry-ffi/src/lib.rs @@ -75,6 +75,7 @@ pub use jsvalue::{ alloc_null_proto_object, alloc_object, build_object_shape, js_array_alloc, js_array_get, js_array_length, js_array_push, js_array_set, js_object_alloc_with_shape, js_object_get_field, js_object_live_slot_count, js_object_set_field, object_field_by_name, JsValue, + SHORT_STRING_MAX_LEN, }; mod closure; diff --git a/crates/perry-stdlib/src/mysql2/connection.rs b/crates/perry-stdlib/src/mysql2/connection.rs index 7faa0ebc11..9670cab22d 100644 --- a/crates/perry-stdlib/src/mysql2/connection.rs +++ b/crates/perry-stdlib/src/mysql2/connection.rs @@ -127,9 +127,10 @@ pub unsafe extern "C" fn js_mysql2_connection_end(conn_handle: Handle) -> *mut P pub unsafe extern "C" fn js_mysql2_connection_query( conn_handle: Handle, sql_ptr: *const u8, - params: JSValue, + params_f: f64, ) -> *mut Promise { let promise = js_promise_new_cross_thread(); + let params = JSValue::from_bits(params_f.to_bits()); // Extract the SQL string let sql = if sql_ptr.is_null() { @@ -151,6 +152,8 @@ pub unsafe extern "C" fn js_mysql2_connection_query( async move { use tokio::time::timeout; + let param_values = param_values?; + // First try as a regular connection if let Some(wrapper) = get_handle_mut::(conn_handle) { if let Some(conn) = wrapper.connection.as_mut() { @@ -159,6 +162,8 @@ pub unsafe extern "C" fn js_mysql2_connection_query( query = match param { ParamValue::Null => query.bind(Option::::None), ParamValue::String(s) => query.bind(s.clone()), + ParamValue::Bytes(bytes) => query.bind(bytes.clone()), + ParamValue::DateTime(date) => query.bind(*date), ParamValue::Number(n) => query.bind(*n), ParamValue::Int(i) => query.bind(*i), ParamValue::Bool(b) => query.bind(*b), @@ -206,6 +211,8 @@ pub unsafe extern "C" fn js_mysql2_connection_query( query = match param { ParamValue::Null => query.bind(Option::::None), ParamValue::String(s) => query.bind(s.clone()), + ParamValue::Bytes(bytes) => query.bind(bytes.clone()), + ParamValue::DateTime(date) => query.bind(*date), ParamValue::Number(n) => query.bind(*n), ParamValue::Int(i) => query.bind(*i), ParamValue::Bool(b) => query.bind(*b), @@ -260,9 +267,10 @@ pub unsafe extern "C" fn js_mysql2_connection_query( pub unsafe extern "C" fn js_mysql2_connection_execute( conn_handle: Handle, sql_ptr: *const u8, - params: JSValue, + params_f: f64, ) -> *mut Promise { let promise = js_promise_new_cross_thread(); + let params = JSValue::from_bits(params_f.to_bits()); let sql = if sql_ptr.is_null() { String::new() @@ -282,6 +290,8 @@ pub unsafe extern "C" fn js_mysql2_connection_execute( async move { use tokio::time::timeout; + let param_values = param_values?; + // Try as a regular connection first if let Some(wrapper) = get_handle_mut::(conn_handle) { if let Some(conn) = wrapper.connection.as_mut() { @@ -290,6 +300,8 @@ pub unsafe extern "C" fn js_mysql2_connection_execute( query = match param { ParamValue::Null => query.bind(Option::::None), ParamValue::String(s) => query.bind(s.clone()), + ParamValue::Bytes(bytes) => query.bind(bytes.clone()), + ParamValue::DateTime(date) => query.bind(*date), ParamValue::Number(n) => query.bind(*n), ParamValue::Int(i) => query.bind(*i), ParamValue::Bool(b) => query.bind(*b), @@ -350,6 +362,8 @@ pub unsafe extern "C" fn js_mysql2_connection_execute( query = match param { ParamValue::Null => query.bind(Option::::None), ParamValue::String(s) => query.bind(s.clone()), + ParamValue::Bytes(bytes) => query.bind(bytes.clone()), + ParamValue::DateTime(date) => query.bind(*date), ParamValue::Number(n) => query.bind(*n), ParamValue::Int(i) => query.bind(*i), ParamValue::Bool(b) => query.bind(*b), diff --git a/crates/perry-stdlib/src/mysql2/pool.rs b/crates/perry-stdlib/src/mysql2/pool.rs index 8bb242b4d5..80c52f6e41 100644 --- a/crates/perry-stdlib/src/mysql2/pool.rs +++ b/crates/perry-stdlib/src/mysql2/pool.rs @@ -126,9 +126,9 @@ pub unsafe extern "C" fn js_mysql2_pool_end(pool_handle: Handle) -> *mut Promise /// /// `params` is the optional second arg user code passes to `db.query(sql, [..])`. /// The codegen dispatch table for `("mysql2", "Pool", "query")` declares -/// `args: &[NA_STR, NA_PTR]` so the call site always emits 3 arguments +/// `args: &[NA_STR, NA_F64]` so the call site always emits 3 arguments /// (handle + sql + params). When the user omits `params`, codegen pads the -/// slot with `0` (i64 nullptr); `extract_params_from_jsvalue` returns an +/// slot with JS `undefined`; `extract_params_from_jsvalue` returns an /// empty Vec for that case. When the user passes an array, sqlx builds a /// prepared statement and binds each value — same code path as /// `js_mysql2_pool_execute`. Without binding, sqlx sends the binary execute @@ -139,9 +139,10 @@ pub unsafe extern "C" fn js_mysql2_pool_end(pool_handle: Handle) -> *mut Promise pub unsafe extern "C" fn js_mysql2_pool_query( pool_handle: Handle, sql_ptr: *const u8, - params: JSValue, + params_f: f64, ) -> *mut Promise { let promise = js_promise_new_cross_thread(); + let params = JSValue::from_bits(params_f.to_bits()); // Extract the SQL string let sql = if sql_ptr.is_null() { @@ -168,6 +169,8 @@ pub unsafe extern "C" fn js_mysql2_pool_query( use crate::common::get_handle; use tokio::time::timeout; + let param_values = param_values?; + if let Some(wrapper) = get_handle::(pool_handle) { // Build the query with parameter bindings (no-op when // param_values is empty, preserving the no-param call shape). @@ -176,6 +179,8 @@ pub unsafe extern "C" fn js_mysql2_pool_query( query = match param { ParamValue::Null => query.bind(Option::::None), ParamValue::String(s) => query.bind(s.clone()), + ParamValue::Bytes(bytes) => query.bind(bytes.clone()), + ParamValue::DateTime(date) => query.bind(*date), ParamValue::Number(n) => query.bind(*n), ParamValue::Int(i) => query.bind(*i), ParamValue::Bool(b) => query.bind(*b), @@ -239,9 +244,10 @@ pub unsafe extern "C" fn js_mysql2_pool_query( pub unsafe extern "C" fn js_mysql2_pool_execute( pool_handle: Handle, sql_ptr: *const u8, - params: JSValue, + params_f: f64, ) -> *mut Promise { let promise = js_promise_new_cross_thread(); + let params = JSValue::from_bits(params_f.to_bits()); // Extract the SQL string let sql = if sql_ptr.is_null() { @@ -265,6 +271,8 @@ pub unsafe extern "C" fn js_mysql2_pool_execute( use crate::common::get_handle; use tokio::time::timeout; + let param_values = param_values?; + if let Some(wrapper) = get_handle::(pool_handle) { // Build the query with parameter bindings let mut query = sqlx::query(sqlx::AssertSqlSafe(sql.clone())); @@ -273,6 +281,8 @@ pub unsafe extern "C" fn js_mysql2_pool_execute( query = match param { ParamValue::Null => query.bind(Option::::None), ParamValue::String(s) => query.bind(s.clone()), + ParamValue::Bytes(bytes) => query.bind(bytes.clone()), + ParamValue::DateTime(date) => query.bind(*date), ParamValue::Number(n) => query.bind(*n), ParamValue::Int(i) => query.bind(*i), ParamValue::Bool(b) => query.bind(*b), @@ -327,21 +337,36 @@ pub unsafe extern "C" fn js_mysql2_pool_execute( } /// Enum to hold different parameter value types -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq)] pub(crate) enum ParamValue { Null, String(String), + Bytes(Vec), + DateTime(chrono::NaiveDateTime), Number(f64), Int(i64), Bool(bool), } /// Extract parameter values from a JSValue array -pub(crate) unsafe fn extract_params_from_jsvalue(params: JSValue) -> Vec { +pub(crate) unsafe fn extract_params_from_jsvalue( + params: JSValue, +) -> Result, String> { let mut result = Vec::new(); let bits = params.bits(); + if bits == 0 || params.is_undefined() || params.is_null() { + return Ok(result); + } + + let is_array = + JSValue::from_bits(perry_runtime::js_array_is_array(f64::from_bits(bits)).to_bits()) + .as_bool(); + if !is_array { + return Err("Bind parameters must be an array".to_string()); + } + // Handle both NaN-boxed pointers and raw pointers let arr_ptr: *const perry_runtime::ArrayHeader = if params.is_pointer() { // NaN-boxed pointer (POINTER_TAG = 0x7FFD) @@ -353,14 +378,14 @@ pub(crate) unsafe fn extract_params_from_jsvalue(params: JSValue) -> Vec 0 && upper < 0x7FF0) { bits as *const perry_runtime::ArrayHeader } else { - return result; + return Err("Bind parameters array has no valid runtime pointer".to_string()); } } else { - return result; + return Err("Bind parameters array has no valid runtime pointer".to_string()); }; if arr_ptr.is_null() { - return result; + return Err("Bind parameters array has no valid runtime pointer".to_string()); } let length = js_array_length(arr_ptr); @@ -369,19 +394,26 @@ pub(crate) unsafe fn extract_params_from_jsvalue(params: JSValue) -> Vec()); let bytes = std::slice::from_raw_parts(data_ptr, len); ParamValue::String(String::from_utf8_lossy(bytes).to_string()) - } else { - ParamValue::Null } } else if element.is_bigint() { // Convert BigInt to string (MySQL handles numeric strings correctly) @@ -414,19 +446,34 @@ pub(crate) unsafe fn extract_params_from_jsvalue(params: JSValue) -> Vec= i64::MIN as f64 && n <= i64::MAX as f64 { - ParamValue::Int(n as i64) + let mut byte_len = 0; + let byte_ptr = perry_runtime::buffer::js_value_buffer_or_typedarray_data( + f64::from_bits(element_bits), + &mut byte_len, + ); + if !byte_ptr.is_null() { + let bytes = std::slice::from_raw_parts(byte_ptr, byte_len as usize); + ParamValue::Bytes(bytes.to_vec()) + } else if perry_runtime::date::is_date_value(f64::from_bits(element_bits)) { + let millis = perry_runtime::date::js_date_get_time(f64::from_bits(element_bits)); + if !millis.is_finite() { + return Err(format!("Bind parameter at index {i} is an invalid Date")); + } + let date = chrono::DateTime::::from_timestamp_millis(millis as i64) + .ok_or_else(|| { + format!("Bind parameter at index {i} is outside MySQL's Date range") + })? + .naive_utc(); + ParamValue::DateTime(date) } else { - ParamValue::Number(n) + return Err(format!("Unsupported bind parameter at index {i}")); } }; result.push(param); } - result + Ok(result) } /// pool.getConnection() -> Promise @@ -495,9 +542,10 @@ pub unsafe extern "C" fn js_mysql2_pool_connection_release(conn_handle: Handle) pub unsafe extern "C" fn js_mysql2_pool_connection_query( conn_handle: Handle, sql_ptr: *const u8, - params: JSValue, + params_f: f64, ) -> *mut Promise { let promise = js_promise_new_cross_thread(); + let params = JSValue::from_bits(params_f.to_bits()); // Extract the SQL string let sql = if sql_ptr.is_null() { @@ -519,6 +567,8 @@ pub unsafe extern "C" fn js_mysql2_pool_connection_query( use crate::common::get_handle_mut; use tokio::time::timeout; + let param_values = param_values?; + if let Some(wrapper) = get_handle_mut::(conn_handle) { if let Some(ref mut conn) = wrapper.connection { let mut query = sqlx::query(sqlx::AssertSqlSafe(sql.clone())); @@ -526,6 +576,8 @@ pub unsafe extern "C" fn js_mysql2_pool_connection_query( query = match param { ParamValue::Null => query.bind(Option::::None), ParamValue::String(s) => query.bind(s.clone()), + ParamValue::Bytes(bytes) => query.bind(bytes.clone()), + ParamValue::DateTime(date) => query.bind(*date), ParamValue::Number(n) => query.bind(*n), ParamValue::Int(i) => query.bind(*i), ParamValue::Bool(b) => query.bind(*b), @@ -589,9 +641,10 @@ pub unsafe extern "C" fn js_mysql2_pool_connection_query( pub unsafe extern "C" fn js_mysql2_pool_connection_execute( conn_handle: Handle, sql_ptr: *const u8, - params: JSValue, + params_f: f64, ) -> *mut Promise { let promise = js_promise_new_cross_thread(); + let params = JSValue::from_bits(params_f.to_bits()); // Extract the SQL string let sql = if sql_ptr.is_null() { @@ -614,6 +667,8 @@ pub unsafe extern "C" fn js_mysql2_pool_connection_execute( use crate::common::get_handle_mut; use tokio::time::timeout; + let param_values = param_values?; + if let Some(wrapper) = get_handle_mut::(conn_handle) { if let Some(ref mut conn) = wrapper.connection { // Build the query with parameter bindings @@ -623,6 +678,8 @@ pub unsafe extern "C" fn js_mysql2_pool_connection_execute( query = match param { ParamValue::Null => query.bind(Option::::None), ParamValue::String(s) => query.bind(s.clone()), + ParamValue::Bytes(bytes) => query.bind(bytes.clone()), + ParamValue::DateTime(date) => query.bind(*date), ParamValue::Number(n) => query.bind(*n), ParamValue::Int(i) => query.bind(*i), ParamValue::Bool(b) => query.bind(*b), @@ -686,13 +743,12 @@ mod tests { /// Issue #414: every shape the codegen dispatch table can pass to a /// mysql2 query/execute params slot must be safely consumable. /// - /// The dispatcher emits `args: &[NA_STR, NA_PTR]` for both `db.query` + /// The dispatcher emits `args: &[NA_STR, NA_F64]` for both `db.query` /// and `db.execute`, which in user code can be: - /// - missing (`db.query(sql)`) — codegen pads NA_PTR with `0` + /// - missing (`db.query(sql)`) — codegen passes JS `undefined` /// - undefined (`db.query(sql, undefined)`) — codegen passes TAG_UNDEFINED - /// - an array (`db.query(sql, [42])`) — codegen passes the unboxed - /// `*const ArrayHeader` (raw, lower 48 bits, no tag) - /// - a NaN-boxed pointer (defensive: in case some caller skips unbox) + /// - an array (`db.query(sql, [42])`) — codegen passes its NaN-boxed value + /// - a raw pointer (defensive compatibility with the old NA_PTR ABI) /// /// Pre-fix, only the no-params + execute-only paths were exercised; the /// query path silently dropped a non-zero params arg because the FFI @@ -700,19 +756,20 @@ mod tests { /// extract-and-bind path; these tests pin the four shapes. #[test] fn extract_params_returns_empty_for_codegen_no_args_pad() { - // codegen pads NA_PTR with the literal i64 `0` when the user - // omitted the params arg — the fast path for `db.query(sql)`. - let v = unsafe { extract_params_from_jsvalue(JSValue::from_bits(0)) }; + // Keep accepting the literal `0` used by the old NA_PTR ABI. + let v = unsafe { extract_params_from_jsvalue(JSValue::from_bits(0)) }.unwrap(); assert!(v.is_empty(), "raw 0 must yield no params"); } #[test] fn extract_params_returns_empty_for_undefined_and_null() { let undef = - unsafe { extract_params_from_jsvalue(JSValue::from_bits(0x7FFC_0000_0000_0001)) }; + unsafe { extract_params_from_jsvalue(JSValue::from_bits(0x7FFC_0000_0000_0001)) } + .unwrap(); assert!(undef.is_empty(), "TAG_UNDEFINED must yield no params"); let null = - unsafe { extract_params_from_jsvalue(JSValue::from_bits(0x7FFC_0000_0000_0002)) }; + unsafe { extract_params_from_jsvalue(JSValue::from_bits(0x7FFC_0000_0000_0002)) } + .unwrap(); assert!(null.is_empty(), "TAG_NULL must yield no params"); } @@ -732,7 +789,7 @@ mod tests { // Codegen unboxes the NaN-boxed pointer to a raw i64. Mimic that // by passing the raw lower-48-bits pointer (no tag). let raw_ptr = arr as u64; - let v = extract_params_from_jsvalue(JSValue::from_bits(raw_ptr)); + let v = extract_params_from_jsvalue(JSValue::from_bits(raw_ptr)).unwrap(); assert_eq!(v.len(), 2, "should extract two int params"); match &v[0] { ParamValue::Int(n) => assert_eq!(*n, 42), @@ -756,7 +813,7 @@ mod tests { // Defensive path: caller forgets to unbox before passing. let nan_boxed = (arr as u64) | 0x7FFD_0000_0000_0000; - let v = extract_params_from_jsvalue(JSValue::from_bits(nan_boxed)); + let v = extract_params_from_jsvalue(JSValue::from_bits(nan_boxed)).unwrap(); assert_eq!(v.len(), 1); match &v[0] { ParamValue::Int(n) => assert_eq!(*n, 123), diff --git a/crates/perry-stdlib/src/mysql2/result.rs b/crates/perry-stdlib/src/mysql2/result.rs index dc118dfcba..5f57affb17 100644 --- a/crates/perry-stdlib/src/mysql2/result.rs +++ b/crates/perry-stdlib/src/mysql2/result.rs @@ -133,21 +133,62 @@ impl RawQueryResult { /// Extract a raw value from a MySQL row (safe to call on any thread) fn extract_raw_value(row: &MySqlRow, index: usize, type_name: &str) -> RawValue { match type_name { - "INT" | "TINYINT" | "SMALLINT" | "MEDIUMINT" | "INT UNSIGNED" | "TINYINT UNSIGNED" - | "SMALLINT UNSIGNED" | "MEDIUMINT UNSIGNED" => { + "TINYINT" => { + if let Ok(val) = row.try_get::(index) { + RawValue::Int32(val as i32) + } else { + RawValue::Null + } + } + "TINYINT UNSIGNED" => { + if let Ok(val) = row.try_get::(index) { + RawValue::Int32(val as i32) + } else { + RawValue::Null + } + } + "SMALLINT" => { + if let Ok(val) = row.try_get::(index) { + RawValue::Int32(val as i32) + } else { + RawValue::Null + } + } + "SMALLINT UNSIGNED" => { + if let Ok(val) = row.try_get::(index) { + RawValue::Int32(val as i32) + } else { + RawValue::Null + } + } + "MEDIUMINT" | "INT" => { if let Ok(val) = row.try_get::(index) { RawValue::Int32(val) } else { RawValue::Null } } - "BIGINT" | "BIGINT UNSIGNED" => { + "MEDIUMINT UNSIGNED" | "INT UNSIGNED" => { + if let Ok(val) = row.try_get::(index) { + RawValue::Float64(val as f64) + } else { + RawValue::Null + } + } + "BIGINT" => { if let Ok(val) = row.try_get::(index) { RawValue::Int64(val) } else { RawValue::Null } } + "BIGINT UNSIGNED" => { + if let Ok(val) = row.try_get::(index) { + RawValue::Float64(val as f64) + } else { + RawValue::Null + } + } "FLOAT" | "DOUBLE" | "DECIMAL" => { if let Ok(val) = row.try_get::(index) { RawValue::Float64(val) diff --git a/test-files/test_issue_9310_mysql2_param_values.ts b/test-files/test_issue_9310_mysql2_param_values.ts new file mode 100644 index 0000000000..4348453bc0 --- /dev/null +++ b/test-files/test_issue_9310_mysql2_param_values.ts @@ -0,0 +1,114 @@ +// parity-skip: requires a live MySQL fixture; native wrapper unit-tested +// Regression coverage for issue #9310: mysql2 prepared-statement values must +// survive the JS-value -> Rust -> sqlx binding boundary without substitution. +// +// Run with DB_HOST, DB_PORT, DB_USER, DB_PASSWORD, and DB_NAME set. The SQL +// normalizes Date and Buffer parameters to strings so the assertions inspect +// their exact server-observed contents independently of result-decoder types. +// platforms: skip + +import mysql from 'mysql2/promise'; + +function assertJson(label: string, actual: unknown, expected: unknown): void { + const got = JSON.stringify(actual); + const want = JSON.stringify(expected); + if (got !== want) { + throw new Error(`${label}: expected ${want}, got ${got}`); + } +} + +async function main(): Promise { + const config = { + host: process.env.DB_HOST, + port: Number(process.env.DB_PORT ?? 3306), + user: process.env.DB_USER, + password: process.env.DB_PASSWORD, + database: process.env.DB_NAME, + }; + const pool = mysql.createPool(config); + + try { + for (const count of [3, 8, 12, 17]) { + const sent: (string | null)[] = []; + for (let i = 0; i < count; i += 1) { + sent.push(i % 3 === 1 ? null : `v${i}`); + } + const sql = 'SELECT ' + sent.map((_, i) => `? AS c${i}`).join(', '); + const [rows] = await pool.execute(sql, sent); + const row = (rows as Record[])[0]; + const actual = sent.map((_, i) => row[`c${i}`]); + assertJson(`${count}-parameter values`, actual, sent); + } + + const sentDate = new Date('2024-02-03T04:05:06.789Z'); + const sentBuffer = Buffer.from([0, 1, 127, 128, 255]); + const [rows] = await pool.execute( + [ + 'SELECT ? AS shortString', + ', ? AS longString', + ', ? AS intValue', + ', ? AS floatValue', + ', ? AS boolValue', + ', ? AS nullValue', + ", DATE_FORMAT(?, '%Y-%m-%dT%H:%i:%s.%f') AS dateValue", + ', HEX(?) AS bufferHex', + ].join(''), + ['hi', 'long-string', 42, 3.25, true, null, sentDate, sentBuffer], + ); + const row = (rows as Record[])[0]; + assertJson( + 'typed parameter values', + { + shortString: row.shortString, + longString: row.longString, + intValue: row.intValue, + floatValue: row.floatValue, + boolValue: row.boolValue, + nullValue: row.nullValue, + dateValue: row.dateValue, + bufferHex: row.bufferHex, + }, + { + shortString: 'hi', + longString: 'long-string', + intValue: 42, + floatValue: 3.25, + boolValue: 1, + nullValue: null, + dateValue: '2024-02-03T04:05:06.789000', + bufferHex: '00017F80FF', + }, + ); + + const connection = await mysql.createConnection(config); + try { + const [directRows] = await connection.execute( + 'SELECT ? AS shortString, ? AS intValue, ? AS boolValue', + ['five!', 9310, true], + ); + assertJson('direct-connection parameter values', directRows, [ + { shortString: 'five!', intValue: 9310, boolValue: 1 }, + ]); + } finally { + await connection.end(); + } + + let rejectedMessage = ''; + try { + await pool.execute('SELECT ? AS unsupported', [undefined]); + } catch (error: any) { + rejectedMessage = String(error && error.message ? error.message : error); + } + if (!rejectedMessage.includes('undefined')) { + throw new Error( + `undefined bind parameter was not rejected loudly: ${rejectedMessage}`, + ); + } + + console.log('issue 9310 mysql2 parameter values: OK'); + } finally { + await pool.end(); + } +} + +main();