Skip to content

Commit

Permalink
Enable a few more lints regarding truncations in numerical casts
Browse files Browse the repository at this point in the history
This is similar to diesel-rs/diesel#4170, it's
just not a serve as the diesel change as we do not found any critical
cast here. I also investigated the implementation in the postgres crate
and it seems to be fine as well (i.e error on too large buffer sizes
instead silently truncating)
  • Loading branch information
weiznich committed Aug 23, 2024
1 parent dbdcbc2 commit f714fdc
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 28 deletions.
7 changes: 6 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
11 changes: 8 additions & 3 deletions src/mysql/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)))
})
}

Expand Down Expand Up @@ -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)?;

Expand Down
7 changes: 6 additions & 1 deletion src/mysql/row.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
49 changes: 29 additions & 20 deletions src/mysql/serialize.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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())),
Expand All @@ -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().map_err(cast_helper)?,
)
}
MysqlType::Date | MysqlType::DateTime | MysqlType::Timestamp => {
Expand All @@ -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().map_err(cast_helper)?,
)
}
MysqlType::Numeric
Expand All @@ -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))
}
}
4 changes: 2 additions & 2 deletions src/pg/error_helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
})
}
Expand Down
3 changes: 2 additions & 1 deletion src/pg/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,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)]
Expand Down

0 comments on commit f714fdc

Please sign in to comment.