From 46d3fb8f8e7672780149ca188c9d2e7e7fd7e125 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Thu, 11 Jun 2026 11:48:04 -0700 Subject: [PATCH 1/4] sql: move QueryStatus to shared/, add StatementStatus, and give FieldMessage a payload() accessor --- src/sql/lib.rs | 9 +++-- src/sql/postgres/protocol/ErrorResponse.rs | 14 ++++++++ src/sql/postgres/protocol/FieldMessage.rs | 11 ++++-- src/sql/postgres/protocol/NoticeResponse.rs | 40 +++------------------ src/sql/{mysql => shared}/QueryStatus.rs | 0 src/sql/shared/StatementStatus.rs | 13 +++++++ 6 files changed, 46 insertions(+), 41 deletions(-) rename src/sql/{mysql => shared}/QueryStatus.rs (100%) create mode 100644 src/sql/shared/StatementStatus.rs diff --git a/src/sql/lib.rs b/src/sql/lib.rs index b6716c9705f..a43503328f8 100644 --- a/src/sql/lib.rs +++ b/src/sql/lib.rs @@ -7,8 +7,12 @@ pub mod shared { pub mod connection_flags; #[path = "Data.rs"] pub mod data; + #[path = "QueryStatus.rs"] + pub mod query_status; #[path = "SQLQueryResultMode.rs"] pub mod sql_query_result_mode; + #[path = "StatementStatus.rs"] + pub mod statement_status; pub use column_identifier::ColumnIdentifier; pub use connection_flags::ConnectionFlags; @@ -31,8 +35,6 @@ pub mod mysql { pub mod mysql_request; #[path = "MySQLTypes.rs"] pub mod mysql_types; - #[path = "QueryStatus.rs"] - pub mod query_status; #[path = "SSLMode.rs"] pub mod ssl_mode; #[path = "StatusFlags.rs"] @@ -118,11 +120,12 @@ pub mod mysql { pub use crate::mysql::mysql_types::FieldType; } + pub use crate::shared::query_status; + pub use crate::shared::query_status::Status as QueryStatus; pub use auth_method::AuthMethod; pub use capabilities::Capabilities; pub use connection_state::ConnectionState; pub use mysql_query_result::MySQLQueryResult; - pub use query_status::Status as QueryStatus; pub use ssl_mode::SSLMode; pub use status_flags::{StatusFlag, StatusFlags}; pub use tls_status::TLSStatus; diff --git a/src/sql/postgres/protocol/ErrorResponse.rs b/src/sql/postgres/protocol/ErrorResponse.rs index 0930a1601a7..3f7ac272cf2 100644 --- a/src/sql/postgres/protocol/ErrorResponse.rs +++ b/src/sql/postgres/protocol/ErrorResponse.rs @@ -41,6 +41,20 @@ impl ErrorResponse { ) -> Result { Self::decode_internal(NewReader { wrapped: context }) } + + /// `NoticeResponse` decode: a declared length below 4 decodes as an empty + /// notice instead of failing, unlike `ErrorResponse`. + pub fn decode_notice_internal( + mut reader: NewReader, + ) -> Result { + let remaining_bytes = reader.length()?.saturating_sub(4); + if remaining_bytes > 0 { + return Ok(Self { + messages: FieldMessage::decode_list::(reader)?, + }); + } + Ok(Self::default()) + } } // `to_js` lives on an extension trait in the `bun_sql_jsc` crate. diff --git a/src/sql/postgres/protocol/FieldMessage.rs b/src/sql/postgres/protocol/FieldMessage.rs index 47967a306ab..43fae887007 100644 --- a/src/sql/postgres/protocol/FieldMessage.rs +++ b/src/sql/postgres/protocol/FieldMessage.rs @@ -29,6 +29,13 @@ pub enum FieldMessage { impl fmt::Display for FieldMessage { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.payload()) + } +} + +impl FieldMessage { + /// Every variant carries a single `bun.String` payload. + pub fn payload(&self) -> &String { match self { FieldMessage::Severity(s) | FieldMessage::LocalizedSeverity(s) @@ -47,12 +54,10 @@ impl fmt::Display for FieldMessage { | FieldMessage::Constraint(s) | FieldMessage::File(s) | FieldMessage::Line(s) - | FieldMessage::Routine(s) => write!(f, "{s}"), + | FieldMessage::Routine(s) => s, } } -} -impl FieldMessage { pub fn decode_list( mut reader: NewReader, ) -> Result, AnyPostgresError> { diff --git a/src/sql/postgres/protocol/NoticeResponse.rs b/src/sql/postgres/protocol/NoticeResponse.rs index 07fb345f33e..6ba4dfb3bb3 100644 --- a/src/sql/postgres/protocol/NoticeResponse.rs +++ b/src/sql/postgres/protocol/NoticeResponse.rs @@ -1,35 +1,5 @@ -use super::field_message::FieldMessage; -use super::new_reader::NewReader; -use crate::postgres::AnyPostgresError; - -#[derive(Default)] -pub struct NoticeResponse { - pub messages: Vec, -} - -// Vec drops each element (FieldMessage's Drop) and the buffer -// automatically, so no explicit Drop body is needed. - -impl NoticeResponse { - pub fn decode_internal( - mut reader: NewReader, - ) -> Result { - let mut remaining_bytes = reader.length()?; - remaining_bytes = remaining_bytes.saturating_sub(4); - - if remaining_bytes > 0 { - return Ok(Self { - messages: FieldMessage::decode_list::(reader)?, - }); - } - Ok(Self::default()) - } - - pub fn decode( - context: Container, - ) -> Result { - Self::decode_internal(NewReader { wrapped: context }) - } -} - -// `to_js` lives as an extension-trait method in the bun_sql_jsc crate. +/// NoticeResponse has the same wire format as ErrorResponse — a length-prefixed +/// list of field messages — so it reuses the same type. Notices decode via +/// `decode_notice_internal`, which tolerates a declared length below 4 +/// (decoding as empty) where `ErrorResponse` fails. +pub type NoticeResponse = crate::postgres::protocol::error_response::ErrorResponse; diff --git a/src/sql/mysql/QueryStatus.rs b/src/sql/shared/QueryStatus.rs similarity index 100% rename from src/sql/mysql/QueryStatus.rs rename to src/sql/shared/QueryStatus.rs diff --git a/src/sql/shared/StatementStatus.rs b/src/sql/shared/StatementStatus.rs new file mode 100644 index 00000000000..bce4a693f31 --- /dev/null +++ b/src/sql/shared/StatementStatus.rs @@ -0,0 +1,13 @@ +#[derive(Copy, Clone, Eq, PartialEq)] +pub enum Status { + Pending, + Parsing, + Prepared, + Failed, +} + +impl Status { + pub fn is_running(self) -> bool { + self == Status::Parsing + } +} From dc96ffda531e16326ca7899653a1c09d3cbcfbe1 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Thu, 11 Jun 2026 11:48:08 -0700 Subject: [PATCH 2/4] sql_jsc: extract shared query/connection ctor-arg parsing and statement status across drivers --- src/sql_jsc/lib.rs | 4 + src/sql_jsc/mysql/JSMySQLConnection.rs | 86 ++--------- src/sql_jsc/mysql/JSMySQLQuery.rs | 46 ++---- src/sql_jsc/mysql/MySQLStatement.rs | 57 +------- src/sql_jsc/postgres.rs | 3 - src/sql_jsc/postgres/PostgresSQLConnection.rs | 93 +++--------- src/sql_jsc/postgres/PostgresSQLQuery.rs | 70 ++------- src/sql_jsc/postgres/PostgresSQLStatement.rs | 66 +-------- .../postgres/protocol/error_response_jsc.rs | 6 +- .../postgres/protocol/notice_response_jsc.rs | 28 ---- src/sql_jsc/shared/SQLDataCell.rs | 53 ++++++- src/sql_jsc/shared/connection_ctor_args.rs | 136 ++++++++++++++++++ src/sql_jsc/shared/query_ctor_args.rs | 64 +++++++++ 13 files changed, 313 insertions(+), 399 deletions(-) delete mode 100644 src/sql_jsc/postgres/protocol/notice_response_jsc.rs create mode 100644 src/sql_jsc/shared/connection_ctor_args.rs create mode 100644 src/sql_jsc/shared/query_ctor_args.rs diff --git a/src/sql_jsc/lib.rs b/src/sql_jsc/lib.rs index 4d25a4a93b4..4a47a8066a8 100644 --- a/src/sql_jsc/lib.rs +++ b/src/sql_jsc/lib.rs @@ -20,6 +20,8 @@ pub mod shared { #[path = "CachedStructure.rs"] pub mod cached_structure; + pub mod connection_ctor_args; + pub mod datetime_text; #[path = "ObjectIterator.rs"] @@ -28,6 +30,8 @@ pub mod shared { #[path = "QueryBindingIterator.rs"] pub mod query_binding_iterator; + pub mod query_ctor_args; + #[path = "SQLDataCell.rs"] pub mod sql_data_cell; diff --git a/src/sql_jsc/mysql/JSMySQLConnection.rs b/src/sql_jsc/mysql/JSMySQLConnection.rs index 03063446695..a16b70dafac 100644 --- a/src/sql_jsc/mysql/JSMySQLConnection.rs +++ b/src/sql_jsc/mysql/JSMySQLConnection.rs @@ -4,11 +4,11 @@ use core::ffi::c_void; use crate::jsc::{ CallFrame, EventLoopSqlExt as _, EventLoopTimer, EventLoopTimerState, EventLoopTimerTag, GlobalRef, HasAutoFlush, JSGlobalObject, JSValue, JsCell, JsRef, JsResult, KeepAlive, - VirtualMachine, VirtualMachineSqlExt as _, api::server_config::SSLConfig, - codegen::js_mysql_connection as js, webcore::AutoFlusher, + VirtualMachine, VirtualMachineSqlExt as _, codegen::js_mysql_connection as js, + webcore::AutoFlusher, }; use crate::shared::CachedStructure; -use bun_boringssl_sys as boringssl; +use crate::shared::connection_ctor_args::{self, ConnectionCtorArgs}; use bun_core::strings; use bun_core::{TimespecMockMode, timespec}; use bun_ptr::{AsCtxPtr, BackRef, ParentRef}; @@ -470,75 +470,15 @@ impl JSMySQLConnection { // no other live borrow in this scope. let vm = global_object.bun_vm().as_mut(); let arguments = callframe.arguments(); - let hostname_str = bun_core::OwnedString::new(arguments[0].to_bun_string(global_object)?); - let port = arguments[1].coerce::(global_object)?; - - let username_str = bun_core::OwnedString::new(arguments[2].to_bun_string(global_object)?); - let password_str = bun_core::OwnedString::new(arguments[3].to_bun_string(global_object)?); - let database_str = bun_core::OwnedString::new(arguments[4].to_bun_string(global_object)?); - // TODO: update this to match MySQL. - let ssl_mode: SSLMode = match arguments[5].to_int32() { - 0 => SSLMode::Disable, - 1 => SSLMode::Prefer, - 2 => SSLMode::Require, - 3 => SSLMode::VerifyCa, - 4 => SSLMode::VerifyFull, - _ => SSLMode::Disable, + let Some(args) = ConnectionCtorArgs::::parse(global_object, &mut *vm, arguments)? + else { + return Ok(JSValue::ZERO); }; - - let tls_object = arguments[6]; - - let mut tls_config: SSLConfig = SSLConfig::default(); - let mut secure: Option<*mut uws::SslCtx> = None; - if ssl_mode != SSLMode::Disable { - tls_config = if tls_object.is_boolean() && tls_object.to_boolean() { - SSLConfig::default() - } else if tls_object.is_object() { - match SSLConfig::from_js(&mut *vm, global_object, tls_object) { - Ok(Some(c)) => c, - Ok(None) => SSLConfig::default(), - Err(_) => return Ok(JSValue::ZERO), - } - } else { - return Err(global_object - .throw_invalid_arguments(format_args!("tls must be a boolean or an object"))); - }; - - if global_object.has_exception() { - drop(tls_config); - return Ok(JSValue::ZERO); - } - - // We always request the cert so we can verify it and also we manually - // abort the connection if the hostname doesn't match. Built here so - // CA/cert errors throw synchronously, applied later by upgradeToTLS. - // Goes through the per-VM weak `SSLContextCache` so every pooled - // connection / reconnect shares one `SSL_CTX*` per distinct config. - let mut err = uws::create_bun_socket_error_t::none; - secure = vm - .ssl_ctx_cache() - .get_or_create_opts(&tls_config.as_usockets_for_client_verification(), &mut err); - if secure.is_none() { - drop(tls_config); - return Err( - global_object.throw_value(crate::jsc::create_bun_socket_error_to_js( - err, - global_object, - )), - ); - } - } // Covers `try arguments[7/8].toBunString()` and the null-byte rejection // below. Ownership passes to `MySQLConnection.init` once `Box::new` // succeeds — we null the locals at that point so the connect-fail path // (which `deref()`s the connection) doesn't double-free. - let tls_guard = scopeguard::guard((secure, tls_config), |(s, cfg)| { - if let Some(s) = s { - // SAFETY: secure was created by ssl_ctx_cache; we own one ref until transferred. - unsafe { boringssl::SSL_CTX_free(s) }; - } - drop(cfg); - }); + let tls_guard = connection_ctor_args::guard_tls(args.secure, args.tls_config); let options_str = bun_core::OwnedString::new(arguments[7].to_bun_string(global_object)?); let path_str = bun_core::OwnedString::new(arguments[8].to_bun_string(global_object)?); @@ -546,9 +486,9 @@ impl JSMySQLConnection { // `init` takes `Box<[u8]>` per field (each separately owned), so we // copy each string into its own allocation. `options_buf` becomes an // empty box. - let username: Box<[u8]> = Box::from(username_str.to_utf8_without_ref().slice()); - let password: Box<[u8]> = Box::from(password_str.to_utf8_without_ref().slice()); - let database: Box<[u8]> = Box::from(database_str.to_utf8_without_ref().slice()); + let username: Box<[u8]> = Box::from(args.username_str.to_utf8_without_ref().slice()); + let password: Box<[u8]> = Box::from(args.password_str.to_utf8_without_ref().slice()); + let database: Box<[u8]> = Box::from(args.database_str.to_utf8_without_ref().slice()); let options: Box<[u8]> = Box::from(options_str.to_utf8_without_ref().slice()); let path: Box<[u8]> = Box::from(path_str.to_utf8_without_ref().slice()); let options_buf: Box<[u8]> = Box::default(); @@ -595,7 +535,7 @@ impl JSMySQLConnection { options_buf, tls_config, secure, - ssl_mode, + args.ssl_mode, allow_public_key_retrieval, )), auto_flusher: JsCell::new(AutoFlusher::default()), @@ -616,7 +556,7 @@ impl JSMySQLConnection { let this = ParentRef::from(core::ptr::NonNull::new(ptr).expect("heap::into_raw non-null")); { - let hostname = hostname_str.to_utf8(); + let hostname = args.hostname_str.to_utf8(); // MySQL always opens plain TCP first; STARTTLS adopts into the TLS // group after the SSLRequest exchange. @@ -636,7 +576,7 @@ impl JSMySQLConnection { uws::DispatchKind::Mysql, None, hostname.slice(), - port, + args.port, ptr, false, ) diff --git a/src/sql_jsc/mysql/JSMySQLQuery.rs b/src/sql_jsc/mysql/JSMySQLQuery.rs index efd43032645..bfdb3025ea4 100644 --- a/src/sql_jsc/mysql/JSMySQLQuery.rs +++ b/src/sql_jsc/mysql/JSMySQLQuery.rs @@ -6,6 +6,7 @@ use crate::jsc::{ self as jsc, CallFrame, JSGlobalObject, JSGlobalObjectSqlExt as _, JSValue, JsRef, JsResult, VirtualMachine, VirtualMachineSqlExt as _, }; +use crate::shared::query_ctor_args::QueryCtorArgs; use bun_jsc::JsCell; use bun_ptr::{AsCtxPtr, BackRef, ParentRef}; use bun_sql::mysql::MySQLQueryResult; @@ -95,43 +96,14 @@ impl JSMySQLQuery { global_this: &JSGlobalObject, callframe: &CallFrame, ) -> JsResult { - let arguments = callframe.arguments(); - let mut args = jsc::call_frame::ArgumentsSlice::init(global_this.sql_vm(), arguments); - // defer args.deinit() — handled by Drop - let Some(query) = args.next_eat() else { - return Err(global_this.throw(format_args!("query must be a string"))); - }; - let Some(values) = args.next_eat() else { - return Err(global_this.throw(format_args!("values must be an array"))); - }; - - if !query.is_string() { - return Err(global_this.throw(format_args!("query must be a string"))); - } - - if values.js_type() != jsc::JSType::Array { - return Err(global_this.throw(format_args!("values must be an array"))); - } - - let pending_value: JSValue = args.next_eat().unwrap_or(JSValue::UNDEFINED); - let columns: JSValue = args.next_eat().unwrap_or(JSValue::UNDEFINED); - let js_bigint: JSValue = args.next_eat().unwrap_or(JSValue::FALSE); - let js_simple: JSValue = args.next_eat().unwrap_or(JSValue::FALSE); - - let bigint = js_bigint.is_boolean() && js_bigint.as_boolean(); - let simple = js_simple.is_boolean() && js_simple.as_boolean(); - if simple { - if values.get_length(global_this)? > 0 { - return Err(global_this - .throw_invalid_arguments(format_args!("simple query cannot have parameters"))); - } - if query.get_length(global_this)? >= i32::MAX as u64 { - return Err(global_this.throw_invalid_arguments(format_args!("query is too long"))); - } - } - if !pending_value.js_type().is_array_like() { - return Err(global_this.throw_invalid_argument_type("query", "pendingValue", "Array")); - } + let QueryCtorArgs { + query, + values, + pending_value, + columns, + bigint, + simple, + } = QueryCtorArgs::parse(global_this, callframe.arguments())?; let this_ptr = bun_core::heap::into_raw(Box::new(Self { this_value: JsCell::new(JsRef::empty()), diff --git a/src/sql_jsc/mysql/MySQLStatement.rs b/src/sql_jsc/mysql/MySQLStatement.rs index ea0756d5001..95914708d6c 100644 --- a/src/sql_jsc/mysql/MySQLStatement.rs +++ b/src/sql_jsc/mysql/MySQLStatement.rs @@ -1,15 +1,13 @@ use core::cell::Cell; use crate::jsc::{JSGlobalObject, JSValue}; -use bun_collections::StringHashMap; use crate::mysql::protocol::Signature; use crate::shared::CachedStructure; -use crate::shared::sql_data_cell::Flags as DataCellFlags; +use crate::shared::sql_data_cell::{Flags as DataCellFlags, dedupe_columns}; use bun_sql::mysql::protocol::column_definition41::ColumnDefinition41; use bun_sql::mysql::protocol::error_packet::ErrorPacket; -use bun_sql::shared::ColumnIdentifier; pub use bun_sql::mysql::mysql_param::Param; @@ -88,13 +86,7 @@ impl Default for ExecutionFlags { } } -#[derive(Clone, Copy, PartialEq, Eq)] -pub enum Status { - Pending, - Parsing, - Prepared, - Failed, -} +pub use bun_sql::shared::statement_status::Status; impl MySQLStatement { /// Set the initial intrusive @@ -124,49 +116,8 @@ impl MySQLStatement { self.execution_flags .remove(ExecutionFlags::NEEDS_DUPLICATE_CHECK); - let mut seen_numbers: Vec = Vec::new(); - let mut seen_fields: StringHashMap<()> = StringHashMap::default(); - seen_fields.reserve(self.columns.len()); - - // iterate backwards - let mut remaining = self.columns.len(); - let mut flags = DataCellFlags::default(); - while remaining > 0 { - remaining -= 1; - let field: &mut ColumnDefinition41 = &mut self.columns[remaining]; - match &field.name_or_index { - ColumnIdentifier::Name(name) => { - // reshaped for borrowck — compute `found_existing` before - // mutating `field.name_or_index`. - let found_existing = seen_fields - .get_or_put(name.slice()) - .expect("OOM") - .found_existing; - if found_existing { - field.name_or_index = ColumnIdentifier::Duplicate; - flags.insert(DataCellFlags::HAS_DUPLICATE_COLUMNS); - } - - flags.insert(DataCellFlags::HAS_NAMED_COLUMNS); - } - ColumnIdentifier::Index(index) => { - let index = *index; - if seen_numbers.contains(&index) { - field.name_or_index = ColumnIdentifier::Duplicate; - flags.insert(DataCellFlags::HAS_DUPLICATE_COLUMNS); - } else { - seen_numbers.push(index); - } - - flags.insert(DataCellFlags::HAS_INDEXED_COLUMNS); - } - ColumnIdentifier::Duplicate => { - flags.insert(DataCellFlags::HAS_DUPLICATE_COLUMNS); - } - } - } - - self.fields_flags = flags; + self.fields_flags = + dedupe_columns(self.columns.iter_mut().rev().map(|c| &mut c.name_or_index)); } // Returning `&CachedStructure` diff --git a/src/sql_jsc/postgres.rs b/src/sql_jsc/postgres.rs index b3ef8eec805..eb20c6d831d 100644 --- a/src/sql_jsc/postgres.rs +++ b/src/sql_jsc/postgres.rs @@ -85,9 +85,6 @@ pub mod types { pub mod protocol { #[path = "error_response_jsc.rs"] pub mod error_response_jsc; - - #[path = "notice_response_jsc.rs"] - pub mod notice_response_jsc; } // Re-exports of base-crate protocol/types modules. diff --git a/src/sql_jsc/postgres/PostgresSQLConnection.rs b/src/sql_jsc/postgres/PostgresSQLConnection.rs index eee26fd2279..dfe570124b4 100644 --- a/src/sql_jsc/postgres/PostgresSQLConnection.rs +++ b/src/sql_jsc/postgres/PostgresSQLConnection.rs @@ -31,6 +31,7 @@ use crate::postgres::postgres_sql_query::{self, Status as QueryStatus}; use crate::postgres::postgres_sql_statement::{Error as StatementError, Status as StatementStatus}; use crate::postgres::sasl::SASLStatus; use crate::shared::CachedStructure as PostgresCachedStructure; +use crate::shared::connection_ctor_args::{self, ConnectionCtorArgs}; use bun_sql::postgres::AnyPostgresError; use bun_sql::postgres::PostgresErrorOptions; use bun_sql::postgres::PostgresProtocol as protocol; @@ -1078,77 +1079,15 @@ pub(crate) fn call(global_object: &JSGlobalObject, callframe: &CallFrame) -> JsR // `&mut self` helpers like `ssl_ctx_cache()` / `postgres_socket_group()`. let vm = global_object.bun_vm().as_mut(); let arguments = callframe.arguments(); - let hostname_str = bun_core::OwnedString::new(arguments[0].to_bun_string(global_object)?); - let port = arguments[1].coerce::(global_object)?; - - let username_str = bun_core::OwnedString::new(arguments[2].to_bun_string(global_object)?); - let password_str = bun_core::OwnedString::new(arguments[3].to_bun_string(global_object)?); - let database_str = bun_core::OwnedString::new(arguments[4].to_bun_string(global_object)?); - let ssl_mode: SSLMode = match arguments[5].to_int32() { - 0 => SSLMode::Disable, - 1 => SSLMode::Prefer, - 2 => SSLMode::Require, - 3 => SSLMode::VerifyCa, - 4 => SSLMode::VerifyFull, - _ => SSLMode::Disable, + let Some(args) = ConnectionCtorArgs::::parse(global_object, &mut *vm, arguments)? + else { + return Ok(JSValue::ZERO); }; - - let tls_object = arguments[6]; - - let mut tls_config: jsc::api::ServerConfig::SSLConfig = Default::default(); - let mut secure: Option<*mut uws::SslCtx> = None; - if ssl_mode != SSLMode::Disable { - tls_config = if tls_object.is_boolean() && tls_object.to_boolean() { - Default::default() - } else if tls_object.is_object() { - match jsc::api::ServerConfig::SSLConfig::from_js(&mut *vm, global_object, tls_object) { - Ok(opt) => opt.unwrap_or_default(), - Err(_) => return Ok(JSValue::ZERO), - } - } else { - return Err(global_object - .throw_invalid_arguments(format_args!("tls must be a boolean or an object"))); - }; - - if global_object.has_exception() { - drop(tls_config); - return Ok(JSValue::ZERO); - } - - // We always request the cert so we can verify it and also we manually - // abort the connection if the hostname doesn't match. Built here (not - // at STARTTLS time) so cert/CA errors throw synchronously. Goes - // through the per-VM weak `SSLContextCache` so every connection in the - // pool — and every reconnect — shares one `SSL_CTX*` per distinct - // config instead of building a fresh one per `PostgresSQLConnection`. - let mut err: uws::create_bun_socket_error_t = uws::create_bun_socket_error_t::none; - secure = vm - .ssl_ctx_cache() - .get_or_create_opts(&tls_config.as_usockets_for_client_verification(), &mut err); - if secure.is_none() { - drop(tls_config); - return Err( - global_object.throw_value(crate::jsc::create_bun_socket_error_to_js( - err, - global_object, - )), - ); - } - } // Covers `try arguments[7/8].toBunString()` and the null-byte rejection // below. Ownership passes into `ptr.*` once allocated — `into_inner` // recovers them just before the Box is built so the connect-fail path's // `ptr.deinit()` is the sole cleanup. - // guard owns `(secure, tls_config)` by value. Do NOT - // `drop_in_place` a stack local that Rust would also auto-drop on unwind — - // that double-frees. The closure's `_tls_config` is dropped exactly once by - // normal scope-exit drop here. - let errdefer_guard = scopeguard::guard((secure, tls_config), |(secure, _tls_config)| { - if let Some(s) = secure { - // SAFETY: SSL_CTX_free is safe to call on a valid SSL_CTX*. - unsafe { BoringSSL::c::SSL_CTX_free(s) }; - } - }); + let errdefer_guard = connection_ctor_args::guard_tls(args.secure, args.tls_config); // `StringBuilder::append` takes `&mut self` and returns a borrow // of the backing buffer, so successive appends can't keep their `&[u8]` @@ -1168,11 +1107,11 @@ pub(crate) fn call(global_object: &JSGlobalObject, callframe: &CallFrame) -> JsR let options_buf: Box<[u8]> = 'brk: { let mut b = bun_core::StringBuilder::default(); - b.cap += username_str.utf8_byte_length() + b.cap += args.username_str.utf8_byte_length() + 1 - + password_str.utf8_byte_length() + + args.password_str.utf8_byte_length() + 1 - + database_str.utf8_byte_length() + + args.database_str.utf8_byte_length() + 1 + options_str.utf8_byte_length() + 1 @@ -1180,15 +1119,15 @@ pub(crate) fn call(global_object: &JSGlobalObject, callframe: &CallFrame) -> JsR + 1; let _ = b.allocate(); - let u = username_str.to_utf8_without_ref(); + let u = args.username_str.to_utf8_without_ref(); username = bun_ptr::RawSlice::new(b.append(u.slice())); drop(u); - let p = password_str.to_utf8_without_ref(); + let p = args.password_str.to_utf8_without_ref(); password = bun_ptr::RawSlice::new(b.append(p.slice())); drop(p); - let d = database_str.to_utf8_without_ref(); + let d = args.database_str.to_utf8_without_ref(); database = bun_ptr::RawSlice::new(b.append(d.slice())); drop(d); @@ -1269,12 +1208,12 @@ pub(crate) fn call(global_object: &JSGlobalObject, callframe: &CallFrame) -> JsR authentication_state: JsCell::new(AuthenticationState::Pending), secure, tls_config, - tls_status: Cell::new(if ssl_mode != SSLMode::Disable { + tls_status: Cell::new(if args.ssl_mode != SSLMode::Disable { TLSStatus::Pending } else { TLSStatus::None }), - ssl_mode, + ssl_mode: args.ssl_mode, idle_timeout_interval_ms: u32::try_from(idle_timeout).expect("int cast"), connection_timeout_ms: u32::try_from(connection_timeout).expect("int cast"), flags: Cell::new(if use_unnamed_prepared_statements { @@ -1298,7 +1237,7 @@ pub(crate) fn call(global_object: &JSGlobalObject, callframe: &CallFrame) -> JsR let this = ParentRef::from(core::ptr::NonNull::new(ptr).expect("heap::into_raw non-null")); { - let hostname = hostname_str.to_utf8(); + let hostname = args.hostname_str.to_utf8(); // Postgres always opens plain TCP first (SSLRequest happens in-band), // so even `ssl_mode != .disable` lands in the TCP group; `setupTLS()` @@ -1320,7 +1259,7 @@ pub(crate) fn call(global_object: &JSGlobalObject, callframe: &CallFrame) -> JsR uws::SocketKind::Postgres, None, hostname.slice(), - port, + args.port, ptr, false, ) @@ -3013,7 +2952,7 @@ impl PostgresSQLConnection { } MessageType::NoticeResponse => { debug!("UNSUPPORTED NoticeResponse"); - let _resp = protocol::NoticeResponse::decode_internal(reader.reborrow())?; + let _resp = protocol::NoticeResponse::decode_notice_internal(reader.reborrow())?; // _resp dropped at scope end } MessageType::NotificationResponse => { diff --git a/src/sql_jsc/postgres/PostgresSQLQuery.rs b/src/sql_jsc/postgres/PostgresSQLQuery.rs index ac655840ca9..c2c3b45768b 100644 --- a/src/sql_jsc/postgres/PostgresSQLQuery.rs +++ b/src/sql_jsc/postgres/PostgresSQLQuery.rs @@ -4,6 +4,7 @@ use core::mem; use crate::jsc::{ CallFrame, JSGlobalObject, JSValue, JsError, JsRef, JsResult, VirtualMachineSqlExt as _, }; +use crate::shared::query_ctor_args::QueryCtorArgs; use bun_core::String as BunString; use bun_jsc::JsCell; use bun_ptr::AsCtxPtr; @@ -107,28 +108,7 @@ impl Default for Flags { } } -#[repr(u8)] -#[derive(Clone, Copy, PartialEq, Eq)] -pub enum Status { - /// The query was just enqueued, statement status can be checked for more details - Pending, - /// The query is being bound to the statement - Binding, - /// The query is running - Running, - /// The query is waiting for a partial response - PartialResponse, - /// The query was successful - Success, - /// The query failed - Fail, -} - -impl Status { - pub fn is_running(self) -> bool { - (self as u8) > (Status::Pending as u8) && (self as u8) < (Status::Success as u8) - } -} +pub use bun_sql::shared::query_status::Status; impl PostgresSQLQuery { // `ref_()`/`deref()` provided by `#[derive(CellRefCounted)]`. @@ -382,44 +362,14 @@ impl PostgresSQLQuery { // Registered directly as `createQuery` via // `put_host_functions!` in `postgres.rs`, so no exported symbol is needed. pub fn call(global_this: &JSGlobalObject, callframe: &CallFrame) -> JsResult { - let arguments = callframe.arguments(); - let mut args = - crate::jsc::call_frame::ArgumentsSlice::init(global_this.bun_vm(), arguments); - // ArgumentsSlice has Drop. - let Some(query) = args.next_eat() else { - return Err(global_this.throw(format_args!("query must be a string"))); - }; - let Some(values) = args.next_eat() else { - return Err(global_this.throw(format_args!("values must be an array"))); - }; - - if !query.is_string() { - return Err(global_this.throw(format_args!("query must be a string"))); - } - - if values.js_type() != crate::jsc::JSType::Array { - return Err(global_this.throw(format_args!("values must be an array"))); - } - - let pending_value: JSValue = args.next_eat().unwrap_or(JSValue::UNDEFINED); - let columns: JSValue = args.next_eat().unwrap_or(JSValue::UNDEFINED); - let js_bigint: JSValue = args.next_eat().unwrap_or(JSValue::FALSE); - let js_simple: JSValue = args.next_eat().unwrap_or(JSValue::FALSE); - - let bigint = js_bigint.is_boolean() && js_bigint.as_boolean(); - let simple = js_simple.is_boolean() && js_simple.as_boolean(); - if simple { - if values.get_length(global_this)? > 0 { - return Err(global_this - .throw_invalid_arguments(format_args!("simple query cannot have parameters"))); - } - if query.get_length(global_this)? >= i32::MAX as u64 { - return Err(global_this.throw_invalid_arguments(format_args!("query is too long"))); - } - } - if !pending_value.js_type().is_array_like() { - return Err(global_this.throw_invalid_argument_type("query", "pendingValue", "Array")); - } + let QueryCtorArgs { + query, + values, + pending_value, + columns, + bigint, + simple, + } = QueryCtorArgs::parse(global_this, callframe.arguments())?; let ptr = bun_core::heap::into_raw(Box::new(PostgresSQLQuery::default())); diff --git a/src/sql_jsc/postgres/PostgresSQLStatement.rs b/src/sql_jsc/postgres/PostgresSQLStatement.rs index 3dafeec8245..a2c4647baba 100644 --- a/src/sql_jsc/postgres/PostgresSQLStatement.rs +++ b/src/sql_jsc/postgres/PostgresSQLStatement.rs @@ -1,17 +1,15 @@ use core::cell::Cell; use crate::jsc::{JSGlobalObject, JSValue, JsResult}; -use bun_collections::StringHashMap; use crate::postgres::error_jsc::postgres_error_to_js; use crate::postgres::signature::Signature; use crate::shared::cached_structure::CachedStructure as PostgresCachedStructure; -use crate::shared::sql_data_cell::Flags as DataCellFlags; +use crate::shared::sql_data_cell::{Flags as DataCellFlags, dedupe_columns}; use bun_sql::postgres::any_postgres_error::AnyPostgresError; use bun_sql::postgres::postgres_protocol as protocol; use bun_sql::postgres::postgres_types::int4; -use bun_sql::shared::ColumnIdentifier; bun_core::declare_scope!(Postgres, visible); @@ -71,19 +69,7 @@ impl Error { } } -#[derive(Copy, Clone, Eq, PartialEq)] -pub enum Status { - Pending, - Parsing, - Prepared, - Failed, -} - -impl Status { - pub fn is_running(self) -> bool { - self == Status::Parsing - } -} +pub use bun_sql::shared::statement_status::Status; impl PostgresSQLStatement { /// Set the initial intrusive @@ -103,52 +89,8 @@ impl PostgresSQLStatement { } self.needs_duplicate_check = false; - let mut seen_numbers: Vec = Vec::new(); - let mut seen_fields: StringHashMap<()> = StringHashMap::default(); - seen_fields.reserve(self.fields.len()); - - // iterate backwards - let mut remaining = self.fields.len(); - let mut flags = DataCellFlags::default(); - while remaining > 0 { - remaining -= 1; - let field: &mut protocol::FieldDescription = &mut self.fields[remaining]; - match &field.name_or_index { - ColumnIdentifier::Name(name) => { - // Note: reshaped for borrowck — compute `found_existing` - // before mutating `field.name_or_index`. - // StringHashMap - // clones to an owned `Box<[u8]>` key. Fine for a transient - // dedup set. - let found_existing = seen_fields - .get_or_put(name.slice()) - .expect("OOM") - .found_existing; - if found_existing { - field.name_or_index = ColumnIdentifier::Duplicate; - flags.insert(DataCellFlags::HAS_DUPLICATE_COLUMNS); - } - - flags.insert(DataCellFlags::HAS_NAMED_COLUMNS); - } - ColumnIdentifier::Index(index) => { - let index = *index; - if seen_numbers.contains(&index) { - field.name_or_index = ColumnIdentifier::Duplicate; - flags.insert(DataCellFlags::HAS_DUPLICATE_COLUMNS); - } else { - seen_numbers.push(index); - } - - flags.insert(DataCellFlags::HAS_INDEXED_COLUMNS); - } - ColumnIdentifier::Duplicate => { - flags.insert(DataCellFlags::HAS_DUPLICATE_COLUMNS); - } - } - } - - self.fields_flags = flags; + self.fields_flags = + dedupe_columns(self.fields.iter_mut().rev().map(|f| &mut f.name_or_index)); } // Note: returning diff --git a/src/sql_jsc/postgres/protocol/error_response_jsc.rs b/src/sql_jsc/postgres/protocol/error_response_jsc.rs index bedf0fd59e6..2813335ed89 100644 --- a/src/sql_jsc/postgres/protocol/error_response_jsc.rs +++ b/src/sql_jsc/postgres/protocol/error_response_jsc.rs @@ -7,15 +7,11 @@ use bun_sql::postgres::protocol::field_message::FieldMessage; use crate::postgres::error_jsc::create_postgres_error; use bun_sql::postgres::any_postgres_error::PostgresErrorOptions; -use super::notice_response_jsc::field_message_payload; - pub(crate) fn to_js(this: &ErrorResponse, global_object: &JSGlobalObject) -> JSValue { let mut b = StringBuilder::default(); for msg in this.messages.iter() { - // Every - // FieldMessage variant carries a single bun.String payload. - b.cap += field_message_payload(msg).utf8_byte_length() + 1; + b.cap += msg.payload().utf8_byte_length() + 1; } let _ = b.allocate(); diff --git a/src/sql_jsc/postgres/protocol/notice_response_jsc.rs b/src/sql_jsc/postgres/protocol/notice_response_jsc.rs deleted file mode 100644 index 6aa8cbf31e3..00000000000 --- a/src/sql_jsc/postgres/protocol/notice_response_jsc.rs +++ /dev/null @@ -1,28 +0,0 @@ -use bun_sql::postgres::protocol::field_message::FieldMessage; - -/// Every `FieldMessage` variant -/// carries a single `bun.String` payload, so an exhaustive match collapses to -/// the single binding. The match lives here (not as a `payload()` accessor on -/// `bun_sql::FieldMessage`) because this is its only consumer. -pub(crate) fn field_message_payload(msg: &FieldMessage) -> &bun_core::String { - match msg { - FieldMessage::Severity(s) - | FieldMessage::LocalizedSeverity(s) - | FieldMessage::Code(s) - | FieldMessage::Message(s) - | FieldMessage::Detail(s) - | FieldMessage::Hint(s) - | FieldMessage::Position(s) - | FieldMessage::InternalPosition(s) - | FieldMessage::Internal(s) - | FieldMessage::Where(s) - | FieldMessage::Schema(s) - | FieldMessage::Table(s) - | FieldMessage::Column(s) - | FieldMessage::Datatype(s) - | FieldMessage::Constraint(s) - | FieldMessage::File(s) - | FieldMessage::Line(s) - | FieldMessage::Routine(s) => s, - } -} diff --git a/src/sql_jsc/shared/SQLDataCell.rs b/src/sql_jsc/shared/SQLDataCell.rs index dd84b710485..225f3244ec0 100644 --- a/src/sql_jsc/shared/SQLDataCell.rs +++ b/src/sql_jsc/shared/SQLDataCell.rs @@ -2,8 +2,10 @@ use core::ptr; use core::slice; use crate::jsc::{ExternColumnIdentifier, JSGlobalObject, JSType, JSValue, JsError, JsResult}; +use bun_collections::StringHashMap; +use bun_core::UnwrapOrOom as _; use bun_core::wtf::WTFStringImpl; -use bun_sql::shared::Data; +use bun_sql::shared::{ColumnIdentifier, Data}; // Note: This entire type is passed by pointer // across FFI to C++ (`JSC__constructObjectFromDataCell`). Field layout is @@ -381,6 +383,55 @@ bitflags::bitflags! { } } +/// Rewrites repeated column identifiers to [`ColumnIdentifier::Duplicate`] and +/// accumulates the column-set [`Flags`]. Callers pass the columns in reverse +/// order so the LAST occurrence of a repeated name/index keeps its identifier. +pub fn dedupe_columns<'a>( + columns: impl ExactSizeIterator, +) -> Flags { + let mut seen_numbers: Vec = Vec::new(); + // StringHashMap clones to an owned `Box<[u8]>` key. Fine for a transient + // dedup set. + let mut seen_fields: StringHashMap<()> = StringHashMap::default(); + seen_fields.reserve(columns.len()); + + let mut flags = Flags::default(); + for name_or_index in columns { + match &*name_or_index { + ColumnIdentifier::Name(name) => { + // reshaped for borrowck — compute `found_existing` before + // mutating `*name_or_index`. + let found_existing = seen_fields + .get_or_put(name.slice()) + .unwrap_or_oom() + .found_existing; + if found_existing { + *name_or_index = ColumnIdentifier::Duplicate; + flags.insert(Flags::HAS_DUPLICATE_COLUMNS); + } + + flags.insert(Flags::HAS_NAMED_COLUMNS); + } + ColumnIdentifier::Index(index) => { + let index = *index; + if seen_numbers.contains(&index) { + *name_or_index = ColumnIdentifier::Duplicate; + flags.insert(Flags::HAS_DUPLICATE_COLUMNS); + } else { + seen_numbers.push(index); + } + + flags.insert(Flags::HAS_INDEXED_COLUMNS); + } + ColumnIdentifier::Duplicate => { + flags.insert(Flags::HAS_DUPLICATE_COLUMNS); + } + } + } + + flags +} + // Declared inline rather than in a dedicated `*_sys` crate: this is the only // extern this crate calls and its sole consumer is the wrapper above. unsafe extern "C" { diff --git a/src/sql_jsc/shared/connection_ctor_args.rs b/src/sql_jsc/shared/connection_ctor_args.rs new file mode 100644 index 00000000000..248234002cc --- /dev/null +++ b/src/sql_jsc/shared/connection_ctor_args.rs @@ -0,0 +1,136 @@ +//! Shared connection-constructor prologue for the Postgres and MySQL +//! `createConnection(hostname, port, username, password, database, sslMode, +//! tls, ...)` host functions, through the per-VM `SSL_CTX*` cache lookup. + +use crate::jsc::{ + JSGlobalObject, JSValue, JsResult, VirtualMachine, VirtualMachineSqlExt as _, + api::server_config::SSLConfig, +}; +use bun_uws as uws; + +pub(crate) trait SslModeArg: Copy + PartialEq { + /// Wire order of the JS-side enum; index 0 is `Disable`. + const MODES: [Self; 5]; +} + +macro_rules! impl_ssl_mode_arg { + ($ty:ty) => { + impl SslModeArg for $ty { + const MODES: [Self; 5] = [ + Self::Disable, + Self::Prefer, + Self::Require, + Self::VerifyCa, + Self::VerifyFull, + ]; + } + }; +} +// Both drivers use the same five postgres-shaped modes: the JS side +// (`normalizeSSLMode` in src/js/internal/sql/shared.ts) normalizes each +// driver's accepted ssl-mode spellings to this one wire enum, so MySQL's +// native ssl-mode vocabulary never crosses this boundary. +impl_ssl_mode_arg!(bun_sql::mysql::ssl_mode::SSLMode); +impl_ssl_mode_arg!(bun_sql::postgres::SSLMode); + +type GuardState = (Option<*mut uws::SslCtx>, SSLConfig); +pub(crate) type TlsGuard = scopeguard::ScopeGuard; + +/// Errdefer over `(secure, tls_config)`: frees the cached `SSL_CTX*` +/// reference and drops the config unless disarmed via +/// `ScopeGuard::into_inner` once ownership transfers into the connection. +pub(crate) fn guard_tls(secure: Option<*mut uws::SslCtx>, tls_config: SSLConfig) -> TlsGuard { + fn free((secure, _tls_config): GuardState) { + if let Some(s) = secure { + // SAFETY: `secure` holds one `ssl_ctx_cache` reference owned by the caller. + unsafe { bun_boringssl_sys::SSL_CTX_free(s) }; + } + } + scopeguard::guard((secure, tls_config), free as fn(GuardState)) +} + +pub(crate) struct ConnectionCtorArgs { + pub hostname_str: bun_core::OwnedString, + pub port: i32, + pub username_str: bun_core::OwnedString, + pub password_str: bun_core::OwnedString, + pub database_str: bun_core::OwnedString, + pub ssl_mode: M, + pub tls_config: SSLConfig, + /// `SSL_CTX*` holding one reference the caller must release on every + /// early exit (via [`guard_tls`]) until it transfers into the connection. + pub secure: Option<*mut uws::SslCtx>, +} + +impl ConnectionCtorArgs { + /// Parses `arguments[0..=6]`. Returns `Ok(None)` when a JS exception is + /// already pending and the caller should `return Ok(JSValue::ZERO)`. + pub(crate) fn parse( + global_object: &JSGlobalObject, + vm: &mut VirtualMachine, + arguments: &[JSValue], + ) -> JsResult> { + let hostname_str = bun_core::OwnedString::new(arguments[0].to_bun_string(global_object)?); + let port = arguments[1].coerce::(global_object)?; + let username_str = bun_core::OwnedString::new(arguments[2].to_bun_string(global_object)?); + let password_str = bun_core::OwnedString::new(arguments[3].to_bun_string(global_object)?); + let database_str = bun_core::OwnedString::new(arguments[4].to_bun_string(global_object)?); + let modes = M::MODES; + let ssl_mode = usize::try_from(arguments[5].to_int32()) + .ok() + .and_then(|i| modes.get(i)) + .copied() + .unwrap_or(modes[0]); + + let tls_object = arguments[6]; + let mut tls_config = SSLConfig::default(); + let mut secure: Option<*mut uws::SslCtx> = None; + if ssl_mode != modes[0] { + tls_config = if tls_object.is_boolean() && tls_object.to_boolean() { + SSLConfig::default() + } else if tls_object.is_object() { + match SSLConfig::from_js(&mut *vm, global_object, tls_object) { + Ok(opt) => opt.unwrap_or_default(), + Err(_) => return Ok(None), + } + } else { + return Err(global_object + .throw_invalid_arguments(format_args!("tls must be a boolean or an object"))); + }; + + if global_object.has_exception() { + return Ok(None); + } + + // We always request the cert so we can verify it and manually + // abort if the hostname doesn't match. Built here (not at STARTTLS + // time) so cert/CA errors throw synchronously; the per-VM weak + // `SSLContextCache` shares one `SSL_CTX*` per distinct config + // across pooled connections and reconnects. + let mut err = uws::create_bun_socket_error_t::none; + secure = vm + .ssl_ctx_cache() + .get_or_create_opts(&tls_config.as_usockets_for_client_verification(), &mut err); + if secure.is_none() { + drop(tls_config); + return Err( + global_object.throw_value(crate::jsc::create_bun_socket_error_to_js( + err, + global_object, + )), + ); + } + } + + Ok(Some(Self { + hostname_str, + port, + username_str, + password_str, + database_str, + ssl_mode, + tls_config, + secure, + })) + } +} diff --git a/src/sql_jsc/shared/query_ctor_args.rs b/src/sql_jsc/shared/query_ctor_args.rs new file mode 100644 index 00000000000..15378826b99 --- /dev/null +++ b/src/sql_jsc/shared/query_ctor_args.rs @@ -0,0 +1,64 @@ +//! Shared `createQuery(query, values, pendingValue?, columns?, bigint?, +//! simple?)` constructor-argument parsing/validation used by both the +//! Postgres and MySQL query constructors. + +use crate::jsc::{JSGlobalObject, JSGlobalObjectSqlExt as _, JSType, JSValue, JsResult}; + +pub(crate) struct QueryCtorArgs { + pub query: JSValue, + pub values: JSValue, + pub pending_value: JSValue, + pub columns: JSValue, + pub bigint: bool, + pub simple: bool, +} + +impl QueryCtorArgs { + pub(crate) fn parse(global_this: &JSGlobalObject, arguments: &[JSValue]) -> JsResult { + let mut args = + crate::jsc::call_frame::ArgumentsSlice::init(global_this.sql_vm(), arguments); + let Some(query) = args.next_eat() else { + return Err(global_this.throw(format_args!("query must be a string"))); + }; + let Some(values) = args.next_eat() else { + return Err(global_this.throw(format_args!("values must be an array"))); + }; + + if !query.is_string() { + return Err(global_this.throw(format_args!("query must be a string"))); + } + + if values.js_type() != JSType::Array { + return Err(global_this.throw(format_args!("values must be an array"))); + } + + let pending_value: JSValue = args.next_eat().unwrap_or(JSValue::UNDEFINED); + let columns: JSValue = args.next_eat().unwrap_or(JSValue::UNDEFINED); + let js_bigint: JSValue = args.next_eat().unwrap_or(JSValue::FALSE); + let js_simple: JSValue = args.next_eat().unwrap_or(JSValue::FALSE); + + let bigint = js_bigint.is_boolean() && js_bigint.as_boolean(); + let simple = js_simple.is_boolean() && js_simple.as_boolean(); + if simple { + if values.get_length(global_this)? > 0 { + return Err(global_this + .throw_invalid_arguments(format_args!("simple query cannot have parameters"))); + } + if query.get_length(global_this)? >= i32::MAX as u64 { + return Err(global_this.throw_invalid_arguments(format_args!("query is too long"))); + } + } + if !pending_value.js_type().is_array_like() { + return Err(global_this.throw_invalid_argument_type("query", "pendingValue", "Array")); + } + + Ok(Self { + query, + values, + pending_value, + columns, + bigint, + simple, + }) + } +} From 159cea498610a8d057c03bc366de125510d5f4b1 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Thu, 11 Jun 2026 11:52:21 -0700 Subject: [PATCH 3/4] sql: cover NoticeResponse framing in the multi-statement mock-server test --- .../postgres-multi-statement-fields.test.ts | 29 +++++++++++++++++-- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/test/js/sql/postgres-multi-statement-fields.test.ts b/test/js/sql/postgres-multi-statement-fields.test.ts index 40cd4f5f097..ff1223525d5 100644 --- a/test/js/sql/postgres-multi-statement-fields.test.ts +++ b/test/js/sql/postgres-multi-statement-fields.test.ts @@ -111,11 +111,34 @@ test("simple query with multiple statements uses each RowDescription's column na } }); -// NotificationResponse ('A', sent by NOTIFY) and unknown async messages can arrive -// between result sets. The protocol reader must consume exactly the message body so -// the following messages stay correctly framed. +// NotificationResponse ('A', sent by NOTIFY), NoticeResponse ('N', sent by +// RAISE NOTICE and server chatter like "relation exists, skipping") and unknown +// async messages can arrive between result sets. The protocol reader must +// consume exactly the message body so the following messages stay correctly +// framed. for (const [name, asyncMessage] of [ ["NotificationResponse", pkt("A", Buffer.concat([int32(4321), cstr("some_channel"), cstr("some payload")]))], + // NoticeResponse shares ErrorResponse's field-list format: repeated + // (field-type byte + cstring), closed by a single zero byte. It must be + // decoded and discarded without failing the query. + [ + "NoticeResponse", + pkt( + "N", + Buffer.concat([ + Buffer.from("S"), + cstr("NOTICE"), + Buffer.from("C"), + cstr("00000"), + Buffer.from("M"), + cstr("relation exists, skipping"), + Buffer.from([0]), + ]), + ), + ], + // Degenerate notice with declared length 4 and no field list at all; unlike + // ErrorResponse, notice decoding treats this as an empty notice. + ["empty NoticeResponse", pkt("N", Buffer.alloc(0))], // 'v' = NegotiateProtocolVersion, which the client does not handle explicitly ["unknown message type", pkt("v", Buffer.concat([int32(0), int32(0)]))], ] as const) { From 4507f95d664ad7f7d0b99a190a393f3a39858762 Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Thu, 11 Jun 2026 12:09:51 -0700 Subject: [PATCH 4/4] sql: address review feedback on #32135 - rename ctor_args files to PascalCase per the documented sql_jsc convention - drop StatementStatus::is_running() (zero callers; sites already compare == Parsing) - drop misleading "unlike ErrorResponse" clause from empty-notice test comment --- src/sql/shared/StatementStatus.rs | 6 ------ src/sql_jsc/lib.rs | 2 ++ .../{connection_ctor_args.rs => ConnectionCtorArgs.rs} | 0 src/sql_jsc/shared/{query_ctor_args.rs => QueryCtorArgs.rs} | 0 test/js/sql/postgres-multi-statement-fields.test.ts | 3 +-- 5 files changed, 3 insertions(+), 8 deletions(-) rename src/sql_jsc/shared/{connection_ctor_args.rs => ConnectionCtorArgs.rs} (100%) rename src/sql_jsc/shared/{query_ctor_args.rs => QueryCtorArgs.rs} (100%) diff --git a/src/sql/shared/StatementStatus.rs b/src/sql/shared/StatementStatus.rs index bce4a693f31..870d6382248 100644 --- a/src/sql/shared/StatementStatus.rs +++ b/src/sql/shared/StatementStatus.rs @@ -5,9 +5,3 @@ pub enum Status { Prepared, Failed, } - -impl Status { - pub fn is_running(self) -> bool { - self == Status::Parsing - } -} diff --git a/src/sql_jsc/lib.rs b/src/sql_jsc/lib.rs index 4a47a8066a8..78f00137373 100644 --- a/src/sql_jsc/lib.rs +++ b/src/sql_jsc/lib.rs @@ -20,6 +20,7 @@ pub mod shared { #[path = "CachedStructure.rs"] pub mod cached_structure; + #[path = "ConnectionCtorArgs.rs"] pub mod connection_ctor_args; pub mod datetime_text; @@ -30,6 +31,7 @@ pub mod shared { #[path = "QueryBindingIterator.rs"] pub mod query_binding_iterator; + #[path = "QueryCtorArgs.rs"] pub mod query_ctor_args; #[path = "SQLDataCell.rs"] diff --git a/src/sql_jsc/shared/connection_ctor_args.rs b/src/sql_jsc/shared/ConnectionCtorArgs.rs similarity index 100% rename from src/sql_jsc/shared/connection_ctor_args.rs rename to src/sql_jsc/shared/ConnectionCtorArgs.rs diff --git a/src/sql_jsc/shared/query_ctor_args.rs b/src/sql_jsc/shared/QueryCtorArgs.rs similarity index 100% rename from src/sql_jsc/shared/query_ctor_args.rs rename to src/sql_jsc/shared/QueryCtorArgs.rs diff --git a/test/js/sql/postgres-multi-statement-fields.test.ts b/test/js/sql/postgres-multi-statement-fields.test.ts index ff1223525d5..f73019b0e7b 100644 --- a/test/js/sql/postgres-multi-statement-fields.test.ts +++ b/test/js/sql/postgres-multi-statement-fields.test.ts @@ -136,8 +136,7 @@ for (const [name, asyncMessage] of [ ]), ), ], - // Degenerate notice with declared length 4 and no field list at all; unlike - // ErrorResponse, notice decoding treats this as an empty notice. + // Degenerate notice: declared length 4, no field list at all. ["empty NoticeResponse", pkt("N", Buffer.alloc(0))], // 'v' = NegotiateProtocolVersion, which the client does not handle explicitly ["unknown message type", pkt("v", Buffer.concat([int32(0), int32(0)]))],