Skip to content

Commit

Permalink
Connect to a database in MysqlConnection::establish
Browse files Browse the repository at this point in the history
MySQL does handle DNS resolution, but doesn't handle URL parsing for us.
However, other than the `database` field, the arguments to
`mysql_real_connect` are basically just URI fragments, so I don't think
this warrants moving away from always taking a connection URL.

I have never felt less safe using unsafe than calling a function called
`mysql_real_connect`.
  • Loading branch information
sgrif committed Feb 4, 2017
1 parent 1e4faa2 commit 2c41bf9
Show file tree
Hide file tree
Showing 6 changed files with 232 additions and 5 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@ All user visible changes to this project will be documented in this file.
This project adheres to [Semantic Versioning](http://semver.org/), as described
for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/text/1105-api-evolution.md)

## Unreleased

### Changed

* It is no longer possible to exhaustively match against
`result::ConnectionError`.

## [0.10.0] - 2017-02-02

### Added
Expand Down
3 changes: 2 additions & 1 deletion diesel/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pq-sys = { version = "^0.2.0", optional = true }
quickcheck = { version = "0.3.1", optional = true }
serde_json = { version = ">=0.8.0, <0.10.0", optional = true }
uuid = { version = ">=0.2.0, <0.4.0", optional = true, features = ["use_std"] }
url = { version = "1.4.0", optional = true }

[dev-dependencies]
cfg-if = "0.1.0"
Expand All @@ -35,5 +36,5 @@ large-tables = []
huge-tables = ["large-tables"]
postgres = ["pq-sys"]
sqlite = ["libsqlite3-sys"]
mysql = ["mysqlclient-sys"]
mysql = ["mysqlclient-sys", "url"]
with-deprecated = []
18 changes: 14 additions & 4 deletions diesel/src/mysql/connection/mod.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
extern crate mysqlclient_sys as ffi;
mod raw;
mod url;

use connection::{Connection, SimpleConnection};
use query_builder::*;
use query_source::Queryable;
use result::*;
use self::raw::RawConnection;
use self::url::ConnectionOptions;
use super::backend::Mysql;
use types::HasSqlType;

#[allow(missing_debug_implementations, missing_copy_implementations)]
pub struct MysqlConnection;
pub struct MysqlConnection {
_raw_connection: RawConnection,
}

impl SimpleConnection for MysqlConnection {
fn batch_execute(&self, _query: &str) -> QueryResult<()> {
Expand All @@ -19,8 +24,13 @@ impl SimpleConnection for MysqlConnection {
impl Connection for MysqlConnection {
type Backend = Mysql;

fn establish(_database_url: &str) -> ConnectionResult<Self> {
unimplemented!()
fn establish(database_url: &str) -> ConnectionResult<Self> {
let raw_connection = RawConnection::new();
let connection_options = try!(ConnectionOptions::parse(database_url));
try!(raw_connection.connect(connection_options));
Ok(MysqlConnection {
_raw_connection: raw_connection,
})
}

fn execute(&self, _query: &str) -> QueryResult<usize> {
Expand Down
103 changes: 103 additions & 0 deletions diesel/src/mysql/connection/raw.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
extern crate mysqlclient_sys as ffi;

use std::ffi::CStr;
use std::os::{raw as libc};
use std::ptr;
use std::sync::{Once, ONCE_INIT};

use result::{ConnectionResult, ConnectionError};
use super::url::ConnectionOptions;

pub struct RawConnection(*mut ffi::MYSQL);

impl RawConnection {
pub fn new() -> Self {
perform_thread_unsafe_library_initialization();
let raw_connection = unsafe { ffi::mysql_init(ptr::null_mut()) };
if raw_connection.is_null() {
// We're trusting https://dev.mysql.com/doc/refman/5.7/en/mysql-init.html
// that null return always means OOM
panic!("Insufficient memory to allocate connection");
}
let result = RawConnection(raw_connection);

// This is only non-zero for unrecognized options, which should never happen.
let charset_result = unsafe { ffi::mysql_options(
result.0,
ffi::mysql_option::MYSQL_SET_CHARSET_NAME,
b"utf8mb4\0".as_ptr() as *const libc::c_void,
) };
assert_eq!(0, charset_result, "MYSQL_SET_CHARSET_NAME was not \
recognized as an option by MySQL. This should never \
happen.");

result
}

pub fn connect(&self, connection_options: ConnectionOptions) -> ConnectionResult<()> {
let host = try!(connection_options.host());
let user = try!(connection_options.user());
let password = try!(connection_options.password());
let database = try!(connection_options.database());
let port = connection_options.port();

unsafe {
// Make sure you don't use the fake one!
ffi::mysql_real_connect(
self.0,
host.map(|x| x.as_ptr()).unwrap_or(ptr::null_mut()),
user.as_ptr(),
password.map(|x| x.as_ptr()).unwrap_or(ptr::null_mut()),
database.map(|x| x.as_ptr()).unwrap_or(ptr::null_mut()),
port.unwrap_or(0) as u32,
ptr::null_mut(),
0,
)
};

let last_error_message = self.last_error_message();
if last_error_message.is_empty() {
Ok(())
} else {
Err(ConnectionError::BadConnection(last_error_message))
}
}

pub fn last_error_message(&self) -> String {
unsafe { CStr::from_ptr(ffi::mysql_error(self.0)) }
.to_string_lossy()
.into_owned()
}
}

impl Drop for RawConnection {
fn drop(&mut self) {
unsafe { ffi::mysql_close(self.0); }
}
}

/// In a nonmulti-threaded environment, mysql_init() invokes mysql_library_init() automatically as
/// necessary. However, mysql_library_init() is not thread-safe in a multi-threaded environment,
/// and thus neither is mysql_init(). Before calling mysql_init(), either call mysql_library_init()
/// prior to spawning any threads, or use a mutex to protect the mysql_library_init() call. This
/// should be done prior to any other client library call.
///
/// https://dev.mysql.com/doc/refman/5.7/en/mysql-init.html
static MYSQL_THREAD_UNSAFE_INIT: Once = ONCE_INIT;

fn perform_thread_unsafe_library_initialization() {
MYSQL_THREAD_UNSAFE_INIT.call_once(|| {
// mysql_library_init is defined by `#define mysql_library_init mysql_server_init`
// which isn't picked up by bindgen
let error_code = unsafe { ffi::mysql_server_init(0, ptr::null_mut(), ptr::null_mut()) };
if error_code != 0 {
// FIXME: This is documented as Nonzero if an error occurred.
// Presumably the value has some sort of meaning that we should
// reflect in this message. We are going to panic instead of return
// an error here, since the documentation does not indicate whether
// it is safe to call this function twice if the first call failed,
// so I will assume it is not.
panic!("Unable to perform MySQL global initialization");
}
})
}
99 changes: 99 additions & 0 deletions diesel/src/mysql/connection/url.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
extern crate url;

use std::ffi::{CString, NulError};
use self::url::Url;

use result::{ConnectionResult, ConnectionError};

pub struct ConnectionOptions {
url: Url,
}

impl ConnectionOptions {
pub fn parse(database_url: &str) -> ConnectionResult<Self> {
let url = match Url::parse(database_url) {
Ok(url) => url,
Err(_) => return Err(connection_url_error())
};

if url.scheme() != "mysql" {
return Err(connection_url_error());
}

if url.path_segments().map(|x| x.count()).unwrap_or(0) > 1 {
return Err(connection_url_error());
}

Ok(ConnectionOptions {
url: url,
})
}

pub fn host(&self) -> Result<Option<CString>, NulError> {
match self.url.host_str() {
Some(host) => CString::new(host.as_bytes()).map(Some),
None => Ok(None),
}
}

pub fn user(&self) -> Result<CString, NulError> {
CString::new(self.url.username().as_bytes())
}

pub fn password(&self) -> Result<Option<CString>, NulError> {
match self.url.password() {
Some(pw) => CString::new(pw.as_bytes()).map(Some),
None => Ok(None),
}
}

pub fn database(&self) -> Result<Option<CString>, NulError> {
match self.url.path_segments().and_then(|mut iter| iter.nth(0)) {
Some("") | None => Ok(None),
Some(segment) => CString::new(segment.as_bytes()).map(Some),
}
}

pub fn port(&self) -> Option<u16> {
self.url.port()
}
}

fn connection_url_error() -> ConnectionError {
let msg = "MySQL connection URLs must be in the form \
`mysql://[[user]:[password]@]host[:port][/database]`";
ConnectionError::InvalidConnectionUrl(msg.into())
}

#[test]
fn urls_with_schemes_other_than_mysql_are_errors() {
assert!(ConnectionOptions::parse("postgres://localhost").is_err());
assert!(ConnectionOptions::parse("http://localhost").is_err());
assert!(ConnectionOptions::parse("file:///tmp/mysql.sock").is_err());
assert!(ConnectionOptions::parse("socket:///tmp/mysql.sock").is_err());
assert!(ConnectionOptions::parse("mysql://localhost").is_ok());
}

#[test]
fn urls_must_have_zero_or_one_path_segments() {
assert!(ConnectionOptions::parse("mysql://localhost/foo/bar").is_err());
assert!(ConnectionOptions::parse("mysql://localhost/foo").is_ok());
}

#[test]
fn first_path_segment_is_treated_as_database() {
let foo_cstr = CString::new("foo".as_bytes()).unwrap();
let bar_cstr = CString::new("bar".as_bytes()).unwrap();
assert_eq!(
Ok(Some(foo_cstr)),
ConnectionOptions::parse("mysql://localhost/foo").unwrap().database()
);
assert_eq!(
Ok(Some(bar_cstr)),
ConnectionOptions::parse("mysql://localhost/bar").unwrap().database()
);
assert_eq!(
Ok(None),
ConnectionOptions::parse("mysql://localhost").unwrap().database()
);
}
7 changes: 7 additions & 0 deletions diesel/src/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,9 @@ impl DatabaseErrorInformation for String {
pub enum ConnectionError {
InvalidCString(NulError),
BadConnection(String),
InvalidConnectionUrl(String),
#[doc(hidden)]
__Nonexhaustive, // Match against _ instead, more variants may be added in the future
}

#[derive(Debug, PartialEq)]
Expand Down Expand Up @@ -148,6 +151,8 @@ impl Display for ConnectionError {
match self {
&ConnectionError::InvalidCString(ref nul_err) => nul_err.fmt(f),
&ConnectionError::BadConnection(ref s) => write!(f, "{}", &s),
&ConnectionError::InvalidConnectionUrl(ref s) => write!(f, "{}", &s),
&ConnectionError::__Nonexhaustive => unreachable!(),
}
}
}
Expand All @@ -157,6 +162,8 @@ impl StdError for ConnectionError {
match self {
&ConnectionError::InvalidCString(ref nul_err) => nul_err.description(),
&ConnectionError::BadConnection(ref s) => &s,
&ConnectionError::InvalidConnectionUrl(ref s) => &s,
&ConnectionError::__Nonexhaustive => unreachable!(),
}
}
}
Expand Down

0 comments on commit 2c41bf9

Please sign in to comment.