Merge pull request #182 from weiznich/fix/cast_truncations

Enable a few more lints regarding truncations in numerical casts

Author: weiznich

CHANGELOG.md

@@ -6,7 +6,10 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/
 
 ## [Unreleased]
 
+## [0.5.1] - 2024-11-01
+
 * Add crate feature `pool` for extending connection pool implements through external crate
+* Implement `Deref` and `DerefMut` for `AsyncConnectionWrapper` to allow using it in an async context as well
 
 ## [0.5.0] - 2024-07-19
 
@@ -79,4 +82,5 @@ in the pool should be checked if they are still valid
 [0.4.0]: https://github.com/weiznich/diesel_async/compare/v0.3.2...v0.4.0
 [0.4.1]: https://github.com/weiznich/diesel_async/compare/v0.4.0...v0.4.1
 [0.5.0]: https://github.com/weiznich/diesel_async/compare/v0.4.0...v0.5.0
-[Unreleased]: https://github.com/weiznich/diesel_async/compare/v0.5.0...main
+[0.5.1]: https://github.com/weiznich/diesel_async/compare/v0.5.0...v0.5.1
+[Unreleased]: https://github.com/weiznich/diesel_async/compare/v0.5.1...main

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "diesel-async"
-version = "0.5.0"
+version = "0.5.1"
 authors = ["Georg Semmler <[email protected]>"]
 edition = "2021"
 autotests = false

src/lib.rs

@@ -66,7 +66,12 @@
 //! # }
 //! ```
 
-#![warn(missing_docs)]
+#![warn(
+    missing_docs,
+    clippy::cast_possible_wrap,
+    clippy::cast_possible_truncation,
+    clippy::cast_sign_loss
+)]
 
 use diesel::backend::Backend;
 use diesel::connection::Instrumentation;

src/mysql/mod.rs

@@ -150,7 +150,8 @@ impl AsyncConnection for AsyncMysqlConnection {
             + 'query,
     {
         self.with_prepared_statement(source, |conn, stmt, binds| async move {
-            conn.exec_drop(&*stmt, binds).await.map_err(ErrorHelper)?;
+            let params = mysql_async::Params::try_from(binds)?;
+            conn.exec_drop(&*stmt, params).await.map_err(ErrorHelper)?;
             // We need to close any non-cached statement explicitly here as otherwise
             // we might error out on too many open statements. See https://github.com/weiznich/diesel_async/issues/26
             // for details
@@ -165,7 +166,9 @@ impl AsyncConnection for AsyncMysqlConnection {
             if let MaybeCached::CannotCache(stmt) = stmt {
                 conn.close(stmt).await.map_err(ErrorHelper)?;
             }
-            Ok(conn.affected_rows() as usize)
+            conn.affected_rows()
+                .try_into()
+                .map_err(|e| diesel::result::Error::DeserializationError(Box::new(e)))
         })
     }
 
@@ -325,8 +328,10 @@ impl AsyncMysqlConnection {
         mut tx: futures_channel::mpsc::Sender<QueryResult<MysqlRow>>,
     ) -> QueryResult<()> {
         use futures_util::sink::SinkExt;
+        let params = mysql_async::Params::try_from(binds)?;
+
         let res = conn
-            .exec_iter(stmt_for_exec, binds)
+            .exec_iter(stmt_for_exec, params)
             .await
             .map_err(ErrorHelper)?;
 

src/mysql/row.rs

@@ -99,7 +99,12 @@ impl<'a> diesel::row::Row<'a, Mysql> for MysqlRow {
                 Some(Cow::Owned(buffer))
             }
             _t => {
-                let mut buffer = Vec::with_capacity(value.bin_len() as usize);
+                let mut buffer = Vec::with_capacity(
+                    value
+                        .bin_len()
+                        .try_into()
+                        .expect("Failed to cast byte size to usize"),
+                );
                 mysql_common::proto::MySerialize::serialize(value, &mut buffer);
                 Some(Cow::Owned(buffer))
             }

src/mysql/serialize.rs

@@ -1,6 +1,7 @@
 use diesel::mysql::data_types::MysqlTime;
 use diesel::mysql::MysqlType;
 use diesel::mysql::MysqlValue;
+use diesel::QueryResult;
 use mysql_async::{Params, Value};
 use std::convert::TryInto;
 
@@ -9,10 +10,11 @@ pub(super) struct ToSqlHelper {
     pub(super) binds: Vec<Option<Vec<u8>>>,
 }
 
-fn to_value((metadata, bind): (MysqlType, Option<Vec<u8>>)) -> Value {
-    match bind {
+fn to_value((metadata, bind): (MysqlType, Option<Vec<u8>>)) -> QueryResult<Value> {
+    let cast_helper = |e| diesel::result::Error::SerializationError(Box::new(e));
+    let v = match bind {
         Some(bind) => match metadata {
-            MysqlType::Tiny => Value::Int((bind[0] as i8) as i64),
+            MysqlType::Tiny => Value::Int(i8::from_be_bytes([bind[0]]) as i64),
             MysqlType::Short => Value::Int(i16::from_ne_bytes(bind.try_into().unwrap()) as _),
             MysqlType::Long => Value::Int(i32::from_ne_bytes(bind.try_into().unwrap()) as _),
             MysqlType::LongLong => Value::Int(i64::from_ne_bytes(bind.try_into().unwrap())),
@@ -38,11 +40,11 @@ fn to_value((metadata, bind): (MysqlType, Option<Vec<u8>>)) -> Value {
                 .expect("This does not fail");
                 Value::Time(
                     time.neg,
-                    time.day as _,
-                    time.hour as _,
-                    time.minute as _,
-                    time.second as _,
-                    time.second_part as _,
+                    time.day,
+                    time.hour.try_into().map_err(cast_helper)?,
+                    time.minute.try_into().map_err(cast_helper)?,
+                    time.second.try_into().map_err(cast_helper)?,
+                    time.second_part.try_into().expect("Cast does not fail"),
                 )
             }
             MysqlType::Date | MysqlType::DateTime | MysqlType::Timestamp => {
@@ -52,13 +54,13 @@ fn to_value((metadata, bind): (MysqlType, Option<Vec<u8>>)) -> Value {
                 >::from_sql(MysqlValue::new(&bind, metadata))
                 .expect("This does not fail");
                 Value::Date(
-                    time.year as _,
-                    time.month as _,
-                    time.day as _,
-                    time.hour as _,
-                    time.minute as _,
-                    time.second as _,
-                    time.second_part as _,
+                    time.year.try_into().map_err(cast_helper)?,
+                    time.month.try_into().map_err(cast_helper)?,
+                    time.day.try_into().map_err(cast_helper)?,
+                    time.hour.try_into().map_err(cast_helper)?,
+                    time.minute.try_into().map_err(cast_helper)?,
+                    time.second.try_into().map_err(cast_helper)?,
+                    time.second_part.try_into().expect("Cast does not fail"),
                 )
             }
             MysqlType::Numeric
@@ -70,12 +72,19 @@ fn to_value((metadata, bind): (MysqlType, Option<Vec<u8>>)) -> Value {
             _ => unreachable!(),
         },
         None => Value::NULL,
-    }
+    };
+    Ok(v)
 }
 
-impl From<ToSqlHelper> for Params {
-    fn from(ToSqlHelper { metadata, binds }: ToSqlHelper) -> Self {
-        let values = metadata.into_iter().zip(binds).map(to_value).collect();
-        Params::Positional(values)
+impl TryFrom<ToSqlHelper> for Params {
+    type Error = diesel::result::Error;
+
+    fn try_from(ToSqlHelper { metadata, binds }: ToSqlHelper) -> Result<Self, Self::Error> {
+        let values = metadata
+            .into_iter()
+            .zip(binds)
+            .map(to_value)
+            .collect::<Result<Vec<_>, Self::Error>>()?;
+        Ok(Params::Positional(values))
     }
 }

src/pg/error_helper.rs

@@ -81,9 +81,9 @@ impl diesel::result::DatabaseErrorInformation for PostgresDbErrorWrapper {
 
     fn statement_position(&self) -> Option<i32> {
         use tokio_postgres::error::ErrorPosition;
-        self.0.position().map(|e| match e {
+        self.0.position().and_then(|e| match *e {
             ErrorPosition::Original(position) | ErrorPosition::Internal { position, .. } => {
-                *position as i32
+                position.try_into().ok()
             }
         })
     }

src/pg/mod.rs

@@ -274,7 +274,8 @@ async fn execute_prepared(
     let res = tokio_postgres::Client::execute(&conn, &stmt, &binds as &[_])
         .await
         .map_err(ErrorHelper)?;
-    Ok(res as usize)
+    res.try_into()
+        .map_err(|e| diesel::result::Error::DeserializationError(Box::new(e)))
 }
 
 #[inline(always)]

tests/instrumentation.rs

@@ -116,7 +116,7 @@ async fn check_events_are_emitted_for_load() {
 async fn check_events_are_emitted_for_execute_returning_count_does_not_contain_cache_for_uncached_queries(
 ) {
     let (events_to_check, mut conn) = setup_test_case().await;
-    conn.execute_returning_count(&diesel::sql_query("select 1"))
+    conn.execute_returning_count(diesel::sql_query("select 1"))
         .await
         .unwrap();
     let events = events_to_check.lock().unwrap();
@@ -141,7 +141,7 @@ async fn check_events_are_emitted_for_load_does_not_contain_cache_for_uncached_q
 async fn check_events_are_emitted_for_execute_returning_count_does_contain_error_for_failures() {
     let (events_to_check, mut conn) = setup_test_case().await;
     let _ = conn
-        .execute_returning_count(&diesel::sql_query("invalid"))
+        .execute_returning_count(diesel::sql_query("invalid"))
         .await;
     let events = events_to_check.lock().unwrap();
     assert_eq!(events.len(), 2, "{:?}", events);

tests/pooling.rs

@@ -17,7 +17,7 @@ async fn save_changes_bb8() {
 
     let mut conn = pool.get().await.unwrap();
 
-    super::setup(&mut *conn).await;
+    super::setup(&mut conn).await;
 
     diesel::insert_into(users::table)
         .values(users::name.eq("John"))
@@ -51,7 +51,7 @@ async fn save_changes_deadpool() {
 
     let mut conn = pool.get().await.unwrap();
 
-    super::setup(&mut *conn).await;
+    super::setup(&mut conn).await;
 
     diesel::insert_into(users::table)
         .values(users::name.eq("John"))
@@ -85,7 +85,7 @@ async fn save_changes_mobc() {
 
     let mut conn = pool.get().await.unwrap();
 
-    super::setup(&mut *conn).await;
+    super::setup(&mut conn).await;
 
     diesel::insert_into(users::table)
         .values(users::name.eq("John"))

tests/type_check.rs

@@ -200,8 +200,8 @@ async fn test_datetime() {
     type_check::<_, sql_types::Datetime>(
         conn,
         chrono::NaiveDateTime::new(
-            chrono::NaiveDate::from_ymd_opt(2021, 09, 30).unwrap(),
-            chrono::NaiveTime::from_hms_milli_opt(12, 06, 42, 0).unwrap(),
+            chrono::NaiveDate::from_ymd_opt(2021, 9, 30).unwrap(),
+            chrono::NaiveTime::from_hms_milli_opt(12, 6, 42, 0).unwrap(),
         ),
     )
     .await;