From 821774aa06dd6cc0d60f1dd32b067b2bafbeb7e5 Mon Sep 17 00:00:00 2001 From: Anatoly Ikorsky Date: Thu, 27 Apr 2023 17:25:19 +0300 Subject: [PATCH 01/13] Update dependencies --- Cargo.toml | 31 ++++++++++++--------- src/conn/mod.rs | 72 ++++++++++++++++++++++++------------------------ src/error/mod.rs | 4 +++ src/lib.rs | 2 +- 4 files changed, 59 insertions(+), 50 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 7500598..060a580 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,23 +32,27 @@ default = [ "flate2/zlib", # set of enabled-by-default mysql_common features - "mysql_common/bigdecimal03", + "mysql_common/bigdecimal", "mysql_common/rust_decimal", - "mysql_common/time03", - "mysql_common/uuid", + "mysql_common/time", "mysql_common/frunk", + "mysql_common/derive", # use global buffer pool by default "buffer-pool", ] default-rustls = [ "rustls-tls", - "flate2/zlib", - "mysql_common/bigdecimal03", + + # default-rustls uses rust_backend for flate2. + "flate2/rust_backend", + + "mysql_common/bigdecimal", "mysql_common/rust_decimal", - "mysql_common/time03", - "mysql_common/uuid", + "mysql_common/time", "mysql_common/frunk", + "mysql_common/derive", + "buffer-pool", ] minimal = ["flate2/zlib"] @@ -69,11 +73,11 @@ bytes = "1.0.1" crossbeam = "0.8.1" io-enum = "1.0.0" flate2 = { version = "1.0", default-features = false } -lru = "0.8.1" -mysql_common = { version = "0.29.2", default-features = false } -socket2 = "0.4" +lru = "0.10" +mysql_common = { version = "0.30", default-features = false } +socket2 = "0.5.2" once_cell = "1.7.2" -pem = "1.0.1" +pem = "2.0.1" percent-encoding = "2.1.0" serde = "1" serde_json = "1" @@ -85,7 +89,7 @@ version = "0.2.3" optional = true [dependencies.rustls] -version = "0.20.0" +version = "0.21.0" features = ["dangerous_configuration"] optional = true @@ -95,10 +99,11 @@ optional = true [dependencies.webpki] version = "0.22.0" +features = ["std"] optional = true [dependencies.webpki-roots] -version = "0.22.1" +version = "0.23.0" optional = true [target.'cfg(target_os = "windows")'.dependencies] diff --git a/src/conn/mod.rs b/src/conn/mod.rs index 2596965..4ea5a6a 100644 --- a/src/conn/mod.rs +++ b/src/conn/mod.rs @@ -11,7 +11,6 @@ use mysql_common::{ constants::UTF8MB4_GENERAL_CI, crypto, io::{ParseBuf, ReadMysqlExt}, - misc::raw::{bytes::NullBytes, Either, RawBytes}, named_params::parse_named_params, packets::{ binlog_request::BinlogRequest, AuthPlugin, AuthSwitchRequest, Column, ComStmtClose, @@ -57,9 +56,9 @@ use crate::{ io::Stream, prelude::*, DriverError::{ - MismatchedStmtParams, NamedParamsForPositionalQuery, OldMysqlPasswordDisabled, - Protocol41NotSet, ReadOnlyTransNotSupported, SetupError, UnexpectedPacket, - UnknownAuthPlugin, UnsupportedProtocol, + CleartextPluginDisabled, MismatchedStmtParams, NamedParamsForPositionalQuery, + OldMysqlPasswordDisabled, Protocol41NotSet, ReadOnlyTransNotSupported, SetupError, + UnexpectedPacket, UnknownAuthPlugin, UnsupportedProtocol, }, Error::{self, DriverError, MySqlError}, LocalInfileHandler, Opts, OptsBuilder, Params, QueryResult, Result, Transaction, @@ -539,38 +538,37 @@ impl Conn { AuthPlugin::Other(Cow::Borrowed(b"mysql_clear_password")) ) { if !self.0.opts.get_enable_cleartext_plugin() { - return Err(DriverError(UnknownAuthPlugin( - "mysql_clear_password".into(), - ))); + return Err(DriverError(CleartextPluginDisabled)); } } let nonce = auth_switch_request.plugin_data(); let plugin_data = match auth_switch_request.auth_plugin() { x @ AuthPlugin::MysqlOldPassword => { - x.gen_data(self.0.opts.get_pass(), nonce).map(Either::Left) - } - x @ AuthPlugin::MysqlNativePassword => { - x.gen_data(self.0.opts.get_pass(), nonce).map(Either::Left) - } - x @ AuthPlugin::CachingSha2Password => { - x.gen_data(self.0.opts.get_pass(), nonce).map(Either::Left) + if self.0.opts.get_secure_auth() { + return Err(DriverError(OldMysqlPasswordDisabled)); + } + x.gen_data(self.0.opts.get_pass(), nonce) } - AuthPlugin::Other(ref name) => { - if name.as_ref() == b"mysql_clear_password" { - Some(Either::Right(Either::Left( - RawBytes::::new( - self.0.opts.get_pass().unwrap_or_default().as_bytes(), - ) - .into_owned(), - ))) - } else { - Some(Either::Right(Either::Right([]))) + x @ AuthPlugin::MysqlNativePassword => x.gen_data(self.0.opts.get_pass(), nonce), + x @ AuthPlugin::CachingSha2Password => x.gen_data(self.0.opts.get_pass(), nonce), + x @ AuthPlugin::MysqlClearPassword => { + if !self.0.opts.get_enable_cleartext_plugin() { + return Err(DriverError(UnknownAuthPlugin( + "mysql_clear_password".into(), + ))); } + + x.gen_data(self.0.opts.get_pass(), nonce) } + AuthPlugin::Other(_) => None, + }; + + if let Some(plugin_data) = plugin_data { + self.write_struct(&plugin_data.into_owned())?; + } else { + self.write_packet(&mut &[0_u8; 0][..])?; } - .unwrap_or_else(|| Either::Right(Either::Right([]))); - self.write_struct(&plugin_data)?; self.continue_auth(&auth_switch_request.auth_plugin(), nonce, true) } @@ -622,7 +620,9 @@ impl Conn { _ => AuthPlugin::MysqlNativePassword, }; - let auth_data = auth_plugin.gen_data(self.0.opts.get_pass(), &*nonce); + let auth_data = auth_plugin + .gen_data(self.0.opts.get_pass(), &*nonce) + .map(|x| x.into_owned()); self.write_handshake_response(&auth_plugin, auth_data.as_deref())?; self.continue_auth(&auth_plugin, &*nonce, false)?; @@ -741,16 +741,16 @@ impl Conn { self.continue_mysql_native_password_auth(nonce, auth_switched)?; Ok(()) } - AuthPlugin::Other(ref name) => { - if name.as_ref() == b"mysql_clear_password" - && self.0.opts.get_enable_cleartext_plugin() - { - self.continue_mysql_native_password_auth(nonce, auth_switched)?; - Ok(()) - } else { - let plugin_name = String::from_utf8_lossy(name).into(); - Err(DriverError(UnknownAuthPlugin(plugin_name))) + AuthPlugin::MysqlClearPassword => { + if !self.0.opts.get_enable_cleartext_plugin() { + return Err(DriverError(CleartextPluginDisabled)); } + self.continue_mysql_native_password_auth(nonce, auth_switched)?; + Ok(()) + } + AuthPlugin::Other(ref name) => { + let plugin_name = String::from_utf8_lossy(name).into(); + Err(DriverError(UnknownAuthPlugin(plugin_name))) } } } diff --git a/src/error/mod.rs b/src/error/mod.rs index 7976265..298287e 100644 --- a/src/error/mod.rs +++ b/src/error/mod.rs @@ -216,6 +216,7 @@ pub enum DriverError { MixedParams, UnknownAuthPlugin(String), OldMysqlPasswordDisabled, + CleartextPluginDisabled, } impl error::Error for DriverError { @@ -279,6 +280,9 @@ impl fmt::Display for DriverError { "`old_mysql_password` plugin is insecure and disabled by default", ) } + DriverError::CleartextPluginDisabled => { + write!(f, "mysql_clear_password must be enabled on the client side") + } } } } diff --git a/src/lib.rs b/src/lib.rs index 18f511f..37eea8d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -938,7 +938,7 @@ pub mod prelude { #[doc(inline)] pub use crate::myc::row::ColumnIndex; #[doc(inline)] - pub use crate::myc::value::convert::{ConvIr, FromValue, ToValue}; + pub use crate::myc::value::convert::{FromValue, ToValue}; /// Trait for protocol markers [`crate::Binary`] and [`crate::Text`]. pub trait Protocol: crate::conn::query_result::Protocol {} From 2a975962c675181fd8ad194934dbe187a9b26c5f Mon Sep 17 00:00:00 2001 From: Anatoly Ikorsky Date: Wed, 10 May 2023 08:51:50 +0300 Subject: [PATCH 02/13] Add PoolOpts --- src/conn/opts/mod.rs | 58 ++++++++++++- src/conn/opts/pool_opts.rs | 163 +++++++++++++++++++++++++++++++++++++ src/error/mod.rs | 11 +++ src/lib.rs | 5 +- 4 files changed, 235 insertions(+), 2 deletions(-) create mode 100644 src/conn/opts/pool_opts.rs diff --git a/src/conn/opts/mod.rs b/src/conn/opts/mod.rs index 425ec82..e16898d 100644 --- a/src/conn/opts/mod.rs +++ b/src/conn/opts/mod.rs @@ -13,7 +13,9 @@ use std::{ borrow::Cow, collections::HashMap, hash::Hash, net::SocketAddr, path::Path, time::Duration, }; -use crate::{consts::CapabilityFlags, Compression, LocalInfileHandler, UrlError}; +use crate::{ + consts::CapabilityFlags, Compression, LocalInfileHandler, PoolConstraints, PoolOpts, UrlError, +}; /// Default value for client side per-connection statement cache. pub const DEFAULT_STMT_CACHE_SIZE: usize = 32; @@ -21,6 +23,8 @@ pub const DEFAULT_STMT_CACHE_SIZE: usize = 32; mod native_tls_opts; mod rustls_opts; +pub mod pool_opts; + #[cfg(feature = "native-tls")] pub use native_tls_opts::ClientIdentity; @@ -158,6 +162,9 @@ pub(crate) struct InnerOpts { /// Driver will require SSL connection if this option isn't `None` (default to `None`). ssl_opts: Option, + /// Connection pool options (defaults to [`PoolOpts::default`]). + pool_opts: PoolOpts, + /// Callback to handle requests for local files. /// /// These are caused by using `LOAD DATA LOCAL INFILE` queries. @@ -244,6 +251,7 @@ impl Default for InnerOpts { prefer_socket: true, init: vec![], ssl_opts: None, + pool_opts: PoolOpts::default(), tcp_keepalive_time: None, #[cfg(any(target_os = "linux", target_os = "macos",))] tcp_keepalive_probe_interval_secs: None, @@ -354,6 +362,11 @@ impl Opts { self.0.ssl_opts.as_ref() } + /// Connection pool options (defaults to [`Default::default`]). + pub fn get_pool_opts(&self) -> &PoolOpts { + &self.0.pool_opts + } + /// Whether `TCP_NODELAY` will be set for mysql connection. pub fn get_tcp_nodelay(&self) -> bool { self.0.tcp_nodelay @@ -555,6 +568,8 @@ impl OptsBuilder { /// OptsBuilder::new().from_hash_map(client); /// ``` /// `HashMap` key,value pairs: + /// - pool_min = upper bound for [`PoolConstraints`] + /// - pool_max = lower bound for [`PoolConstraints`] /// - user = Username /// - password = Password /// - host = Host name or ip address @@ -575,8 +590,23 @@ impl OptsBuilder { /// /// **Note:** You do **not** have to use myloginrs lib. pub fn from_hash_map(mut self, client: &HashMap) -> Result { + let mut pool_min = PoolConstraints::DEFAULT.min(); + let mut pool_max = PoolConstraints::DEFAULT.max(); + for (key, value) in client.iter() { match key.as_str() { + "pool_min" => match value.parse::() { + Ok(parsed) => pool_min = parsed, + Err(_) => { + return Err(UrlError::InvalidValue(key.to_string(), value.to_string())) + } + }, + "pool_max" => match value.parse::() { + Ok(parsed) => pool_max = parsed, + Err(_) => { + return Err(UrlError::InvalidValue(key.to_string(), value.to_string())) + } + }, "user" => self.opts.0.user = Some(value.to_string()), "password" => self.opts.0.pass = Some(value.to_string()), "host" => { @@ -682,12 +712,30 @@ impl OptsBuilder { return Err(UrlError::InvalidValue(key.to_string(), value.to_string())) } }, + "reset_connection" => match value.parse::() { + Ok(parsed) => { + self.opts.0.pool_opts = self.opts.0.pool_opts.with_reset_connection(parsed) + } + Err(_) => { + return Err(UrlError::InvalidValue(key.to_string(), value.to_string())) + } + }, _ => { //throw an error if there is an unrecognized param return Err(UrlError::UnknownParameter(key.to_string())); } } } + + if let Some(pool_constraints) = PoolConstraints::new(pool_min, pool_max) { + self.opts.0.pool_opts = self.opts.0.pool_opts.with_constraints(pool_constraints); + } else { + return Err(UrlError::InvalidPoolConstraints { + min: pool_min, + max: pool_max, + }); + } + Ok(self) } @@ -830,6 +878,14 @@ impl OptsBuilder { self } + /// Connection pool options (see [`Opts::get_pool_opts`]). + /// + /// Pass `None` to reset to default. + pub fn pool_opts>>(mut self, pool_opts: T) -> Self { + self.opts.0.pool_opts = pool_opts.into().unwrap_or_default(); + self + } + /// Callback to handle requests for local files. These are /// caused by using `LOAD DATA LOCAL INFILE` queries. The /// callback is passed the filename, and a `Write`able object diff --git a/src/conn/opts/pool_opts.rs b/src/conn/opts/pool_opts.rs new file mode 100644 index 0000000..ea1aad4 --- /dev/null +++ b/src/conn/opts/pool_opts.rs @@ -0,0 +1,163 @@ +// Copyright (c) 2023 rust-mysql-simple contributors +// +// Licensed under the Apache License, Version 2.0 +// or the MIT +// license , at your +// option. All files in the project carrying such notice may not be copied, +// modified, or distributed except according to those terms. + +macro_rules! const_assert { + ($name:ident, $($xs:expr),+ $(,)*) => { + #[allow(unknown_lints, clippy::eq_op)] + const $name: [(); 0 - !($($xs)&&+) as usize] = []; + }; +} + +/// Connection pool options. +/// +/// ``` +/// # use mysql::{PoolOpts, PoolConstraints}; +/// # use std::time::Duration; +/// let pool_opts = PoolOpts::default() +/// .with_constraints(PoolConstraints::new(15, 30).unwrap()) +/// .with_reset_connection(false); +/// ``` +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +pub struct PoolOpts { + constraints: PoolConstraints, + reset_connection: bool, +} + +impl PoolOpts { + /// Calls `Self::default`. + pub fn new() -> Self { + Self::default() + } + + /// Creates the default [`PoolOpts`] with the given constraints. + pub fn with_constraints(mut self, constraints: PoolConstraints) -> Self { + self.constraints = constraints; + self + } + + /// Returns pool constraints. + pub fn constraints(&self) -> PoolConstraints { + self.constraints + } + + /// Sets whether to reset connection upon returning it to a pool (defaults to `true`). + /// + /// Default behavior increases reliability but comes with cons: + /// + /// * reset procedure removes all prepared statements, i.e. kills prepared statements cache + /// * connection reset is quite fast but requires additional client-server roundtrip + /// (might also requires requthentication for older servers) + /// + /// The purpose of the reset procedure is to: + /// + /// * rollback any opened transactions + /// * reset transaction isolation level + /// * reset session variables + /// * delete user variables + /// * remove temporary tables + /// * remove all PREPARE statement (this action kills prepared statements cache) + /// + /// So to encrease overall performance you can safely opt-out of the default behavior + /// if you are not willing to change the session state in an unpleasant way. + /// + /// It is also possible to selectively opt-in/out using [`Conn::reset_connection`]. + /// + /// # Connection URL + /// + /// You can use `reset_connection` URL parameter to set this value. E.g. + /// + /// ``` + /// # use mysql::*; + /// # use std::time::Duration; + /// # fn main() -> Result<()> { + /// let opts = Opts::from_url("mysql://localhost/db?reset_connection=false")?; + /// assert_eq!(opts.get_pool_opts().reset_connection(), false); + /// # Ok(()) } + /// ``` + pub fn with_reset_connection(mut self, reset_connection: bool) -> Self { + self.reset_connection = reset_connection; + self + } + + /// Returns the `reset_connection` value (see [`PoolOpts::with_reset_connection`]). + pub fn reset_connection(&self) -> bool { + self.reset_connection + } +} + +impl Default for PoolOpts { + fn default() -> Self { + Self { + constraints: PoolConstraints::DEFAULT, + reset_connection: true, + } + } +} + +/// Connection pool constraints. +/// +/// This type stores `min` and `max` constraints for [`crate::Pool`] and ensures that `min <= max`. +#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)] +pub struct PoolConstraints { + min: usize, + max: usize, +} + +const_assert!( + _DEFAULT_POOL_CONSTRAINTS_ARE_CORRECT, + PoolConstraints::DEFAULT.min <= PoolConstraints::DEFAULT.max, +); + +impl PoolConstraints { + /// Default pool constraints. + pub const DEFAULT: PoolConstraints = PoolConstraints { min: 10, max: 100 }; + + /// Creates new [`PoolConstraints`] if constraints are valid (`min <= max`). + /// + /// # Connection URL + /// + /// You can use `pool_min` and `pool_max` URL parameters to define pool constraints. + /// + /// ``` + /// # use mysql::*; + /// # fn main() -> Result<()> { + /// let opts = Opts::from_url("mysql://localhost/db?pool_min=0&pool_max=151")?; + /// assert_eq!(opts.get_pool_opts().constraints(), PoolConstraints::new(0, 151).unwrap()); + /// # Ok(()) } + /// ``` + pub fn new(min: usize, max: usize) -> Option { + if min <= max { + Some(PoolConstraints { min, max }) + } else { + None + } + } + + /// Lower bound of this pool constraints. + pub const fn min(&self) -> usize { + self.min + } + + /// Upper bound of this pool constraints. + pub const fn max(&self) -> usize { + self.max + } +} + +impl Default for PoolConstraints { + fn default() -> Self { + PoolConstraints::DEFAULT + } +} + +impl From for (usize, usize) { + /// Transforms constraints to a pair of `(min, max)`. + fn from(PoolConstraints { min, max }: PoolConstraints) -> Self { + (min, max) + } +} diff --git a/src/error/mod.rs b/src/error/mod.rs index 298287e..7217ba2 100644 --- a/src/error/mod.rs +++ b/src/error/mod.rs @@ -302,6 +302,10 @@ pub enum UrlError { /// (feature_name, value) InvalidValue(String, String), UnknownParameter(String), + InvalidPoolConstraints { + min: usize, + max: usize, + }, BadUrl, } @@ -329,6 +333,13 @@ impl fmt::Display for UrlError { UrlError::UnknownParameter(ref parameter) => { write!(f, "Unknown URL parameter `{}'", parameter) } + UrlError::InvalidPoolConstraints { min, max } => { + write!( + f, + "Invalid pool constraints: pool_min ({}) > pool_max ({})", + min, max + ) + } UrlError::BadUrl => write!(f, "Invalid or incomplete connection URL"), } } diff --git a/src/lib.rs b/src/lib.rs index 37eea8d..7293094 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -896,7 +896,10 @@ pub use crate::conn::local_infile::{LocalInfile, LocalInfileHandler}; #[doc(inline)] pub use crate::conn::opts::SslOpts; #[doc(inline)] -pub use crate::conn::opts::{Opts, OptsBuilder, DEFAULT_STMT_CACHE_SIZE}; +pub use crate::conn::opts::{ + pool_opts::{PoolConstraints, PoolOpts}, + Opts, OptsBuilder, DEFAULT_STMT_CACHE_SIZE, +}; #[doc(inline)] pub use crate::conn::pool::{Pool, PooledConn}; #[doc(inline)] From 4275c01b081cde484eda456ffd906fb1fb7aa3f6 Mon Sep 17 00:00:00 2001 From: Anatoly Ikorsky Date: Thu, 11 May 2023 16:01:24 +0300 Subject: [PATCH 03/13] Improve connection reset and pool behavior. * introduce ChangeUserOpts * implement Conn::change_user and PooledConn::change_user * change user insted of reconnect in Conn::reset * implement PoolOpts::check_health --- src/conn/local_infile.rs | 42 +--- src/conn/mod.rs | 365 +++++++++++++++++++++--------- src/conn/opts/mod.rs | 134 ++++++++++- src/conn/opts/pool_opts.rs | 48 +++- src/conn/pool/inner.rs | 109 +++++++++ src/conn/{pool.rs => pool/mod.rs} | 294 +++++++++++++----------- src/lib.rs | 2 +- 7 files changed, 717 insertions(+), 277 deletions(-) create mode 100644 src/conn/pool/inner.rs rename src/conn/{pool.rs => pool/mod.rs} (76%) diff --git a/src/conn/local_infile.rs b/src/conn/local_infile.rs index 4350785..9975e2c 100644 --- a/src/conn/local_infile.rs +++ b/src/conn/local_infile.rs @@ -25,37 +25,16 @@ pub(crate) type LocalInfileInner = /// Note that older versions of Mysql server may not support this functionality. /// /// ```rust -/// # use std::io::Write; -/// # use mysql::{ -/// # Pool, -/// # Opts, -/// # OptsBuilder, -/// # LocalInfileHandler, -/// # from_row, -/// # error::Error, -/// # prelude::*, -/// # }; -/// use mysql::prelude::Queryable; -/// # fn get_opts() -> Opts { -/// # let url = if let Ok(url) = std::env::var("DATABASE_URL") { -/// # let opts = Opts::from_url(&url).expect("DATABASE_URL invalid"); -/// # if opts.get_db_name().expect("a database name is required").is_empty() { -/// # panic!("database name is empty"); -/// # } -/// # url -/// # } else { -/// # "mysql://root:password@127.0.0.1:3307/mysql".to_string() -/// # }; -/// # Opts::from_url(&*url).unwrap() -/// # } -/// # let opts = get_opts(); -/// # let pool = Pool::new_manual(1, 1, opts).unwrap(); -/// # let mut conn = pool.get_conn().unwrap(); -/// # conn.query_drop("CREATE TEMPORARY TABLE mysql.Users (id INT, name TEXT, age INT, email TEXT)").unwrap(); -/// # conn.exec_drop("INSERT INTO mysql.Users (id, name, age, email) VALUES (?, ?, ?, ?)", -/// # (1, "John", 17, "foo@bar.baz")).unwrap(); -/// conn.query_drop("CREATE TEMPORARY TABLE mysql.tbl(a TEXT)").unwrap(); +/// # mysql::doctest_wrapper!(__result, { +/// use mysql::*; +/// use mysql::prelude::*; +/// +/// use std::io::Write; /// +/// let pool = Pool::new(get_opts())?; +/// let mut conn = pool.get_conn().unwrap(); +/// +/// conn.query_drop("CREATE TEMPORARY TABLE mysql.tbl(a TEXT)").unwrap(); /// conn.set_local_infile_handler(Some( /// LocalInfileHandler::new(|file_name, writer| { /// writer.write_all(b"row1: file name is ")?; @@ -70,7 +49,7 @@ pub(crate) type LocalInfileInner = /// Ok(_) => (), /// Err(Error::MySqlError(ref e)) if e.code == 1148 => { /// // functionality is not supported by the server -/// return; +/// return Ok(()); /// } /// err => { /// err.unwrap(); @@ -83,6 +62,7 @@ pub(crate) type LocalInfileInner = /// result, /// vec!["row1: file name is file_name".to_string(), "row2: foobar".to_string()], /// ); +/// # }); /// ``` #[derive(Clone)] pub struct LocalInfileHandler(pub(crate) LocalInfileInner); diff --git a/src/conn/mod.rs b/src/conn/mod.rs index 4ea5a6a..e6752a4 100644 --- a/src/conn/mod.rs +++ b/src/conn/mod.rs @@ -13,10 +13,11 @@ use mysql_common::{ io::{ParseBuf, ReadMysqlExt}, named_params::parse_named_params, packets::{ - binlog_request::BinlogRequest, AuthPlugin, AuthSwitchRequest, Column, ComStmtClose, - ComStmtExecuteRequestBuilder, ComStmtSendLongData, CommonOkPacket, ErrPacket, - HandshakePacket, HandshakeResponse, OkPacket, OkPacketDeserializer, OkPacketKind, - OldAuthSwitchRequest, OldEofPacket, ResultSetTerminator, SessionStateInfo, + binlog_request::BinlogRequest, AuthPlugin, AuthSwitchRequest, Column, ComChangeUser, + ComChangeUserMoreData, ComStmtClose, ComStmtExecuteRequestBuilder, ComStmtSendLongData, + CommonOkPacket, ErrPacket, HandshakePacket, HandshakeResponse, OkPacket, + OkPacketDeserializer, OkPacketKind, OldAuthSwitchRequest, OldEofPacket, + ResultSetTerminator, SessionStateInfo, }, proto::{codec::Compression, sync_framed::MySyncFramed, MySerialize}, }; @@ -55,6 +56,7 @@ use crate::{ from_value, from_value_opt, io::Stream, prelude::*, + ChangeUserOpts, DriverError::{ CleartextPluginDisabled, MismatchedStmtParams, NamedParamsForPositionalQuery, OldMysqlPasswordDisabled, Protocol41NotSet, ReadOnlyTransNotSupported, SetupError, @@ -173,13 +175,18 @@ struct ConnInner { connected: bool, has_results: bool, local_infile_handler: Option, + + auth_plugin: AuthPlugin<'static>, + nonce: Vec, + + /// This flag is to opt-in/opt-out from reset upon return to a pool. + pub(crate) reset_upon_return: bool, } impl ConnInner { fn empty(opts: Opts) -> Self { ConnInner { stmt_cache: StmtCache::new(opts.get_stmt_cache_size()), - opts, stream: None, capability_flags: CapabilityFlags::empty(), status_flags: StatusFlags::empty(), @@ -192,6 +199,11 @@ impl ConnInner { server_version: None, mariadb_server_version: None, local_infile_handler: None, + auth_plugin: AuthPlugin::MysqlNativePassword, + nonce: Vec::new(), + reset_upon_return: opts.get_pool_opts().reset_connection(), + + opts, } } } @@ -353,7 +365,7 @@ impl Conn { Ok(conn) } - fn soft_reset(&mut self) -> Result<()> { + fn exec_com_reset_connection(&mut self) -> Result<()> { self.write_command(Command::COM_RESET_CONNECTION, &[])?; let packet = self.read_packet()?; self.handle_ok::(&packet)?; @@ -362,31 +374,83 @@ impl Conn { Ok(()) } - fn hard_reset(&mut self) -> Result<()> { - self.0.stmt_cache.clear(); - self.0.capability_flags = CapabilityFlags::empty(); - self.0.status_flags = StatusFlags::empty(); - self.0.connection_id = 0; - self.0.character_set = 0; - self.0.ok_packet = None; + fn exec_com_change_user(&mut self, opts: ChangeUserOpts) -> Result<()> { + opts.update_opts(&mut self.0.opts); + let com_change_user = ComChangeUser::new() + .with_user(self.0.opts.get_user().map(|x| x.as_bytes())) + .with_database(self.0.opts.get_db_name().map(|x| x.as_bytes())) + .with_auth_plugin_data( + self.0 + .auth_plugin + .gen_data(self.0.opts.get_pass(), &self.0.nonce) + .as_deref(), + ) + .with_more_data(Some( + ComChangeUserMoreData::new(if self.server_version() >= (5, 5, 3) { + UTF8MB4_GENERAL_CI + } else { + UTF8_GENERAL_CI + }) + .with_auth_plugin(Some(self.0.auth_plugin.clone())) + .with_connect_attributes( + if self.0.opts.get_connect_attrs().is_empty() { + None + } else { + Some(self.0.opts.get_connect_attrs().clone()) + }, + ), + )) + .into_owned(); + self.write_command_raw(&com_change_user)?; self.0.last_command = 0; - self.0.connected = false; - self.0.has_results = false; - self.connect_stream()?; - self.connect() + self.0.stmt_cache.clear(); + self.continue_auth(false) } - /// Resets `MyConn` (drops state then reconnects). + /// Tries to reset the connection. + /// + /// This function will try to invoke COM_RESET_CONNECTION with + /// a fall back to COM_CHANGE_USER on older servers. + /// + /// ## Note + /// + /// Re-executes [`Opts::get_init`]. pub fn reset(&mut self) -> Result<()> { - match (self.0.server_version, self.0.mariadb_server_version) { - (Some(ref version), _) if *version > (5, 7, 3) => { - self.soft_reset().or_else(|_| self.hard_reset()) - } - (_, Some(ref version)) if *version >= (10, 2, 7) => { - self.soft_reset().or_else(|_| self.hard_reset()) + let reset_result = match (self.0.server_version, self.0.mariadb_server_version) { + (Some(ref version), _) if *version > (5, 7, 3) => self.exec_com_reset_connection(), + (_, Some(ref version)) if *version >= (10, 2, 7) => self.exec_com_reset_connection(), + _ => return self.exec_com_change_user(ChangeUserOpts::DEFAULT), + }; + + match reset_result { + Ok(_) => (), + Err(crate::Error::MySqlError(_)) => { + // fallback to COM_CHANGE_USER if server reports an error for COM_RESET_CONNECTION + self.exec_com_change_user(ChangeUserOpts::DEFAULT)?; } - _ => self.hard_reset(), + Err(e) => return Err(e), } + + for cmd in self.0.opts.get_init() { + self.query_drop(cmd)?; + } + + Ok(()) + } + + /// Executes [`COM_CHANGE_USER`][1]. + /// + /// This might be used as an older and slower alternative to `COM_RESET_CONNECTION` that + /// works on MySql prior to 5.7.3 (MariaDb prior ot 10.2.4). + /// + /// ## Note + /// + /// * Using non-default `opts` for a pooled connection is discouraging. + /// * Connection options will be updated permanently. + /// + /// [1]: https://dev.mysql.com/doc/c-api/5.7/en/mysql-change-user.html + pub fn change_user(&mut self, opts: ChangeUserOpts) -> Result<()> { + self.exec_com_change_user(opts) } fn switch_to_ssl(&mut self, ssl_opts: SslOpts) -> Result<()> { @@ -542,24 +606,29 @@ impl Conn { } } - let nonce = auth_switch_request.plugin_data(); - let plugin_data = match auth_switch_request.auth_plugin() { - x @ AuthPlugin::MysqlOldPassword => { + self.0.nonce = auth_switch_request.plugin_data().to_vec(); + self.0.auth_plugin = auth_switch_request.auth_plugin().into_owned(); + let plugin_data = match self.0.auth_plugin { + ref x @ AuthPlugin::MysqlOldPassword => { if self.0.opts.get_secure_auth() { return Err(DriverError(OldMysqlPasswordDisabled)); } - x.gen_data(self.0.opts.get_pass(), nonce) + x.gen_data(self.0.opts.get_pass(), &self.0.nonce) + } + ref x @ AuthPlugin::MysqlNativePassword => { + x.gen_data(self.0.opts.get_pass(), &self.0.nonce) + } + ref x @ AuthPlugin::CachingSha2Password => { + x.gen_data(self.0.opts.get_pass(), &self.0.nonce) } - x @ AuthPlugin::MysqlNativePassword => x.gen_data(self.0.opts.get_pass(), nonce), - x @ AuthPlugin::CachingSha2Password => x.gen_data(self.0.opts.get_pass(), nonce), - x @ AuthPlugin::MysqlClearPassword => { + ref x @ AuthPlugin::MysqlClearPassword => { if !self.0.opts.get_enable_cleartext_plugin() { return Err(DriverError(UnknownAuthPlugin( "mysql_clear_password".into(), ))); } - x.gen_data(self.0.opts.get_pass(), nonce) + x.gen_data(self.0.opts.get_pass(), &self.0.nonce) } AuthPlugin::Other(_) => None, }; @@ -569,7 +638,8 @@ impl Conn { } else { self.write_packet(&mut &[0_u8; 0][..])?; } - self.continue_auth(&auth_switch_request.auth_plugin(), nonce, true) + + self.continue_auth(true) } fn do_handshake(&mut self) -> Result<()> { @@ -603,7 +673,7 @@ impl Conn { } // Handshake scramble is always 21 bytes length (20 + zero terminator) - let nonce = { + self.0.nonce = { let mut nonce = Vec::from(handshake.scramble_1_ref()); nonce.extend_from_slice(handshake.scramble_2_ref().unwrap_or(&[][..])); // Trim zero terminator. Fill with zeroes if nonce @@ -615,16 +685,13 @@ impl Conn { // Allow only CachingSha2Password and MysqlNativePassword here // because sha256_password is deprecated and other plugins won't // appear here. - let auth_plugin = match handshake.auth_plugin() { - Some(x @ AuthPlugin::CachingSha2Password) => x, + self.0.auth_plugin = match handshake.auth_plugin() { + Some(x @ AuthPlugin::CachingSha2Password) => x.into_owned(), _ => AuthPlugin::MysqlNativePassword, }; - let auth_data = auth_plugin - .gen_data(self.0.opts.get_pass(), &*nonce) - .map(|x| x.into_owned()); - self.write_handshake_response(&auth_plugin, auth_data.as_deref())?; - self.continue_auth(&auth_plugin, &*nonce, false)?; + self.write_handshake_response()?; + self.continue_auth(false)?; if self.has_capability(CapabilityFlags::CLIENT_COMPRESS) { self.switch_to_compressed(); @@ -706,17 +773,19 @@ impl Conn { self.write_struct(&ssl_request) } - fn write_handshake_response( - &mut self, - auth_plugin: &AuthPlugin<'_>, - scramble_buf: Option<&[u8]>, - ) -> Result<()> { + fn write_handshake_response(&mut self) -> Result<()> { + let auth_data = self + .0 + .auth_plugin + .gen_data(self.0.opts.get_pass(), &*self.0.nonce) + .map(|x| x.into_owned()); + let handshake_response = HandshakeResponse::new( - scramble_buf, + auth_data.as_deref(), self.0.server_version.unwrap_or((0, 0, 0)), self.0.opts.get_user().map(str::as_bytes), self.0.opts.get_db_name().map(str::as_bytes), - Some(auth_plugin.clone()), + Some(self.0.auth_plugin.clone()), self.0.capability_flags, Some(self.connect_attrs().clone()), ); @@ -726,26 +795,21 @@ impl Conn { self.write_packet(&mut &*buf) } - fn continue_auth( - &mut self, - auth_plugin: &AuthPlugin<'_>, - nonce: &[u8], - auth_switched: bool, - ) -> Result<()> { - match auth_plugin { + fn continue_auth(&mut self, auth_switched: bool) -> Result<()> { + match self.0.auth_plugin { AuthPlugin::CachingSha2Password => { - self.continue_caching_sha2_password_auth(nonce, auth_switched)?; + self.continue_caching_sha2_password_auth(auth_switched)?; Ok(()) } AuthPlugin::MysqlNativePassword | AuthPlugin::MysqlOldPassword => { - self.continue_mysql_native_password_auth(nonce, auth_switched)?; + self.continue_mysql_native_password_auth(auth_switched)?; Ok(()) } AuthPlugin::MysqlClearPassword => { if !self.0.opts.get_enable_cleartext_plugin() { return Err(DriverError(CleartextPluginDisabled)); } - self.continue_mysql_native_password_auth(nonce, auth_switched)?; + self.continue_mysql_native_password_auth(auth_switched)?; Ok(()) } AuthPlugin::Other(ref name) => { @@ -755,11 +819,7 @@ impl Conn { } } - fn continue_mysql_native_password_auth( - &mut self, - nonce: &[u8], - auth_switched: bool, - ) -> Result<()> { + fn continue_mysql_native_password_auth(&mut self, auth_switched: bool) -> Result<()> { let payload = self.read_packet()?; match payload[0] { @@ -772,7 +832,8 @@ impl Conn { } else { let _ = ParseBuf(&*payload).parse::(())?; // we'll map OldAuthSwitchRequest to an AuthSwitchRequest with mysql_old_password plugin. - AuthSwitchRequest::new("mysql_old_password".as_bytes(), nonce) + AuthSwitchRequest::new("mysql_old_password".as_bytes(), &*self.0.nonce) + .into_owned() }; self.perform_auth_switch(auth_switch) } @@ -780,11 +841,7 @@ impl Conn { } } - fn continue_caching_sha2_password_auth( - &mut self, - nonce: &[u8], - auth_switched: bool, - ) -> Result<()> { + fn continue_caching_sha2_password_auth(&mut self, auth_switched: bool) -> Result<()> { let payload = self.read_packet()?; match payload[0] { @@ -819,7 +876,7 @@ impl Conn { .unwrap_or_else(Vec::new); pass.push(0); for i in 0..pass.len() { - pass[i] ^= nonce[i % nonce.len()]; + pass[i] ^= self.0.nonce[i % self.0.nonce.len()]; } let encrypted_pass = crypto::encrypt(&*pass, key); self.write_packet(&mut encrypted_pass.as_slice())?; @@ -1184,6 +1241,17 @@ impl Conn { self.request_binlog(request)?; Ok(BinlogStream::new(self)) } + + fn cleanup_for_pool(&mut self) -> Result<()> { + self.set_local_infile_handler(None); + if self.0.reset_upon_return { + self.reset()?; + } + + self.0.reset_upon_return = self.0.opts.get_pool_opts().reset_connection(); + + Ok(()) + } } #[cfg(unix)] @@ -1251,6 +1319,7 @@ mod test { }; use mysql_common::{binlog::events::EventData, packets::binlog_request::BinlogRequest}; + use rand::Fill; use time::PrimitiveDateTime; use crate::{ @@ -1338,47 +1407,62 @@ mod test { fn query_traits() -> Result<(), Box> { macro_rules! test_query { ($conn : expr) => { - "CREATE TABLE tmplak (a INT)".run($conn)?; + "CREATE TABLE IF NOT EXISTS tmplak (a INT)" + .run($conn) + .unwrap(); + "DELETE FROM tmplak".run($conn).unwrap(); - "INSERT INTO tmplak (a) VALUES (?)".with((42,)).run($conn)?; + "INSERT INTO tmplak (a) VALUES (?)" + .with((42,)) + .run($conn) + .unwrap(); "INSERT INTO tmplak (a) VALUES (?)" .with((43..=44).map(|x| (x,))) .batch($conn)?; - let first: Option = "SELECT a FROM tmplak LIMIT 1".first($conn)?; + let first: Option = "SELECT a FROM tmplak LIMIT 1".first($conn).unwrap(); assert_eq!(first, Some(42), "first text"); - let first: Option = "SELECT a FROM tmplak LIMIT 1".with(()).first($conn)?; + let first: Option = "SELECT a FROM tmplak LIMIT 1" + .with(()) + .first($conn) + .unwrap(); assert_eq!(first, Some(42), "first bin"); - let count = "SELECT a FROM tmplak".run($conn)?.count(); + let count = "SELECT a FROM tmplak".run($conn).unwrap().count(); assert_eq!(count, 3, "run text"); - let count = "SELECT a FROM tmplak".with(()).run($conn)?.count(); + let count = "SELECT a FROM tmplak".with(()).run($conn).unwrap().count(); assert_eq!(count, 3, "run bin"); - let all: Vec = "SELECT a FROM tmplak".fetch($conn)?; + let all: Vec = "SELECT a FROM tmplak".fetch($conn).unwrap(); assert_eq!(all, vec![42, 43, 44], "fetch text"); - let all: Vec = "SELECT a FROM tmplak".with(()).fetch($conn)?; + let all: Vec = "SELECT a FROM tmplak".with(()).fetch($conn).unwrap(); assert_eq!(all, vec![42, 43, 44], "fetch bin"); - let mapped = "SELECT a FROM tmplak".map($conn, |x: u8| x + 1)?; + let mapped = "SELECT a FROM tmplak".map($conn, |x: u8| x + 1).unwrap(); assert_eq!(mapped, vec![43, 44, 45], "map text"); - let mapped = "SELECT a FROM tmplak".with(()).map($conn, |x: u8| x + 1)?; + let mapped = "SELECT a FROM tmplak" + .with(()) + .map($conn, |x: u8| x + 1) + .unwrap(); assert_eq!(mapped, vec![43, 44, 45], "map bin"); - let sum = "SELECT a FROM tmplak".fold($conn, 0_u8, |acc, x: u8| acc + x)?; + let sum = "SELECT a FROM tmplak" + .fold($conn, 0_u8, |acc, x: u8| acc + x) + .unwrap(); assert_eq!(sum, 42 + 43 + 44, "fold text"); let sum = "SELECT a FROM tmplak" .with(()) - .fold($conn, 0_u8, |acc, x: u8| acc + x)?; + .fold($conn, 0_u8, |acc, x: u8| acc + x) + .unwrap(); assert_eq!(sum, 42 + 43 + 44, "fold bin"); - "DROP TABLE tmplak".run($conn)?; + "DROP TABLE tmplak".run($conn).unwrap(); }; } @@ -1735,6 +1819,102 @@ mod test { .unwrap_err(); } + #[test] + fn should_change_user() -> crate::Result<()> { + let mut conn = Conn::new(get_opts()).unwrap(); + assert_eq!( + conn.query_first::("SELECT @foo") + .unwrap() + .unwrap(), + Value::NULL + ); + + conn.query_drop("SET @foo = 'foo'").unwrap(); + + assert_eq!( + conn.query_first::("SELECT @foo") + .unwrap() + .unwrap(), + "foo", + ); + + conn.change_user(Default::default()).unwrap(); + assert_eq!( + conn.query_first::("SELECT @foo") + .unwrap() + .unwrap(), + Value::NULL + ); + + let plugins: &[&str] = + if conn.0.mariadb_server_version.is_none() && conn.server_version() >= (5, 8, 0) { + &["mysql_native_password", "caching_sha2_password"] + } else { + &["mysql_native_password"] + }; + + for plugin in plugins { + let mut rng = rand::thread_rng(); + let mut pass = [0u8; 10]; + pass.try_fill(&mut rng).unwrap(); + let pass: String = IntoIterator::into_iter(pass) + .map(|x| ((x % (123 - 97)) + 97) as char) + .collect(); + + conn.query_drop("DELETE FROM mysql.user WHERE user = '__mats'") + .unwrap(); + conn.query_drop("FLUSH PRIVILEGES").unwrap(); + + if conn.0.mariadb_server_version.is_some() || conn.server_version() < (5, 7, 0) { + if matches!(conn.server_version(), (5, 6, _)) { + conn.query_drop( + "CREATE USER '__mats'@'%' IDENTIFIED WITH mysql_old_password", + ) + .unwrap(); + conn.query_drop(format!( + "SET PASSWORD FOR '__mats'@'%' = OLD_PASSWORD({})", + Value::from(pass.clone()).as_sql(false) + )) + .unwrap(); + } else { + conn.query_drop("CREATE USER '__mats'@'%'").unwrap(); + conn.query_drop(format!( + "SET PASSWORD FOR '__mats'@'%' = PASSWORD({})", + Value::from(pass.clone()).as_sql(false) + )) + .unwrap(); + } + } else { + conn.query_drop(format!( + "CREATE USER '__mats'@'%' IDENTIFIED WITH {} BY {}", + plugin, + Value::from(pass.clone()).as_sql(false) + )) + .unwrap(); + }; + + conn.query_drop("FLUSH PRIVILEGES").unwrap(); + + let mut conn2 = Conn::new(get_opts().secure_auth(false)).unwrap(); + conn2 + .change_user( + crate::ChangeUserOpts::default() + .with_db_name(None) + .with_user(Some("__mats".into())) + .with_pass(Some(pass)), + ) + .unwrap(); + let (db, user) = conn2 + .query_first::<(Option, String), _>("SELECT DATABASE(), USER();") + .unwrap() + .unwrap(); + assert_eq!(db, None); + assert!(user.starts_with("__mats")); + } + + Ok(()) + } + #[test] fn prep_exec() { let mut conn = Conn::new(get_opts()).unwrap(); @@ -1808,23 +1988,6 @@ mod test { } } - /// Library panics with "incomplete connection" in case of subsequent - /// failed calls to `reset` when the server is down. - /// (see [blackbeam/rust-mysql-simple#317][1]). - /// - /// [1]: https://github.com/blackbeam/rust-mysql-simple/issues/317 - #[test] - fn issue_317() { - let mut c = Conn::new(get_opts()).unwrap(); - c.0.opts = get_opts().tcp_port(55555).into(); - let version = std::mem::replace(&mut c.0.server_version, Some((0, 0, 0))); - let mdbversion = std::mem::replace(&mut c.0.mariadb_server_version, Some((0, 0, 0))); - c.reset().unwrap_err(); - c.0.server_version = version; - c.0.mariadb_server_version = mdbversion; - let _ = c.reset(); - } - #[test] fn should_drop_multi_result_set() { let opts = OptsBuilder::from_opts(get_opts()).db_name(Some("mysql")); diff --git a/src/conn/opts/mod.rs b/src/conn/opts/mod.rs index e16898d..c8eb913 100644 --- a/src/conn/opts/mod.rs +++ b/src/conn/opts/mod.rs @@ -10,7 +10,7 @@ use percent_encoding::percent_decode; use url::Url; use std::{ - borrow::Cow, collections::HashMap, hash::Hash, net::SocketAddr, path::Path, time::Duration, + borrow::Cow, collections::HashMap, fmt, hash::Hash, net::SocketAddr, path::Path, time::Duration, }; use crate::{ @@ -203,7 +203,7 @@ pub(crate) struct InnerOpts { /// Additional client capabilities to set (defaults to empty). /// - /// This value will be OR'ed with other client capabilities during connection initialisation. + /// This value will be OR'ed with other client capabilities during connection initialization. /// /// ### Note /// @@ -438,7 +438,7 @@ impl Opts { /// Additional client capabilities to set (defaults to empty). /// - /// This value will be OR'ed with other client capabilities during connection initialisation. + /// This value will be OR'ed with other client capabilities during connection initialization. /// /// ### Note /// @@ -720,6 +720,14 @@ impl OptsBuilder { return Err(UrlError::InvalidValue(key.to_string(), value.to_string())) } }, + "check_health" => match value.parse::() { + Ok(parsed) => { + self.opts.0.pool_opts = self.opts.0.pool_opts.with_check_health(parsed) + } + Err(_) => { + return Err(UrlError::InvalidValue(key.to_string(), value.to_string())) + } + }, _ => { //throw an error if there is an unrecognized param return Err(UrlError::UnknownParameter(key.to_string())); @@ -950,7 +958,7 @@ impl OptsBuilder { /// Additional client capabilities to set (defaults to empty). /// - /// This value will be OR'ed with other client capabilities during connection initialisation. + /// This value will be OR'ed with other client capabilities during connection initialization. /// /// ### Note /// @@ -1147,6 +1155,124 @@ fn from_url(url: &str) -> Result { .map(Into::into) } +/// [`COM_CHANGE_USER`][1] options. +/// +/// Connection [`Opts`] are going to be updated accordingly upon `COM_CHANGE_USER`. +/// +/// [`Opts`] won't be updated by default, because default `ChangeUserOpts` will reuse +/// connection's `user`, `pass` and `db_name`. +/// +/// [1]: https://dev.mysql.com/doc/c-api/5.7/en/mysql-change-user.html +#[derive(Clone, Eq, PartialEq)] +pub struct ChangeUserOpts { + user: Option>, + pass: Option>, + db_name: Option>, +} + +impl ChangeUserOpts { + pub const DEFAULT: Self = Self { + user: None, + pass: None, + db_name: None, + }; + + pub(crate) fn update_opts(self, opts: &mut Opts) { + if self.user.is_none() && self.pass.is_none() && self.db_name.is_none() { + return; + } + + let mut builder = OptsBuilder::from_opts(opts.clone()); + + if let Some(user) = self.user { + builder = builder.user(user); + } + + if let Some(pass) = self.pass { + builder = builder.pass(pass); + } + + if let Some(db_name) = self.db_name { + builder = builder.db_name(db_name); + } + + *opts = Opts::from(builder); + } + + /// Creates change user options that'll reuse connection options. + pub fn new() -> Self { + Self { + user: None, + pass: None, + db_name: None, + } + } + + /// Set [`Opts::get_user`] to the given value. + pub fn with_user(mut self, user: Option) -> Self { + self.user = Some(user); + self + } + + /// Set [`Opts::get_pass`] to the given value. + pub fn with_pass(mut self, pass: Option) -> Self { + self.pass = Some(pass); + self + } + + /// Set [`Opts::get_db_name`] to the given value. + pub fn with_db_name(mut self, db_name: Option) -> Self { + self.db_name = Some(db_name); + self + } + + /// Returns user. + /// + /// * if `None` then `self` does not meant to change user + /// * if `Some(None)` then `self` will clear user + /// * if `Some(Some(_))` then `self` will change user + pub fn user(&self) -> Option> { + self.user.as_ref().map(|x| x.as_deref()) + } + + /// Returns password. + /// + /// * if `None` then `self` does not meant to change password + /// * if `Some(None)` then `self` will clear password + /// * if `Some(Some(_))` then `self` will change password + pub fn pass(&self) -> Option> { + self.pass.as_ref().map(|x| x.as_deref()) + } + + /// Returns database name. + /// + /// * if `None` then `self` does not meant to change database name + /// * if `Some(None)` then `self` will clear database name + /// * if `Some(Some(_))` then `self` will change database name + pub fn db_name(&self) -> Option> { + self.db_name.as_ref().map(|x| x.as_deref()) + } +} + +impl Default for ChangeUserOpts { + fn default() -> Self { + Self::new() + } +} + +impl fmt::Debug for ChangeUserOpts { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("ChangeUserOpts") + .field("user", &self.user) + .field( + "pass", + &self.pass.as_ref().map(|x| x.as_ref().map(|_| "...")), + ) + .field("db_name", &self.db_name) + .finish() + } +} + #[cfg(test)] mod test { use mysql_common::proto::codec::Compression; diff --git a/src/conn/opts/pool_opts.rs b/src/conn/opts/pool_opts.rs index ea1aad4..d204e56 100644 --- a/src/conn/opts/pool_opts.rs +++ b/src/conn/opts/pool_opts.rs @@ -26,6 +26,7 @@ macro_rules! const_assert { pub struct PoolOpts { constraints: PoolConstraints, reset_connection: bool, + check_health: bool, } impl PoolOpts { @@ -45,13 +46,13 @@ impl PoolOpts { self.constraints } - /// Sets whether to reset connection upon returning it to a pool (defaults to `true`). + /// Sets whether to reset the connection upon returning it to a pool (defaults to `true`). /// /// Default behavior increases reliability but comes with cons: /// /// * reset procedure removes all prepared statements, i.e. kills prepared statements cache /// * connection reset is quite fast but requires additional client-server roundtrip - /// (might also requires requthentication for older servers) + /// (might require re-authentication for older servers) /// /// The purpose of the reset procedure is to: /// @@ -62,7 +63,7 @@ impl PoolOpts { /// * remove temporary tables /// * remove all PREPARE statement (this action kills prepared statements cache) /// - /// So to encrease overall performance you can safely opt-out of the default behavior + /// So to increase overall performance you can safely opt-out of the default behavior /// if you are not willing to change the session state in an unpleasant way. /// /// It is also possible to selectively opt-in/out using [`Conn::reset_connection`]. @@ -88,6 +89,31 @@ impl PoolOpts { pub fn reset_connection(&self) -> bool { self.reset_connection } + + /// Sets whether to check connection health upon retrieving it from a pool (defaults to `true`). + /// + /// If `true`, then `Conn::ping` will be invoked on a non-fresh pooled connection. + /// + /// # Connection URL + /// + /// Use `check_health` URL parameter to set this value. E.g. + /// + /// ``` + /// # use mysql::*; + /// # use std::time::Duration; + /// # fn main() -> Result<()> { + /// let opts = Opts::from_url("mysql://localhost/db?check_health=false")?; + /// assert_eq!(opts.get_pool_opts().check_health(), false); + /// # Ok(()) } + /// ``` + pub fn with_check_health(mut self, check_health: bool) -> Self { + self.check_health = check_health; + self + } + + pub fn check_health(&self) -> bool { + self.check_health + } } impl Default for PoolOpts { @@ -95,6 +121,7 @@ impl Default for PoolOpts { Self { constraints: PoolConstraints::DEFAULT, reset_connection: true, + check_health: true, } } } @@ -113,6 +140,16 @@ const_assert!( PoolConstraints::DEFAULT.min <= PoolConstraints::DEFAULT.max, ); +pub struct Assert; +impl Assert { + pub const LEQ: usize = R - L; +} + +#[allow(path_statements)] +pub const fn gte() { + Assert::::LEQ; +} + impl PoolConstraints { /// Default pool constraints. pub const DEFAULT: PoolConstraints = PoolConstraints { min: 10, max: 100 }; @@ -138,6 +175,11 @@ impl PoolConstraints { } } + pub const fn new_const() -> PoolConstraints { + gte::(); + PoolConstraints { min: MIN, max: MAX } + } + /// Lower bound of this pool constraints. pub const fn min(&self) -> usize { self.min diff --git a/src/conn/pool/inner.rs b/src/conn/pool/inner.rs new file mode 100644 index 0000000..104f2cb --- /dev/null +++ b/src/conn/pool/inner.rs @@ -0,0 +1,109 @@ +use std::{ + collections::VecDeque, + sync::{ + atomic::{AtomicUsize, Ordering}, + Condvar, Mutex, + }, +}; + +use crate::{Conn, Opts, PoolOpts}; + +#[derive(Debug)] +pub struct Protected { + opts: Opts, + connections: VecDeque, +} + +impl Protected { + fn new(opts: Opts) -> crate::Result { + let constraints = opts.get_pool_opts().constraints(); + + let mut this = Protected { + connections: VecDeque::with_capacity(constraints.max()), + opts, + }; + + for _ in 0..constraints.min() { + this.new_conn()?; + } + + Ok(this) + } + + pub fn new_conn(&mut self) -> crate::Result<()> { + match Conn::new(self.opts.clone()) { + Ok(conn) => { + self.connections.push_back(conn); + Ok(()) + } + Err(err) => Err(err), + } + } + + pub fn take_by_query(&mut self, query: &[u8]) -> Option { + match self + .connections + .iter() + .position(|conn| conn.has_stmt(query)) + { + Some(position) => self.connections.swap_remove_back(position), + None => None, + } + } + + pub fn pop_front(&mut self) -> Option { + self.connections.pop_front() + } + + pub fn push_back(&mut self, conn: Conn) { + self.connections.push_back(conn) + } +} + +pub struct Inner { + protected: (Mutex, Condvar), + pool_opts: PoolOpts, + count: AtomicUsize, +} + +impl Inner { + pub fn increase(&self) { + let prev = self.count.fetch_add(1, Ordering::SeqCst); + debug_assert!(prev < self.max_constraint()); + } + + pub fn decrease(&self) { + let prev = self.count.fetch_sub(1, Ordering::SeqCst); + debug_assert!(prev > 0); + } + + pub fn count(&self) -> usize { + let value = self.count.load(Ordering::SeqCst); + debug_assert!(value <= self.max_constraint()); + value + } + + pub fn is_full(&self) -> bool { + self.count() == self.max_constraint() + } + + pub fn opts(&self) -> &PoolOpts { + &self.pool_opts + } + + pub fn max_constraint(&self) -> usize { + self.pool_opts.constraints().max() + } + + pub fn protected(&self) -> &(Mutex, Condvar) { + &self.protected + } + + pub fn new(opts: Opts) -> crate::Result { + Ok(Self { + count: AtomicUsize::new(opts.get_pool_opts().constraints().min()), + pool_opts: opts.get_pool_opts().clone(), + protected: (Mutex::new(Protected::new(opts)?), Condvar::new()), + }) + } +} diff --git a/src/conn/pool.rs b/src/conn/pool/mod.rs similarity index 76% rename from src/conn/pool.rs rename to src/conn/pool/mod.rs index fa3f636..52e413c 100644 --- a/src/conn/pool.rs +++ b/src/conn/pool/mod.rs @@ -7,62 +7,23 @@ // modified, or distributed except according to those terms. use std::{ - collections::VecDeque, fmt, ops::Deref, - sync::{ - atomic::{AtomicUsize, Ordering}, - Arc, Condvar, Mutex, - }, + sync::Arc, time::{Duration, Instant}, }; use crate::{ conn::query_result::{Binary, Text}, prelude::*, - Conn, DriverError, Error, LocalInfileHandler, Opts, Params, QueryResult, Result, Statement, - Transaction, TxOpts, + ChangeUserOpts, Conn, DriverError, LocalInfileHandler, Opts, Params, QueryResult, Result, + Statement, Transaction, TxOpts, }; -#[derive(Debug)] -struct InnerPool { - opts: Opts, - pool: VecDeque, -} - -impl InnerPool { - fn new(min: usize, max: usize, opts: Opts) -> Result { - if min > max || max == 0 { - return Err(Error::DriverError(DriverError::InvalidPoolConstraints)); - } - let mut pool = InnerPool { - opts, - pool: VecDeque::with_capacity(max), - }; - for _ in 0..min { - pool.new_conn()?; - } - Ok(pool) - } - fn new_conn(&mut self) -> Result<()> { - match Conn::new(self.opts.clone()) { - Ok(conn) => { - self.pool.push_back(conn); - Ok(()) - } - Err(err) => Err(err), - } - } -} +mod inner; -struct ArcedPool { - inner: (Mutex, Condvar), - min: usize, - max: usize, - count: AtomicUsize, -} - -/// `Pool` serves to provide you with a [`PooledConn`](struct.PooledConn.html)'s. +/// Thread-safe cloneable smart pointer to a connection pool. +/// /// However you can prepare statements directly on `Pool` without /// invoking [`Pool::get_conn`](struct.Pool.html#method.get_conn). /// @@ -76,11 +37,12 @@ struct ArcedPool { /// # use mysql::*; /// # use mysql::prelude::*; /// # let mut conn = Conn::new(get_opts())?; -/// let opts = get_opts(); +/// # let pool_opts = PoolOpts::new().with_constraints(PoolConstraints::new_const::<5, 10>()); +/// # let opts = get_opts().pool_opts(pool_opts); /// let pool = Pool::new(opts).unwrap(); /// let mut threads = Vec::new(); /// -/// for _ in 0..100 { +/// for _ in 0..1000 { /// let pool = pool.clone(); /// threads.push(std::thread::spawn(move || { /// let mut conn = pool.get_conn().unwrap(); @@ -99,22 +61,18 @@ struct ArcedPool { /// [`PooledConn`](struct.PooledConn.html) documentation. #[derive(Clone)] pub struct Pool { - arced_pool: Arc, - check_health: bool, - use_cache: bool, + inner: Arc, } impl Pool { /// Will return connection taken from a pool. /// - /// Will verify and fix it via `Conn::ping` and `Conn::reset` if `call_ping` is `true`. - /// Will try to get concrete connection if `id` is `Some(_)`. /// Will wait til timeout if `timeout_ms` is `Some(_)` fn _get_conn>( &self, stmt: Option, timeout_ms: Option, - call_ping: bool, + mut call_ping: bool, ) -> Result { let times = if let Some(timeout_ms) = timeout_ms { Some((Instant::now(), Duration::from_millis(timeout_ms.into()))) @@ -122,19 +80,12 @@ impl Pool { None }; - let &(ref inner_pool, ref condvar) = &self.arced_pool.inner; + let &(ref protected, ref condvar) = self.inner.protected(); - let conn = if self.use_cache { - if let Some(query) = stmt { - let mut id = None; - let mut pool = inner_pool.lock()?; - for (i, conn) in pool.pool.iter().rev().enumerate() { - if conn.has_stmt(query.as_ref()) { - id = Some(i); - break; - } - } - id.and_then(|id| pool.pool.swap_remove_back(id)) + let conn = if !self.inner.opts().reset_connection() { + // stmt cache considered enabled if reset_connection is false + if let Some(ref query) = stmt { + protected.lock()?.take_by_query(query.as_ref()) } else { None } @@ -145,32 +96,33 @@ impl Pool { let mut conn = if let Some(conn) = conn { conn } else { - let mut pool = inner_pool.lock()?; + let mut protected = protected.lock()?; loop { - if let Some(conn) = pool.pool.pop_front() { - drop(pool); + if let Some(conn) = protected.pop_front() { + drop(protected); break conn; - } else if self.arced_pool.count.load(Ordering::Relaxed) < self.arced_pool.max { - pool.new_conn()?; - self.arced_pool.count.fetch_add(1, Ordering::SeqCst); - } else { - pool = if let Some((start, timeout)) = times { + } else if self.inner.is_full() { + protected = if let Some((start, timeout)) = times { if start.elapsed() > timeout { return Err(DriverError::Timeout.into()); } - condvar.wait_timeout(pool, timeout)?.0 + condvar.wait_timeout(protected, timeout)?.0 } else { - condvar.wait(pool)? + condvar.wait(protected)? } + } else { + protected.new_conn()?; + self.inner.increase(); + // we do not have to call ping for a fresh connection + call_ping = false; } } }; - if call_ping && self.check_health && !conn.ping() { - if let Err(err) = conn.reset() { - self.arced_pool.count.fetch_sub(1, Ordering::SeqCst); - return Err(err); - } + if call_ping && self.inner.opts().check_health() && !conn.ping() { + // existing connection seem to be dead, retrying.. + self.inner.decrease(); + return self._get_conn(stmt, timeout_ms, call_ping); } Ok(PooledConn { @@ -179,51 +131,18 @@ impl Pool { }) } - /// Creates new pool with `min = 10` and `max = 100`. + /// Creates new pool with the given options (see [`Opts`]). pub fn new(opts: T) -> Result where Opts: TryFrom, crate::Error: From, { - Pool::new_manual(10, 100, opts) - } - - /// Same as `new` but you can set `min` and `max`. - pub fn new_manual(min: usize, max: usize, opts: T) -> Result - where - Opts: TryFrom, - crate::Error: From, - { - let pool = InnerPool::new(min, max, opts.try_into()?)?; Ok(Pool { - arced_pool: Arc::new(ArcedPool { - inner: (Mutex::new(pool), Condvar::new()), - min, - max, - count: AtomicUsize::new(min), - }), - use_cache: true, - check_health: true, + inner: Arc::new(inner::Inner::new(Opts::try_from(opts)?)?), }) } - /// A way to turn off searching for cached statement (on by default). - #[doc(hidden)] - pub fn use_cache(&mut self, use_cache: bool) { - self.use_cache = use_cache; - } - - /// A way to turn off connection health check on each call to `get_conn` (on by default). - pub fn check_health(&mut self, check_health: bool) { - self.check_health = check_health; - } - /// Gives you a [`PooledConn`](struct.PooledConn.html). - /// - /// `Pool` will check that connection is alive via - /// [`Conn::ping`](struct.Conn.html#method.ping) and will - /// call [`Conn::reset`](struct.Conn.html#method.reset) if - /// necessary. pub fn get_conn(&self) -> Result { self._get_conn(None::, None, true) } @@ -256,15 +175,14 @@ impl fmt::Debug for Pool { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( f, - "Pool {{ min: {}, max: {}, count: {} }}", - self.arced_pool.min, - self.arced_pool.max, - self.arced_pool.count.load(Ordering::Relaxed) + "Pool {{ constraints: {:?}, count: {} }}", + self.inner.opts().constraints(), + self.inner.count(), ) } } -/// Pooled mysql connection which will return to the pool on `drop`. +/// Pooled mysql connection. /// /// You should prefer using `prep` along `exec` instead of `query` from the Queryable trait where /// possible, except cases when statement has no params and when it has no return values or return @@ -312,16 +230,27 @@ impl Deref for PooledConn { impl Drop for PooledConn { fn drop(&mut self) { - if self.pool.arced_pool.count.load(Ordering::Relaxed) > self.pool.arced_pool.max - || self.conn.is_none() - { - self.pool.arced_pool.count.fetch_sub(1, Ordering::SeqCst); - } else { - self.conn.as_mut().unwrap().set_local_infile_handler(None); - let mut pool = (self.pool.arced_pool.inner).0.lock().unwrap(); - pool.pool.push_back(self.conn.take().unwrap()); - drop(pool); - (self.pool.arced_pool.inner).1.notify_one(); + if let Some(mut conn) = self.conn.take() { + match conn.cleanup_for_pool() { + Ok(_) => { + let (protected, condvar) = self.pool.inner.protected(); + match protected.lock() { + Ok(mut protected) => { + protected.push_back(conn); + drop(protected); + condvar.notify_one(); + } + Err(_) => { + // everything is broken + self.pool.inner.decrease(); + } + } + } + Err(_) => { + // the connection is broken + self.pool.inner.decrease(); + } + } } } } @@ -372,6 +301,23 @@ impl PooledConn { .unwrap() .set_local_infile_handler(handler); } + + /// Invokes `COM_CHANGE_USER` (see [`Conn::change_user`] docs). + pub fn change_user(&mut self) -> Result<()> { + self.conn + .as_mut() + .unwrap() + .change_user(ChangeUserOpts::default()) + } + + /// Turns on/off automatic connection reset upon return to a pool (see [`Opts::get_pool_opts`]). + /// + /// Initial value is taken from [`crate::PoolOpts::reset_connection`]. + pub fn reset_connection(&mut self, reset_connection: bool) { + if let Some(conn) = self.conn.as_mut() { + conn.0.reset_upon_return = reset_connection; + } + } } impl Queryable for PooledConn { @@ -404,7 +350,7 @@ mod test { use crate::{ from_value, prelude::*, test_misc::get_opts, DriverError, Error, OptsBuilder, Pool, - TxOpts, + PoolConstraints, PoolOpts, TxOpts, Value, }; #[test] @@ -458,7 +404,10 @@ mod test { #[test] fn should_fix_connectivity_errors_on_prepare() { - let pool = Pool::new_manual(2, 2, get_opts()).unwrap(); + let pool = Pool::new(get_opts().pool_opts( + PoolOpts::default().with_constraints(PoolConstraints::new_const::<2, 2>()), + )) + .unwrap(); let mut conn = pool.get_conn().unwrap(); let id: u32 = pool @@ -478,7 +427,10 @@ mod test { #[test] fn should_fix_connectivity_errors_on_prep_exec() { - let pool = Pool::new_manual(2, 2, get_opts()).unwrap(); + let pool = Pool::new(get_opts().pool_opts( + PoolOpts::default().with_constraints(PoolConstraints::new_const::<2, 2>()), + )) + .unwrap(); let mut conn = pool.get_conn().unwrap(); let id: u32 = pool @@ -497,7 +449,10 @@ mod test { } #[test] fn should_fix_connectivity_errors_on_start_transaction() { - let pool = Pool::new_manual(2, 2, get_opts()).unwrap(); + let pool = Pool::new(get_opts().pool_opts( + PoolOpts::default().with_constraints(PoolConstraints::new_const::<2, 2>()), + )) + .unwrap(); let mut conn = pool.get_conn().unwrap(); let id: u32 = pool @@ -530,7 +485,10 @@ mod test { } #[test] fn should_timeout_if_no_connections_available() { - let pool = Pool::new_manual(0, 1, get_opts()).unwrap(); + let pool = Pool::new(get_opts().pool_opts( + PoolOpts::default().with_constraints(PoolConstraints::new_const::<0, 1>()), + )) + .unwrap(); let conn1 = pool.try_get_conn(357).unwrap(); let conn2 = pool.try_get_conn(357); assert!(conn2.is_err()); @@ -575,7 +533,14 @@ mod test { #[test] #[allow(unused_variables)] fn should_start_transaction_on_Pool() { - let pool = Pool::new_manual(1, 10, get_opts()).unwrap(); + let pool = Pool::new( + get_opts().pool_opts( + PoolOpts::default() + .with_constraints(PoolConstraints::new_const::<1, 10>()) + .with_reset_connection(false), + ), + ) + .unwrap(); pool.get_conn() .unwrap() .query_drop("CREATE TEMPORARY TABLE mysql.tbl(a INT)") @@ -632,7 +597,9 @@ mod test { #[test] fn should_reuse_connections() -> crate::Result<()> { - let pool = Pool::new_manual(1, 1, get_opts())?; + let pool = Pool::new(get_opts().pool_opts( + PoolOpts::default().with_constraints(PoolConstraints::new_const::<1, 1>()), + ))?; let mut conn = pool.get_conn()?; let server_version = conn.server_version(); @@ -689,6 +656,59 @@ mod test { } } + #[test] + fn should_opt_out_of_connection_reset() { + let pool_opts = PoolOpts::new().with_constraints(PoolConstraints::new_const::<1, 1>()); + let opts = get_opts().pool_opts(pool_opts.clone()); + + let pool = Pool::new(opts.clone()).unwrap(); + + let mut conn = pool.get_conn().unwrap(); + assert_eq!( + conn.query_first::("SELECT @foo").unwrap(), + Some(Value::NULL) + ); + conn.query_drop("SET @foo = 'foo'").unwrap(); + assert_eq!( + conn.query_first::("SELECT @foo") + .unwrap() + .unwrap(), + "foo", + ); + drop(conn); + + conn = pool.get_conn().unwrap(); + assert_eq!( + conn.query_first::("SELECT @foo").unwrap(), + Some(Value::NULL) + ); + conn.query_drop("SET @foo = 'foo'").unwrap(); + conn.reset_connection(false); + drop(conn); + + conn = pool.get_conn().unwrap(); + assert_eq!( + conn.query_first::("SELECT @foo") + .unwrap() + .unwrap(), + "foo", + ); + drop(conn); + + let pool = Pool::new(opts.pool_opts(pool_opts.with_reset_connection(false))).unwrap(); + conn = pool.get_conn().unwrap(); + conn.query_drop("SET @foo = 'foo'").unwrap(); + drop(conn); + conn = pool.get_conn().unwrap(); + assert_eq!( + conn.query_first::("SELECT @foo") + .unwrap() + .unwrap(), + "foo", + ); + drop(conn); + } + #[cfg(feature = "nightly")] mod bench { use test; diff --git a/src/lib.rs b/src/lib.rs index 7293094..fb480ef 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -898,7 +898,7 @@ pub use crate::conn::opts::SslOpts; #[doc(inline)] pub use crate::conn::opts::{ pool_opts::{PoolConstraints, PoolOpts}, - Opts, OptsBuilder, DEFAULT_STMT_CACHE_SIZE, + ChangeUserOpts, Opts, OptsBuilder, DEFAULT_STMT_CACHE_SIZE, }; #[doc(inline)] pub use crate::conn::pool::{Pool, PooledConn}; From dd902d3d1259053c8bbf524f53a50f25c801f228 Mon Sep 17 00:00:00 2001 From: Anatoly Ikorsky Date: Thu, 11 May 2023 16:42:29 +0300 Subject: [PATCH 04/13] Update readme --- README.md | 28 +++++++++++++++++++++++++--- src/lib.rs | 28 +++++++++++++++++++++++++--- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 23c98f4..6c8abf6 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ Features: * MySql binary protocol support, i.e. support of prepared statements and binary result sets; * support of multi-result sets; * support of named parameters for prepared statements (see the [Named Parameters](#named-parameters) section); -* optional per-connection cache of prepared statements (see the [Statement Cache](#statement-cache) section); +* per-connection cache of prepared statements (see the [Statement Cache](#statement-cache) section); * buffer pool (see the [Buffer Pool](#buffer-pool) section); * support of MySql packets larger than 2^24; * support of Unix sockets and Windows named pipes; @@ -26,7 +26,8 @@ Features: * support of MySql protocol compression; * support of auth plugins: * **mysql_native_password** - for MySql prior to v8; - * **caching_sha2_password** - for MySql v8 and higher. + * **caching_sha2_password** - for MySql v8 and higher; + * **mysql_clear_password** - opt-in (see [`Opts::get_enable_cleartext_plugin`]. ### Installation @@ -176,7 +177,14 @@ let _ = Opts::from_url("mysql://user:pass%20word@127.0.0.1:3307/some_db?")?; Supported URL parameters (for the meaning of each field please refer to the docs on `Opts` structure in the create API docs): -* `prefer_socket: true | false` - defines the value of the same field in the `Opts` structure; +* `user: string` – MySql client user name +* `password: string` – MySql client password; +* `db_name: string` – MySql database name; +* `host: Host` – MySql server hostname/ip; +* `port: u16` – MySql server port; +* `pool_min: usize` – see [`PoolConstraints::min`]; +* `pool_max: usize` – see [`PoolConstraints::max`]; +* `prefer_socket: true | false` - see [`Opts::get_prefer_socket`]; * `tcp_keepalive_time_ms: u32` - defines the value (in milliseconds) of the `tcp_keepalive_time` field in the `Opts` structure; * `tcp_keepalive_probe_interval_secs: u32` - defines the value @@ -188,6 +196,10 @@ structure in the create API docs): * `tcp_user_timeout_ms` - defines the value (in milliseconds) of the `tcp_user_timeout` field in the `Opts` structure; * `stmt_cache_size: u32` - defines the value of the same field in the `Opts` structure; +* `enable_cleartext_plugin` – see [`Opts::get_enable_cleartext_plugin`]; +* `secure_auth` – see [`Opts::get_secure_auth`]; +* `reset_connection` – see [`PoolOpts::get_reset_connection`]; +* `check_health` – see [`PoolOpts::get_check_health`]; * `compress` - defines the value of the same field in the `Opts` structure. Supported value are: * `true` - enables compression with the default compression level; @@ -646,6 +658,16 @@ assert!(conn_2.exec_drop(&stmt_1, ("foo",)).is_err()); #### Statement cache +##### Note + +Statemet cache only works for: +1. for raw [`Conn`] +2. for [`PooledConn`]: + * within it's lifetime if [`PoolOpts::reset_connection`] is `true` + * within the lifetime of a wrapped [`Conn`] if [`PoolOpts::reset_connection`] is `false` + +##### Description + `Conn` will manage the cache of prepared statements on the client side, so subsequent calls to prepare with the same statement won't lead to a client-server roundtrip. Cache size for each connection is determined by the `stmt_cache_size` field of the `Opts` structure. diff --git a/src/lib.rs b/src/lib.rs index fb480ef..2482940 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -19,7 +19,7 @@ //! * MySql binary protocol support, i.e. support of prepared statements and binary result sets; //! * support of multi-result sets; //! * support of named parameters for prepared statements (see the [Named Parameters](#named-parameters) section); -//! * optional per-connection cache of prepared statements (see the [Statement Cache](#statement-cache) section); +//! * per-connection cache of prepared statements (see the [Statement Cache](#statement-cache) section); //! * buffer pool (see the [Buffer Pool](#buffer-pool) section); //! * support of MySql packets larger than 2^24; //! * support of Unix sockets and Windows named pipes; @@ -27,7 +27,8 @@ //! * support of MySql protocol compression; //! * support of auth plugins: //! * **mysql_native_password** - for MySql prior to v8; -//! * **caching_sha2_password** - for MySql v8 and higher. +//! * **caching_sha2_password** - for MySql v8 and higher; +//! * **mysql_clear_password** - opt-in (see [`Opts::get_enable_cleartext_plugin`]. //! //! ## Installation //! @@ -181,7 +182,14 @@ //! Supported URL parameters (for the meaning of each field please refer to the docs on `Opts` //! structure in the create API docs): //! -//! * `prefer_socket: true | false` - defines the value of the same field in the `Opts` structure; +//! * `user: string` – MySql client user name +//! * `password: string` – MySql client password; +//! * `db_name: string` – MySql database name; +//! * `host: Host` – MySql server hostname/ip; +//! * `port: u16` – MySql server port; +//! * `pool_min: usize` – see [`PoolConstraints::min`]; +//! * `pool_max: usize` – see [`PoolConstraints::max`]; +//! * `prefer_socket: true | false` - see [`Opts::get_prefer_socket`]; //! * `tcp_keepalive_time_ms: u32` - defines the value (in milliseconds) //! of the `tcp_keepalive_time` field in the `Opts` structure; //! * `tcp_keepalive_probe_interval_secs: u32` - defines the value @@ -193,6 +201,10 @@ //! * `tcp_user_timeout_ms` - defines the value (in milliseconds) //! of the `tcp_user_timeout` field in the `Opts` structure; //! * `stmt_cache_size: u32` - defines the value of the same field in the `Opts` structure; +//! * `enable_cleartext_plugin` – see [`Opts::get_enable_cleartext_plugin`]; +//! * `secure_auth` – see [`Opts::get_secure_auth`]; +//! * `reset_connection` – see [`PoolOpts::get_reset_connection`]; +//! * `check_health` – see [`PoolOpts::get_check_health`]; //! * `compress` - defines the value of the same field in the `Opts` structure. //! Supported value are: //! * `true` - enables compression with the default compression level; @@ -687,6 +699,16 @@ //! //! ### Statement cache //! +//! #### Note +//! +//! Statemet cache only works for: +//! 1. for raw [`Conn`] +//! 2. for [`PooledConn`]: +//! * within it's lifetime if [`PoolOpts::reset_connection`] is `true` +//! * within the lifetime of a wrapped [`Conn`] if [`PoolOpts::reset_connection`] is `false` +//! +//! #### Description +//! //! `Conn` will manage the cache of prepared statements on the client side, so subsequent calls //! to prepare with the same statement won't lead to a client-server roundtrip. Cache size //! for each connection is determined by the `stmt_cache_size` field of the `Opts` structure. From 82ba359fd1dbf07982b60ab63098f72cbda98ad3 Mon Sep 17 00:00:00 2001 From: Anatoly Ikorsky Date: Thu, 11 May 2023 16:56:08 +0300 Subject: [PATCH 05/13] clippy --- build.rs | 4 +- src/buffer_pool/enabled.rs | 6 +-- src/conn/binlog_stream.rs | 24 +++++----- src/conn/local_infile.rs | 2 +- src/conn/mod.rs | 78 +++++++++++++++----------------- src/conn/opts/mod.rs | 26 +++-------- src/conn/opts/native_tls_opts.rs | 2 +- src/conn/opts/pool_opts.rs | 1 + src/conn/pool/mod.rs | 24 ++++------ src/conn/query.rs | 4 +- src/conn/query_result.rs | 8 ++-- src/conn/stmt_cache.rs | 2 +- src/io/mod.rs | 10 +--- src/io/tls/native_tls_io.rs | 2 +- 14 files changed, 83 insertions(+), 110 deletions(-) diff --git a/build.rs b/build.rs index 7fd933a..2cf3029 100644 --- a/build.rs +++ b/build.rs @@ -11,8 +11,8 @@ use std::env; fn main() { let names = ["CARGO_CFG_TARGET_OS", "CARGO_CFG_TARGET_ARCH"]; for name in &names { - let value = - env::var(name).expect(&format!("Could not get the environment variable {}", name)); + let value = env::var(name) + .unwrap_or_else(|_| panic!("Could not get the environment variable {}", name)); println!("cargo:rustc-env={}={}", name, value); } } diff --git a/src/buffer_pool/enabled.rs b/src/buffer_pool/enabled.rs index 5d08d21..d2af5fd 100644 --- a/src/buffer_pool/enabled.rs +++ b/src/buffer_pool/enabled.rs @@ -3,12 +3,12 @@ use crossbeam::queue::ArrayQueue; use once_cell::sync::Lazy; -use std::{mem::replace, ops::Deref, sync::Arc}; +use std::{mem::take, ops::Deref, sync::Arc}; const DEFAULT_MYSQL_BUFFER_POOL_CAP: usize = 128; const DEFAULT_MYSQL_BUFFER_SIZE_CAP: usize = 4 * 1024 * 1024; -static BUFFER_POOL: Lazy> = Lazy::new(|| Default::default()); +static BUFFER_POOL: Lazy> = Lazy::new(Default::default); #[inline(always)] pub fn get_buffer() -> Buffer { @@ -97,7 +97,7 @@ impl Deref for Buffer { impl Drop for Buffer { fn drop(&mut self) { if let Some(ref inner) = self.1 { - inner.put(replace(&mut self.0, vec![])); + inner.put(take(&mut self.0)); } } } diff --git a/src/conn/binlog_stream.rs b/src/conn/binlog_stream.rs index 13d6739..b25b801 100644 --- a/src/conn/binlog_stream.rs +++ b/src/conn/binlog_stream.rs @@ -55,36 +55,34 @@ impl Iterator for BinlogStream { } }; - let first_byte = packet.get(0).copied(); + let first_byte = packet.first().copied(); if first_byte == Some(255) { - if let Ok(ErrPacket::Error(err)) = ParseBuf(&*packet).parse(conn.0.capability_flags) { + if let Ok(ErrPacket::Error(err)) = ParseBuf(&packet).parse(conn.0.capability_flags) { self.conn = None; return Some(Err(crate::Error::MySqlError(From::from(err)))); } } - if first_byte == Some(254) && packet.len() < 8 { - if ParseBuf(&*packet) + if first_byte == Some(254) + && packet.len() < 8 + && ParseBuf(&packet) .parse::>(conn.0.capability_flags) .is_ok() - { - self.conn = None; - return None; - } + { + self.conn = None; + return None; } if first_byte == Some(0) { let event_data = &packet[1..]; match self.esr.read(event_data) { - Ok(event) => { - return Some(Ok(event)); - } - Err(err) => return Some(Err(err.into())), + Ok(event) => Some(Ok(event)), + Err(err) => Some(Err(err.into())), } } else { self.conn = None; - return Some(Err(crate::error::DriverError::UnexpectedPacket.into())); + Some(Err(crate::error::DriverError::UnexpectedPacket.into())) } } } diff --git a/src/conn/local_infile.rs b/src/conn/local_infile.rs index 9975e2c..82580c8 100644 --- a/src/conn/local_infile.rs +++ b/src/conn/local_infile.rs @@ -78,7 +78,7 @@ impl LocalInfileHandler { impl PartialEq for LocalInfileHandler { fn eq(&self, other: &LocalInfileHandler) -> bool { - (&*self.0 as *const _) == (&*other.0 as *const _) + std::ptr::eq(&*self.0, &*other.0) } } diff --git a/src/conn/mod.rs b/src/conn/mod.rs index e6752a4..bde2f64 100644 --- a/src/conn/mod.rs +++ b/src/conn/mod.rs @@ -135,9 +135,9 @@ impl Deref for ConnMut<'_, '_, '_> { fn deref(&self) -> &Conn { match self { - ConnMut::Mut(conn) => &**conn, - ConnMut::TxMut(tx) => &*tx.conn, - ConnMut::Owned(conn) => &conn, + ConnMut::Mut(conn) => conn, + ConnMut::TxMut(tx) => &tx.conn, + ConnMut::Owned(conn) => conn, ConnMut::Pooled(conn) => conn.as_ref(), } } @@ -146,8 +146,8 @@ impl Deref for ConnMut<'_, '_, '_> { impl DerefMut for ConnMut<'_, '_, '_> { fn deref_mut(&mut self) -> &mut Conn { match self { - ConnMut::Mut(ref mut conn) => &mut **conn, - ConnMut::TxMut(tx) => &mut *tx.conn, + ConnMut::Mut(conn) => conn, + ConnMut::TxMut(tx) => &mut tx.conn, ConnMut::Owned(ref mut conn) => conn, ConnMut::Pooled(ref mut conn) => conn.as_mut(), } @@ -222,7 +222,7 @@ impl Conn { pub fn server_version(&self) -> (u16, u16, u16) { self.0 .server_version - .or_else(|| self.0.mariadb_server_version) + .or(self.0.mariadb_server_version) .unwrap() } @@ -477,7 +477,7 @@ impl Conn { let tcp_connect_timeout = opts.get_tcp_connect_timeout(); let bind_address = opts.bind_address().cloned(); let stream = if let Some(socket) = opts.get_socket() { - Stream::connect_socket(&*socket, read_timeout, write_timeout)? + Stream::connect_socket(socket, read_timeout, write_timeout)? } else { let port = opts.get_tcp_port(); let ip_or_hostname = match opts.get_host() { @@ -486,7 +486,7 @@ impl Conn { url::Host::Ipv6(ip) => ip.to_string(), }; Stream::connect_tcp( - &*ip_or_hostname, + &ip_or_hostname, port, read_timeout, write_timeout, @@ -519,7 +519,7 @@ impl Conn { let mut buffer = get_buffer(); match self.raw_read_packet(buffer.as_mut()) { Ok(()) if buffer.first() == Some(&0xff) => { - match ParseBuf(&*buffer).parse(self.0.capability_flags)? { + match ParseBuf(&buffer).parse(self.0.capability_flags)? { ErrPacket::Error(server_error) => { self.handle_err(); return Err(MySqlError(From::from(server_error))); @@ -567,7 +567,7 @@ impl Conn { &mut self, buffer: &'a Buffer, ) -> crate::Result> { - let ok = ParseBuf(&**buffer) + let ok = ParseBuf(buffer) .parse::>(self.0.capability_flags)? .into_inner(); self.0.status_flags = ok.status_flags(); @@ -591,19 +591,17 @@ impl Conn { if matches!( auth_switch_request.auth_plugin(), AuthPlugin::MysqlOldPassword - ) { - if self.0.opts.get_secure_auth() { - return Err(DriverError(OldMysqlPasswordDisabled)); - } + ) && self.0.opts.get_secure_auth() + { + return Err(DriverError(OldMysqlPasswordDisabled)); } if matches!( auth_switch_request.auth_plugin(), AuthPlugin::Other(Cow::Borrowed(b"mysql_clear_password")) - ) { - if !self.0.opts.get_enable_cleartext_plugin() { - return Err(DriverError(CleartextPluginDisabled)); - } + ) && !self.0.opts.get_enable_cleartext_plugin() + { + return Err(DriverError(CleartextPluginDisabled)); } self.0.nonce = auth_switch_request.plugin_data().to_vec(); @@ -644,7 +642,7 @@ impl Conn { fn do_handshake(&mut self) -> Result<()> { let payload = self.read_packet()?; - let handshake = ParseBuf(&*payload).parse::(())?; + let handshake = ParseBuf(&payload).parse::(())?; if handshake.protocol_version() != 10u8 { return Err(DriverError(UnsupportedProtocol( @@ -738,7 +736,7 @@ impl Conn { None => { let arg0 = std::env::args_os().next(); let arg0 = arg0.as_ref().map(|x| x.to_string_lossy()); - arg0.unwrap_or_else(|| "".into()).to_owned().to_string() + arg0.unwrap_or_else(|| "".into()).into_owned() } }; @@ -777,7 +775,7 @@ impl Conn { let auth_data = self .0 .auth_plugin - .gen_data(self.0.opts.get_pass(), &*self.0.nonce) + .gen_data(self.0.opts.get_pass(), &self.0.nonce) .map(|x| x.into_owned()); let handshake_response = HandshakeResponse::new( @@ -787,7 +785,7 @@ impl Conn { self.0.opts.get_db_name().map(str::as_bytes), Some(self.0.auth_plugin.clone()), self.0.capability_flags, - Some(self.connect_attrs().clone()), + Some(self.connect_attrs()), ); let mut buf = get_buffer(); @@ -828,9 +826,9 @@ impl Conn { // auth switch 0xfe if !auth_switched => { let auth_switch = if payload.len() > 1 { - ParseBuf(&*payload).parse(())? + ParseBuf(&payload).parse(())? } else { - let _ = ParseBuf(&*payload).parse::(())?; + let _ = ParseBuf(&payload).parse::(())?; // we'll map OldAuthSwitchRequest to an AuthSwitchRequest with mysql_old_password plugin. AuthSwitchRequest::new("mysql_old_password".as_bytes(), &*self.0.nonce) .into_owned() @@ -875,10 +873,10 @@ impl Conn { .map(Vec::from) .unwrap_or_else(Vec::new); pass.push(0); - for i in 0..pass.len() { - pass[i] ^= self.0.nonce[i % self.0.nonce.len()]; + for (i, c) in pass.iter_mut().enumerate() { + *(c) ^= self.0.nonce[i % self.0.nonce.len()]; } - let encrypted_pass = crypto::encrypt(&*pass, key); + let encrypted_pass = crypto::encrypt(&pass, key); self.write_packet(&mut encrypted_pass.as_slice())?; } @@ -888,7 +886,7 @@ impl Conn { _ => Err(DriverError(UnexpectedPacket)), }, 0xfe if !auth_switched => { - let auth_switch_request = ParseBuf(&*payload).parse(())?; + let auth_switch_request = ParseBuf(&payload).parse(())?; self.perform_auth_switch(auth_switch_request) } _ => Err(DriverError(UnexpectedPacket)), @@ -964,10 +962,10 @@ impl Conn { } let (body, as_long_data) = - ComStmtExecuteRequestBuilder::new(stmt.id()).build(&*params); + ComStmtExecuteRequestBuilder::new(stmt.id()).build(params); if as_long_data { - self.send_long_data(stmt.id(), &*params)?; + self.send_long_data(stmt.id(), params)?; } body @@ -1060,7 +1058,7 @@ impl Conn { let mut columns: Vec = Vec::with_capacity(column_count as usize); for _ in 0..column_count { let pld = self.read_packet()?; - let column = ParseBuf(&*pld).parse(())?; + let column = ParseBuf(&pld).parse(())?; columns.push(column); } // skip eof packet @@ -1104,12 +1102,12 @@ impl Conn { fn _true_prepare(&mut self, query: &[u8]) -> Result { self.write_command(Command::COM_STMT_PREPARE, query)?; let pld = self.read_packet()?; - let mut stmt = ParseBuf(&*pld).parse::(self.connection_id())?; + let mut stmt = ParseBuf(&pld).parse::(self.connection_id())?; if stmt.num_params() > 0 { let mut params: Vec = Vec::with_capacity(stmt.num_params() as usize); for _ in 0..stmt.num_params() { let pld = self.read_packet()?; - params.push(ParseBuf(&*pld).parse(())?); + params.push(ParseBuf(&pld).parse(())?); } stmt = stmt.with_params(Some(params)); self.drop_packet()?; @@ -1118,7 +1116,7 @@ impl Conn { let mut columns: Vec = Vec::with_capacity(stmt.num_columns() as usize); for _ in 0..stmt.num_columns() { let pld = self.read_packet()?; - columns.push(ParseBuf(&*pld).parse(())?); + columns.push(ParseBuf(&pld).parse(())?); } stmt = stmt.with_columns(Some(columns)); self.drop_packet()?; @@ -1183,12 +1181,10 @@ impl Conn { self.handle_ok::(&pld)?; return Ok(None); } - } else { - if pld[0] == 0xfe && pld.len() < 8 { - self.0.has_results = false; - self.handle_ok::(&pld)?; - return Ok(None); - } + } else if pld[0] == 0xfe && pld.len() < 8 { + self.0.has_results = false; + self.handle_ok::(&pld)?; + return Ok(None); } Ok(Some(pld)) @@ -1286,7 +1282,7 @@ impl Queryable for Conn { P: Into, { let statement = stmt.as_statement(self)?; - let meta = self._execute(&*statement, params.into())?; + let meta = self._execute(&statement, params.into())?; Ok(QueryResult::new(ConnMut::Mut(self), meta)) } } diff --git a/src/conn/opts/mod.rs b/src/conn/opts/mod.rs index c8eb913..eae3540 100644 --- a/src/conn/opts/mod.rs +++ b/src/conn/opts/mod.rs @@ -549,7 +549,7 @@ impl Opts { /// let connection_opts = mysql::Opts::from_url("mysql://root:password@localhost:3307/mysql?prefer_socket=false").unwrap(); /// let pool = mysql::Pool::new(connection_opts).unwrap(); /// ``` -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, Default)] pub struct OptsBuilder { opts: Opts, } @@ -1069,17 +1069,9 @@ impl From for Opts { } } -impl Default for OptsBuilder { - fn default() -> OptsBuilder { - OptsBuilder { - opts: Opts::default(), - } - } -} - fn get_opts_user_from_url(url: &Url) -> Option { let user = url.username(); - if user != "" { + if !user.is_empty() { Some( percent_decode(user.as_ref()) .decode_utf8_lossy() @@ -1091,15 +1083,11 @@ fn get_opts_user_from_url(url: &Url) -> Option { } fn get_opts_pass_from_url(url: &Url) -> Option { - if let Some(pass) = url.password() { - Some( - percent_decode(pass.as_ref()) - .decode_utf8_lossy() - .into_owned(), - ) - } else { - None - } + url.password().map(|pass| { + percent_decode(pass.as_ref()) + .decode_utf8_lossy() + .into_owned() + }) } fn get_opts_db_name_from_url(url: &Url) -> Option { diff --git a/src/conn/opts/native_tls_opts.rs b/src/conn/opts/native_tls_opts.rs index 0d31f2a..95e0fec 100644 --- a/src/conn/opts/native_tls_opts.rs +++ b/src/conn/opts/native_tls_opts.rs @@ -44,7 +44,7 @@ impl ClientIdentity { pub(crate) fn load(&self) -> crate::Result { let der = std::fs::read(self.pkcs12_path.as_ref())?; Ok(Identity::from_pkcs12( - &*der, + &der, self.password.as_deref().unwrap_or(""), )?) } diff --git a/src/conn/opts/pool_opts.rs b/src/conn/opts/pool_opts.rs index d204e56..4810081 100644 --- a/src/conn/opts/pool_opts.rs +++ b/src/conn/opts/pool_opts.rs @@ -147,6 +147,7 @@ impl Assert { #[allow(path_statements)] pub const fn gte() { + #[allow(clippy::no_effect)] Assert::::LEQ; } diff --git a/src/conn/pool/mod.rs b/src/conn/pool/mod.rs index 52e413c..2000bd7 100644 --- a/src/conn/pool/mod.rs +++ b/src/conn/pool/mod.rs @@ -71,16 +71,12 @@ impl Pool { fn _get_conn>( &self, stmt: Option, - timeout_ms: Option, + timeout: Option, mut call_ping: bool, ) -> Result { - let times = if let Some(timeout_ms) = timeout_ms { - Some((Instant::now(), Duration::from_millis(timeout_ms.into()))) - } else { - None - }; + let times = timeout.map(|timeout| (Instant::now(), timeout)); - let &(ref protected, ref condvar) = self.inner.protected(); + let (protected, condvar) = self.inner.protected(); let conn = if !self.inner.opts().reset_connection() { // stmt cache considered enabled if reset_connection is false @@ -122,7 +118,7 @@ impl Pool { if call_ping && self.inner.opts().check_health() && !conn.ping() { // existing connection seem to be dead, retrying.. self.inner.decrease(); - return self._get_conn(stmt, timeout_ms, call_ping); + return self._get_conn(stmt, timeout, call_ping); } Ok(PooledConn { @@ -147,13 +143,13 @@ impl Pool { self._get_conn(None::, None, true) } - /// Will try to get connection for a duration of `timeout_ms` milliseconds. + /// Will try to get connection for the duration of `timeout`. /// /// # Failure /// This function will return `Error::DriverError(DriverError::Timeout)` if timeout was /// reached while waiting for new connection to become available. - pub fn try_get_conn(&self, timeout_ms: u32) -> Result { - self._get_conn(None::, Some(timeout_ms), true) + pub fn try_get_conn(&self, timeout: Duration) -> Result { + self._get_conn(None::, Some(timeout), true) } /// Shortcut for `pool.get_conn()?.start_transaction(..)`. @@ -489,15 +485,15 @@ mod test { PoolOpts::default().with_constraints(PoolConstraints::new_const::<0, 1>()), )) .unwrap(); - let conn1 = pool.try_get_conn(357).unwrap(); - let conn2 = pool.try_get_conn(357); + let conn1 = pool.try_get_conn(Duration::from_millis(357)).unwrap(); + let conn2 = pool.try_get_conn(Duration::from_millis(357)); assert!(conn2.is_err()); match conn2 { Err(Error::DriverError(DriverError::Timeout)) => assert!(true), _ => assert!(false), } drop(conn1); - assert!(pool.try_get_conn(357).is_ok()); + assert!(pool.try_get_conn(Duration::from_millis(357)).is_ok()); } #[test] diff --git a/src/conn/query.rs b/src/conn/query.rs index ca0c906..17279d3 100644 --- a/src/conn/query.rs +++ b/src/conn/query.rs @@ -334,7 +334,7 @@ where { let mut conn = conn.try_into()?; let statement = self.query.as_statement(&mut *conn)?; - let meta = conn._execute(&*statement, self.params.into())?; + let meta = conn._execute(&statement, self.params.into())?; Ok(QueryResult::new(conn, meta)) } } @@ -382,7 +382,7 @@ where for params in self.params { let params = params.into(); - let meta = conn._execute(&*statement, params)?; + let meta = conn._execute(&statement, params)?; let mut query_result = QueryResult::::new((&mut *conn).into(), meta); while let Some(result_set) = query_result.iter() { for row in result_set { diff --git a/src/conn/query_result.rs b/src/conn/query_result.rs index 4c9c878..d5002b4 100644 --- a/src/conn/query_result.rs +++ b/src/conn/query_result.rs @@ -29,7 +29,7 @@ impl Protocol for Text { fn next(conn: &mut Conn, columns: Arc<[Column]>) -> Result> { match conn.next_row_packet()? { Some(pld) => { - let row = ParseBuf(&*pld).parse::>(columns)?; + let row = ParseBuf(&pld).parse::>(columns)?; Ok(Some(row.into())) } None => Ok(None), @@ -41,7 +41,7 @@ impl Protocol for Binary { fn next(conn: &mut Conn, columns: Arc<[Column]>) -> Result> { match conn.next_row_packet()? { Some(pld) => { - let row = ParseBuf(&*pld).parse::>(columns)?; + let row = ParseBuf(&pld).parse::>(columns)?; Ok(Some(row.into())) } None => Ok(None), @@ -341,9 +341,9 @@ impl Iterator for QueryResult<'_, '_, '_, T> { let state = std::mem::replace(&mut self.state, OnBoundary); match state { - InSet(cols) => match T::next(&mut *self.conn, cols.clone()) { + InSet(cols) => match T::next(&mut self.conn, cols.clone()) { Ok(Some(row)) => { - self.state = InSet(cols.clone()); + self.state = InSet(cols); Some(Ok(row)) } Ok(None) => { diff --git a/src/conn/stmt_cache.rs b/src/conn/stmt_cache.rs index ef08a28..e024649 100644 --- a/src/conn/stmt_cache.rs +++ b/src/conn/stmt_cache.rs @@ -23,7 +23,7 @@ pub struct QueryString(pub Arc>); impl Borrow<[u8]> for QueryString { fn borrow(&self) -> &[u8] { - &**self.0.as_ref() + self.0.as_ref() } } diff --git a/src/io/mod.rs b/src/io/mod.rs index d87908b..77703d8 100644 --- a/src/io/mod.rs +++ b/src/io/mod.rs @@ -135,17 +135,11 @@ impl Stream { } pub fn is_insecure(&self) -> bool { - match self { - Stream::TcpStream(TcpStream::Insecure(_)) => true, - _ => false, - } + matches!(self, Stream::TcpStream(TcpStream::Insecure(_))) } pub fn is_socket(&self) -> bool { - match self { - Stream::SocketStream(_) => true, - _ => false, - } + matches!(self, Stream::SocketStream(_)) } #[cfg(all(not(feature = "native-tls"), not(feature = "rustls")))] diff --git a/src/io/tls/native_tls_io.rs b/src/io/tls/native_tls_io.rs index 10f6fa9..6768bde 100644 --- a/src/io/tls/native_tls_io.rs +++ b/src/io/tls/native_tls_io.rs @@ -32,7 +32,7 @@ impl Stream { let mut root_cert_file = File::open(root_cert_path)?; root_cert_file.read_to_end(&mut root_cert_data)?; - let root_certs = Certificate::from_der(&*root_cert_data) + let root_certs = Certificate::from_der(&root_cert_data) .map(|x| vec![x]) .or_else(|_| { pem::parse_many(&*root_cert_data) From f051909754cb80d9332bb596270b85dfeca6ae93 Mon Sep 17 00:00:00 2001 From: Anatoly Ikorsky Date: Thu, 11 May 2023 17:23:27 +0300 Subject: [PATCH 06/13] Bump version --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 060a580..7604831 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mysql" -version = "23.0.1" +version = "24.0.0" authors = ["blackbeam"] description = "Mysql client library implemented in rust" license = "MIT/Apache-2.0" From 496ee7107ee11265f71e390330ab88440f891af2 Mon Sep 17 00:00:00 2001 From: Anatoly Ikorsky Date: Fri, 12 May 2023 17:28:36 +0300 Subject: [PATCH 07/13] Fix intra-docs links --- README.md | 4 ++-- src/conn/opts/pool_opts.rs | 2 +- src/lib.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 6c8abf6..19f3968 100644 --- a/README.md +++ b/README.md @@ -198,8 +198,8 @@ structure in the create API docs): * `stmt_cache_size: u32` - defines the value of the same field in the `Opts` structure; * `enable_cleartext_plugin` – see [`Opts::get_enable_cleartext_plugin`]; * `secure_auth` – see [`Opts::get_secure_auth`]; -* `reset_connection` – see [`PoolOpts::get_reset_connection`]; -* `check_health` – see [`PoolOpts::get_check_health`]; +* `reset_connection` – see [`PoolOpts::reset_connection`]; +* `check_health` – see [`PoolOpts::check_health`]; * `compress` - defines the value of the same field in the `Opts` structure. Supported value are: * `true` - enables compression with the default compression level; diff --git a/src/conn/opts/pool_opts.rs b/src/conn/opts/pool_opts.rs index 4810081..d283eb7 100644 --- a/src/conn/opts/pool_opts.rs +++ b/src/conn/opts/pool_opts.rs @@ -66,7 +66,7 @@ impl PoolOpts { /// So to increase overall performance you can safely opt-out of the default behavior /// if you are not willing to change the session state in an unpleasant way. /// - /// It is also possible to selectively opt-in/out using [`Conn::reset_connection`]. + /// It is also possible to selectively opt-in/out using [`crate::PooledConn::reset_connection`]. /// /// # Connection URL /// diff --git a/src/lib.rs b/src/lib.rs index 2482940..78669ab 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -203,8 +203,8 @@ //! * `stmt_cache_size: u32` - defines the value of the same field in the `Opts` structure; //! * `enable_cleartext_plugin` – see [`Opts::get_enable_cleartext_plugin`]; //! * `secure_auth` – see [`Opts::get_secure_auth`]; -//! * `reset_connection` – see [`PoolOpts::get_reset_connection`]; -//! * `check_health` – see [`PoolOpts::get_check_health`]; +//! * `reset_connection` – see [`PoolOpts::reset_connection`]; +//! * `check_health` – see [`PoolOpts::check_health`]; //! * `compress` - defines the value of the same field in the `Opts` structure. //! Supported value are: //! * `true` - enables compression with the default compression level; From 2b4d69c9002b60fc9a28786fd7f8981ca89a3929 Mon Sep 17 00:00:00 2001 From: Anatoly Ikorsky Date: Fri, 12 May 2023 17:28:54 +0300 Subject: [PATCH 08/13] Reexport derive macros --- Cargo.toml | 5 +++-- README.md | 6 ++++-- src/lib.rs | 15 ++++++++++----- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 7604831..286aa7b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,8 +36,8 @@ default = [ "mysql_common/rust_decimal", "mysql_common/time", "mysql_common/frunk", - "mysql_common/derive", + "derive", # use global buffer pool by default "buffer-pool", ] @@ -51,14 +51,15 @@ default-rustls = [ "mysql_common/rust_decimal", "mysql_common/time", "mysql_common/frunk", - "mysql_common/derive", + "derive", "buffer-pool", ] minimal = ["flate2/zlib"] rustls-tls = ["rustls", "webpki", "webpki-roots", "rustls-pemfile"] buffer-pool = [] nightly = [] +derive = ["mysql_common/derive"] [dev-dependencies] lazy_static = "1.4.0" diff --git a/README.md b/README.md index 19f3968..b82c8dc 100644 --- a/README.md +++ b/README.md @@ -110,9 +110,10 @@ fn main() -> std::result::Result<(), Box> { * feature sets: - * **default** – includes default `mysql_common` features, `native-tls`, `buffer-pool` - and `flate2/zlib` + * **default** – includes default `mysql_common` features, `native-tls`, `buffer-pool`, + `flate2/zlib` and `derive` * **default-rustls** - same as `default` but with `rustls-tls` instead of `native-tls` + and `flate2/rust_backend` instead of `flate2/zlib` * **minimal** - includes `flate2/zlib` * crate's features: @@ -123,6 +124,7 @@ fn main() -> std::result::Result<(), Box> { (see the [SSL Support](#ssl-support) section) * **buffer-pool** (enabled by default) – enables buffer pooling (see the [Buffer Pool](#buffer-pool) section) + * **derive** (enabled by default) – reexports derive macros under `prelude` * external features enabled by default: diff --git a/src/lib.rs b/src/lib.rs index 78669ab..ad0b7ce 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -112,9 +112,10 @@ //! //! * feature sets: //! -//! * **default** – includes default `mysql_common` features, `native-tls`, `buffer-pool` -//! and `flate2/zlib` +//! * **default** – includes default `mysql_common` features, `native-tls`, `buffer-pool`, +//! `flate2/zlib` and `derive` //! * **default-rustls** - same as `default` but with `rustls-tls` instead of `native-tls` +//! and `flate2/rust_backend` instead of `flate2/zlib` //! * **minimal** - includes `flate2/zlib` //! //! * crate's features: @@ -125,6 +126,7 @@ //! (see the [SSL Support](#ssl-support) section) //! * **buffer-pool** (enabled by default) – enables buffer pooling //! (see the [Buffer Pool](#buffer-pool) section) +//! * **derive** (enabled by default) – reexports derive macros under `prelude` //! //! * external features enabled by default: //! @@ -892,6 +894,9 @@ mod conn; pub mod error; mod io; +#[cfg(feature = "derive")] +extern crate mysql_common; + #[doc(inline)] pub use crate::myc::constants as consts; @@ -959,11 +964,11 @@ pub mod prelude { #[doc(inline)] pub use crate::conn::queryable::{AsStatement, Queryable}; #[doc(inline)] - pub use crate::myc::row::convert::FromRow; + pub use crate::myc::prelude::FromRow; #[doc(inline)] - pub use crate::myc::row::ColumnIndex; + pub use crate::myc::prelude::{FromValue, ToValue}; #[doc(inline)] - pub use crate::myc::value::convert::{FromValue, ToValue}; + pub use crate::myc::row::ColumnIndex; /// Trait for protocol markers [`crate::Binary`] and [`crate::Text`]. pub trait Protocol: crate::conn::query_result::Protocol {} From a06300669adeefe1d64bbc1ed5c4b39d0594fbf6 Mon Sep 17 00:00:00 2001 From: Anatoly Ikorsky Date: Fri, 12 May 2023 17:43:29 +0300 Subject: [PATCH 09/13] Update azure pipeline --- azure-pipelines.yml | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 1db42c8..f140d62 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -45,11 +45,11 @@ jobs: SSL=false COMPRESS=true cargo test SSL=true COMPRESS=true cargo test - SSL=true COMPRESS=false cargo test --no-default-features --features rustls-tls,flate2/zlib,mysql_common/time03,mysql_common/frunk - SSL=true COMPRESS=true cargo test --no-default-features --features rustls-tls,flate2/zlib,mysql_common/time03,mysql_common/frunk + SSL=true COMPRESS=false cargo test --no-default-features --features rustls-tls,flate2/zlib,mysql_common/time,mysql_common/frunk + SSL=true COMPRESS=true cargo test --no-default-features --features rustls-tls,flate2/zlib,mysql_common/time,mysql_common/frunk - SSL=false COMPRESS=true cargo test --no-default-features --features flate2/zlib,mysql_common/time03,mysql_common/frunk - SSL=false COMPRESS=false cargo test --no-default-features --features flate2/zlib,mysql_common/time03,mysql_common/frunk + SSL=false COMPRESS=true cargo test --no-default-features --features flate2/zlib,mysql_common/time,mysql_common/frunk + SSL=false COMPRESS=false cargo test --no-default-features --features flate2/zlib,mysql_common/time,mysql_common/frunk env: RUST_BACKTRACE: 1 DATABASE_URL: mysql://root:root@localhost:3306/mysql @@ -91,11 +91,11 @@ jobs: SSL=false COMPRESS=true cargo test SSL=true COMPRESS=true cargo test - SSL=true COMPRESS=false cargo test --no-default-features --features rustls-tls,flate2/zlib,mysql_common/time03,mysql_common/frunk - SSL=true COMPRESS=true cargo test --no-default-features --features rustls-tls,flate2/zlib,mysql_common/time03,mysql_common/frunk + SSL=true COMPRESS=false cargo test --no-default-features --features rustls-tls,flate2/zlib,mysql_common/time,mysql_common/frunk + SSL=true COMPRESS=true cargo test --no-default-features --features rustls-tls,flate2/zlib,mysql_common/time,mysql_common/frunk - SSL=false COMPRESS=true cargo test --no-default-features --features flate2/zlib,mysql_common/time03,mysql_common/frunk - SSL=false COMPRESS=false cargo test --no-default-features --features flate2/zlib,mysql_common/time03,mysql_common/frunk + SSL=false COMPRESS=true cargo test --no-default-features --features flate2/zlib,mysql_common/time,mysql_common/frunk + SSL=false COMPRESS=false cargo test --no-default-features --features flate2/zlib,mysql_common/time,mysql_common/frunk env: RUST_BACKTRACE: 1 DATABASE_URL: mysql://root@localhost/mysql @@ -142,11 +142,11 @@ jobs: SSL=false COMPRESS=true cargo test SSL=true COMPRESS=true cargo test - SSL=true COMPRESS=false cargo test --no-default-features --features rustls-tls,flate2/zlib,mysql_common/time03,mysql_common/frunk - SSL=true COMPRESS=true cargo test --no-default-features --features rustls-tls,flate2/zlib,mysql_common/time03,mysql_common/frunk + SSL=true COMPRESS=false cargo test --no-default-features --features rustls-tls,flate2/zlib,mysql_common/time,mysql_common/frunk + SSL=true COMPRESS=true cargo test --no-default-features --features rustls-tls,flate2/zlib,mysql_common/time,mysql_common/frunk - SSL=false COMPRESS=true cargo test --no-default-features --features flate2/zlib,mysql_common/time03,mysql_common/frunk - SSL=false COMPRESS=false cargo test --no-default-features --features flate2/zlib,mysql_common/time03,mysql_common/frunk + SSL=false COMPRESS=true cargo test --no-default-features --features flate2/zlib,mysql_common/time,mysql_common/frunk + SSL=false COMPRESS=false cargo test --no-default-features --features flate2/zlib,mysql_common/time,mysql_common/frunk env: RUST_BACKTRACE: 1 DATABASE_URL: mysql://root:password@localhost/mysql @@ -214,11 +214,11 @@ jobs: docker exec container bash -l -c "cd \$HOME && DATABASE_URL=$DATABASE_URL SSL=$SSL cargo test" docker exec container bash -l -c "cd \$HOME && DATABASE_URL=$DATABASE_URL SSL=$SSL COMPRESS=true cargo test" - docker exec container bash -l -c "cd \$HOME && DATABASE_URL=$DATABASE_URL SSL=true COMPRESS=false cargo test --no-default-features --features rustls-tls,flate2/zlib,mysql_common/time03,mysql_common/frunk" - docker exec container bash -l -c "cd \$HOME && DATABASE_URL=$DATABASE_URL SSL=true COMPRESS=true cargo test --no-default-features --features rustls-tls,flate2/zlib,mysql_common/time03,mysql_common/frunk" + docker exec container bash -l -c "cd \$HOME && DATABASE_URL=$DATABASE_URL SSL=true COMPRESS=false cargo test --no-default-features --features rustls-tls,flate2/zlib,mysql_common/time,mysql_common/frunk" + docker exec container bash -l -c "cd \$HOME && DATABASE_URL=$DATABASE_URL SSL=true COMPRESS=true cargo test --no-default-features --features rustls-tls,flate2/zlib,mysql_common/time,mysql_common/frunk" - docker exec container bash -l -c "cd \$HOME && DATABASE_URL=$DATABASE_URL SSL=false COMPRESS=true cargo test --no-default-features --features flate2/zlib,mysql_common/time03,mysql_common/frunk" - docker exec container bash -l -c "cd \$HOME && DATABASE_URL=$DATABASE_URL SSL=false COMPRESS=false cargo test --no-default-features --features flate2/zlib,mysql_common/time03,mysql_common/frunk" + docker exec container bash -l -c "cd \$HOME && DATABASE_URL=$DATABASE_URL SSL=false COMPRESS=true cargo test --no-default-features --features flate2/zlib,mysql_common/time,mysql_common/frunk" + docker exec container bash -l -c "cd \$HOME && DATABASE_URL=$DATABASE_URL SSL=false COMPRESS=false cargo test --no-default-features --features flate2/zlib,mysql_common/time,mysql_common/frunk" env: RUST_BACKTRACE: 1 DATABASE_URL: mysql://root:password@localhost/mysql @@ -286,11 +286,11 @@ jobs: docker exec container bash -l -c "cd \$HOME && DATABASE_URL=$DATABASE_URL SSL=true cargo test" docker exec container bash -l -c "cd \$HOME && DATABASE_URL=$DATABASE_URL SSL=true COMPRESS=true cargo test" - docker exec container bash -l -c "cd \$HOME && DATABASE_URL=$DATABASE_URL SSL=true COMPRESS=false cargo test --no-default-features --features rustls-tls,flate2/zlib,mysql_common/time03,mysql_common/frunk" - docker exec container bash -l -c "cd \$HOME && DATABASE_URL=$DATABASE_URL SSL=true COMPRESS=true cargo test --no-default-features --features rustls-tls,flate2/zlib,mysql_common/time03,mysql_common/frunk" + docker exec container bash -l -c "cd \$HOME && DATABASE_URL=$DATABASE_URL SSL=true COMPRESS=false cargo test --no-default-features --features rustls-tls,flate2/zlib,mysql_common/time,mysql_common/frunk" + docker exec container bash -l -c "cd \$HOME && DATABASE_URL=$DATABASE_URL SSL=true COMPRESS=true cargo test --no-default-features --features rustls-tls,flate2/zlib,mysql_common/time,mysql_common/frunk" - docker exec container bash -l -c "cd \$HOME && DATABASE_URL=$DATABASE_URL SSL=false COMPRESS=true cargo test --no-default-features --features flate2/zlib,mysql_common/time03,mysql_common/frunk" - docker exec container bash -l -c "cd \$HOME && DATABASE_URL=$DATABASE_URL SSL=false COMPRESS=false cargo test --no-default-features --features flate2/zlib,mysql_common/time03,mysql_common/frunk" + docker exec container bash -l -c "cd \$HOME && DATABASE_URL=$DATABASE_URL SSL=false COMPRESS=true cargo test --no-default-features --features flate2/zlib,mysql_common/time,mysql_common/frunk" + docker exec container bash -l -c "cd \$HOME && DATABASE_URL=$DATABASE_URL SSL=false COMPRESS=false cargo test --no-default-features --features flate2/zlib,mysql_common/time,mysql_common/frunk" env: RUST_BACKTRACE: 1 DATABASE_URL: mysql://root:password@localhost/mysql From d303d84da1ed93e66703df26d98d3cf71c67a2ea Mon Sep 17 00:00:00 2001 From: Anatoly Ikorsky Date: Mon, 15 May 2023 12:15:33 +0300 Subject: [PATCH 10/13] Fix reset_does_work test for older servers --- src/conn/mod.rs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/conn/mod.rs b/src/conn/mod.rs index bde2f64..41a61d3 100644 --- a/src/conn/mod.rs +++ b/src/conn/mod.rs @@ -1972,16 +1972,17 @@ mod test { fn reset_does_work() { let mut c = Conn::new(get_opts()).unwrap(); let cid = c.connection_id(); + c.query_drop("SET @foo = 'foo'").unwrap(); + assert_eq!( + c.query_first::("SELECT @foo").unwrap().unwrap(), + "foo", + ); c.reset().unwrap(); - match (c.0.server_version, c.0.mariadb_server_version) { - (Some(ref version), _) if *version > (5, 7, 3) => { - assert_eq!(cid, c.connection_id()); - } - (_, Some(ref version)) if *version >= (10, 2, 7) => { - assert_eq!(cid, c.connection_id()); - } - _ => assert_ne!(cid, c.connection_id()), - } + assert_eq!(cid, c.connection_id()); + assert_eq!( + c.query_first::("SELECT @foo").unwrap().unwrap(), + Value::NULL + ); } #[test] From d2ecb5b30c18d283814bcc6e65fff45866482686 Mon Sep 17 00:00:00 2001 From: Anatoly Ikorsky Date: Mon, 15 May 2023 12:17:48 +0300 Subject: [PATCH 11/13] ci: update macos image --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index f140d62..33d4944 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -57,7 +57,7 @@ jobs: - job: "TestBasicMacOs" pool: - vmImage: "macOS-10.15" + vmImage: "macOS-11" strategy: maxParallel: 10 matrix: From 5d8c5e128340f4c66b721432f0ea1095162ae533 Mon Sep 17 00:00:00 2001 From: Anatoly Ikorsky Date: Mon, 15 May 2023 12:36:07 +0300 Subject: [PATCH 12/13] ci: Update pipelines --- azure-pipelines.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 33d4944..04b97f1 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -201,9 +201,13 @@ jobs: displayName: Run MySql in Docker - bash: | docker exec container bash -l -c "mysql -uroot -ppassword -e \"SET old_passwords = 1; GRANT ALL PRIVILEGES ON *.* TO 'root2'@'%' IDENTIFIED WITH mysql_old_password AS 'password'; SET PASSWORD FOR 'root2'@'%' = OLD_PASSWORD('password')\""; + docker exec container bash -l -c "echo 'deb [trusted=yes] http://archive.debian.org/debian/ stretch main non-free contrib' > /etc/apt/sources.list" + docker exec container bash -l -c "echo 'deb-src [trusted=yes] http://archive.debian.org/debian/ stretch main non-free contrib ' >> /etc/apt/sources.list" + docker exec container bash -l -c "echo 'deb [trusted=yes] http://archive.debian.org/debian-security/ stretch/updates main non-free contrib' >> /etc/apt/sources.list" + docker exec container bash -l -c "echo 'deb [trusted=yes] http://repo.mysql.com/apt/debian/ stretch mysql-5.6' > /etc/apt/sources.list.d/mysql.list" condition: eq(variables['DB_VERSION'], '5.6') - bash: | - docker exec container bash -l -c "apt-get update" + docker exec container bash -l -c "apt-get --allow-unauthenticated -y update" docker exec container bash -l -c "apt-get install -y curl clang libssl-dev pkg-config build-essential" docker exec container bash -l -c "curl https://sh.rustup.rs -sSf | sh -s -- -y --default-toolchain stable" displayName: Install Rust in docker From 71a67d944569db2ce49e8fbd68c7817fe15e6c28 Mon Sep 17 00:00:00 2001 From: Anatoly Ikorsky Date: Wed, 17 May 2023 13:13:18 +0300 Subject: [PATCH 13/13] Workaround MySql Bug #92954 COM_CHANGE_USER does not work if CLIENT_CONNECT_ATTRS is set. This applies to MySql prior to 5.7 Changes: * client won't send CLIENT_CONNECT_ATTRS if `Opts::get_connect_attrs` returns `None` Affected APIs: * `OptsBuilder::connect_attrs` now takes `Option` - set to `None` to opt-out of connect attrs * `Opts::get_connect_attrs` now returns `Option` Affected APIs: --- src/conn/mod.rs | 77 ++++++++++++++++++++++++++------------------ src/conn/opts/mod.rs | 49 ++++++++++++++++++++-------- src/lib.rs | 3 +- 3 files changed, 83 insertions(+), 46 deletions(-) diff --git a/src/conn/mod.rs b/src/conn/mod.rs index 41a61d3..c009b53 100644 --- a/src/conn/mod.rs +++ b/src/conn/mod.rs @@ -392,13 +392,7 @@ impl Conn { UTF8_GENERAL_CI }) .with_auth_plugin(Some(self.0.auth_plugin.clone())) - .with_connect_attributes( - if self.0.opts.get_connect_attrs().is_empty() { - None - } else { - Some(self.0.opts.get_connect_attrs().clone()) - }, - ), + .with_connect_attributes(self.0.opts.get_connect_attrs().cloned()), )) .into_owned(); self.write_command_raw(&com_change_user)?; @@ -412,6 +406,11 @@ impl Conn { /// This function will try to invoke COM_RESET_CONNECTION with /// a fall back to COM_CHANGE_USER on older servers. /// + /// ## Warining + /// + /// There is a long-standing bug in mysql 5.6 that kills this functionality in presence + /// of connection attributes (see [Bug #92954](https://bugs.mysql.com/bug.php?id=92954)). + /// /// ## Note /// /// Re-executes [`Opts::get_init`]. @@ -448,6 +447,11 @@ impl Conn { /// * Using non-default `opts` for a pooled connection is discouraging. /// * Connection options will be updated permanently. /// + /// ## Warining + /// + /// There is a long-standing bug in mysql 5.6 that kills this functionality in presence + /// of connection attributes (see [Bug #92954](https://bugs.mysql.com/bug.php?id=92954)). + /// /// [1]: https://dev.mysql.com/doc/c-api/5.7/en/mysql-change-user.html pub fn change_user(&mut self, opts: ChangeUserOpts) -> Result<()> { self.exec_com_change_user(opts) @@ -714,11 +718,13 @@ impl Conn { | CapabilityFlags::CLIENT_MULTI_RESULTS | CapabilityFlags::CLIENT_PS_MULTI_RESULTS | CapabilityFlags::CLIENT_PLUGIN_AUTH - | CapabilityFlags::CLIENT_CONNECT_ATTRS | (self.0.capability_flags & CapabilityFlags::CLIENT_LONG_FLAG); if self.0.opts.get_compress().is_some() { client_flags.insert(CapabilityFlags::CLIENT_COMPRESS); } + if self.0.opts.get_connect_attrs().is_some() { + client_flags.insert(CapabilityFlags::CLIENT_CONNECT_ATTRS); + } if let Some(db_name) = self.0.opts.get_db_name() { if !db_name.is_empty() { client_flags.insert(CapabilityFlags::CLIENT_CONNECT_WITH_DB); @@ -730,30 +736,34 @@ impl Conn { client_flags | self.0.opts.get_additional_capabilities() } - fn connect_attrs(&self) -> HashMap { - let program_name = match self.0.opts.get_connect_attrs().get("program_name") { - Some(program_name) => program_name.clone(), - None => { - let arg0 = std::env::args_os().next(); - let arg0 = arg0.as_ref().map(|x| x.to_string_lossy()); - arg0.unwrap_or_else(|| "".into()).into_owned() - } - }; + fn connect_attrs(&self) -> Option> { + if let Some(attrs) = self.0.opts.get_connect_attrs() { + let program_name = match attrs.get("program_name") { + Some(program_name) => program_name.clone(), + None => { + let arg0 = std::env::args_os().next(); + let arg0 = arg0.as_ref().map(|x| x.to_string_lossy()); + arg0.unwrap_or_else(|| "".into()).into_owned() + } + }; - let mut attrs = HashMap::new(); + let mut attrs_to_send = HashMap::new(); - attrs.insert("_client_name".into(), "rust-mysql-simple".into()); - attrs.insert("_client_version".into(), env!("CARGO_PKG_VERSION").into()); - attrs.insert("_os".into(), env!("CARGO_CFG_TARGET_OS").into()); - attrs.insert("_pid".into(), process::id().to_string()); - attrs.insert("_platform".into(), env!("CARGO_CFG_TARGET_ARCH").into()); - attrs.insert("program_name".into(), program_name); + attrs_to_send.insert("_client_name".into(), "rust-mysql-simple".into()); + attrs_to_send.insert("_client_version".into(), env!("CARGO_PKG_VERSION").into()); + attrs_to_send.insert("_os".into(), env!("CARGO_CFG_TARGET_OS").into()); + attrs_to_send.insert("_pid".into(), process::id().to_string()); + attrs_to_send.insert("_platform".into(), env!("CARGO_CFG_TARGET_ARCH").into()); + attrs_to_send.insert("program_name".into(), program_name); - for (name, value) in self.0.opts.get_connect_attrs().clone() { - attrs.insert(name, value); - } + for (name, value) in attrs.clone() { + attrs_to_send.insert(name, value); + } - attrs + Some(attrs_to_send) + } else { + None + } } fn do_ssl_request(&mut self) -> Result<()> { @@ -785,7 +795,7 @@ impl Conn { self.0.opts.get_db_name().map(str::as_bytes), Some(self.0.auth_plugin.clone()), self.0.capability_flags, - Some(self.connect_attrs()), + self.connect_attrs(), ); let mut buf = get_buffer(); @@ -1889,6 +1899,9 @@ mod test { .unwrap(); }; + conn.query_drop("DELETE FROM mysql.user WHERE user = ''") + .unwrap(); + let _ = conn.query_drop("SET GLOBAL secure_auth = 0"); conn.query_drop("FLUSH PRIVILEGES").unwrap(); let mut conn2 = Conn::new(get_opts().secure_auth(false)).unwrap(); @@ -2340,7 +2353,9 @@ mod test { #[test] fn should_set_connect_attrs() { - let opts = OptsBuilder::from_opts(get_opts()); + let opts = OptsBuilder::from_opts( + get_opts().connect_attrs::(Some(Default::default())), + ); let mut conn = Conn::new(opts).unwrap(); let support_connect_attrs = match (conn.0.server_version, conn.0.mariadb_server_version) @@ -2401,7 +2416,7 @@ mod test { connect_attrs.insert("foo", "foo val"); connect_attrs.insert("bar", "bar val"); connect_attrs.insert("program_name", "my program name"); - let mut conn = Conn::new(opts.connect_attrs(connect_attrs)).unwrap(); + let mut conn = Conn::new(opts.connect_attrs(Some(connect_attrs))).unwrap(); expected_values.pop(); // remove program_name at the last expected_values.push(("foo", "foo val")); expected_values.push(("bar", "bar val")); diff --git a/src/conn/opts/mod.rs b/src/conn/opts/mod.rs index eae3540..6835b5c 100644 --- a/src/conn/opts/mod.rs +++ b/src/conn/opts/mod.rs @@ -214,7 +214,7 @@ pub(crate) struct InnerOpts { additional_capabilities: CapabilityFlags, /// Connect attributes - connect_attrs: HashMap, + connect_attrs: Option>, /// Disables `mysql_old_password` plugin (defaults to `true`). /// @@ -266,7 +266,7 @@ impl Default for InnerOpts { stmt_cache_size: DEFAULT_STMT_CACHE_SIZE, compress: None, additional_capabilities: CapabilityFlags::empty(), - connect_attrs: HashMap::new(), + connect_attrs: Some(HashMap::new()), secure_auth: true, enable_cleartext_plugin: false, #[cfg(test)] @@ -450,7 +450,7 @@ impl Opts { self.0.additional_capabilities } - /// Connect attributes + /// Connect attributes (the default connect attributes are sent by default). /// /// This value is sent to the server as custom name-value attributes. /// You can see them from performance_schema tables: [`session_account_connect_attrs` @@ -464,10 +464,18 @@ impl Opts { /// /// ### Note /// + /// - set `connect_attrs` to `None` to completely remove connect attributes + /// - set `connect_attrs` to an empty map to send only the default attributes + /// + /// #### Warning + /// + /// > There is a bug in MySql 5.6 that kills COM_CHANGE_USER in the presence of connection + /// > attributes so it's better to stick to `None` for mysql < 5.7. + /// /// Attribute names that begin with an underscore (`_`) are not set by /// application programs because they are reserved for internal use. /// - /// The following attributes are sent in addition to ones set by programs. + /// The following default attributes are sent in addition to ones set by programs. /// /// name | value /// ----------------|-------------------------- @@ -482,8 +490,8 @@ impl Opts { /// [`performance_schema`]: https://dev.mysql.com/doc/refman/8.0/en/performance-schema-system-variables.html#sysvar_performance_schema /// [`performance_schema_session_connect_attrs_size`]: https://dev.mysql.com/doc/refman/en/performance-schema-system-variables.html#sysvar_performance_schema_session_connect_attrs_size /// - pub fn get_connect_attrs(&self) -> &HashMap { - &self.0.connect_attrs + pub fn get_connect_attrs(&self) -> Option<&HashMap> { + self.0.connect_attrs.as_ref() } /// Disables `mysql_old_password` plugin (defaults to `true`). @@ -982,7 +990,7 @@ impl OptsBuilder { self } - /// Connect attributes + /// Connect attributes (the default connect attributes are sent by default). /// /// This value is sent to the server as custom name-value attributes. /// You can see them from performance_schema tables: [`session_account_connect_attrs` @@ -996,10 +1004,18 @@ impl OptsBuilder { /// /// ### Note /// + /// - set `connect_attrs` to `None` to completely remove connect attributes + /// - set `connect_attrs` to an empty map to send only the default attributes + /// + /// #### Warning + /// + /// > There is a bug in MySql 5.6 that kills COM_CHANGE_USER in the presence of connection + /// > attributes so it's better to stick to `None` for mysql < 5.7. + /// /// Attribute names that begin with an underscore (`_`) are not set by /// application programs because they are reserved for internal use. /// - /// The following attributes are sent in addition to ones set by programs. + /// The following default attributes are sent in addition to ones set by programs. /// /// name | value /// ----------------|-------------------------- @@ -1016,14 +1032,19 @@ impl OptsBuilder { /// pub fn connect_attrs + Eq + Hash, T2: Into>( mut self, - connect_attrs: HashMap, + connect_attrs: Option>, ) -> Self { - self.opts.0.connect_attrs = HashMap::with_capacity(connect_attrs.len()); - for (name, value) in connect_attrs { - let name = name.into(); - if !name.starts_with('_') { - self.opts.0.connect_attrs.insert(name, value.into()); + if let Some(connect_attrs) = connect_attrs { + let mut attrs = HashMap::with_capacity(connect_attrs.len()); + for (name, value) in connect_attrs { + let name = name.into(); + if !name.starts_with('_') { + attrs.insert(name, value.into()); + } } + self.opts.0.connect_attrs = Some(attrs); + } else { + self.opts.0.connect_attrs = None; } self } diff --git a/src/lib.rs b/src/lib.rs index ad0b7ce..494ac1e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1017,7 +1017,8 @@ macro_rules! def_get_opts { let database_url = $crate::def_database_url!(); let mut builder = $crate::OptsBuilder::from_opts($crate::Opts::from_url(&*database_url).unwrap()) - .init(vec!["SET GLOBAL sql_mode = 'TRADITIONAL'"]); + .init(vec!["SET GLOBAL sql_mode = 'TRADITIONAL'"]) + .connect_attrs::(None); if test_compression() { builder = builder.compress(Some(Default::default())); }