Skip to content

Commit

Permalink
Remove some as casts (#809)
Browse files Browse the repository at this point in the history
This removes some potential arithmetic issues

See #771 for reference
  • Loading branch information
jonasbb authored Dec 25, 2024
2 parents abb13ca + c910fa1 commit 4c273b2
Show file tree
Hide file tree
Showing 8 changed files with 76 additions and 29 deletions.
25 changes: 19 additions & 6 deletions serde_with/src/chrono_0_4.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@
//!
//! [chrono]: https://docs.rs/chrono/
// Serialization of large numbers can result in overflows
// The time calculations are prone to this, so lint here extra
// https://github.com/jonasbb/serde_with/issues/771
#![warn(clippy::as_conversions)]

use crate::{
formats::{Flexible, Format, Strict, Strictness},
prelude::*,
Expand Down Expand Up @@ -87,13 +92,20 @@ pub mod datetime_utc_ts_seconds_from_any {
where
E: DeError,
{
DateTime::from_timestamp(value as i64, 0).ok_or_else(|| {
let value = i64::try_from(value).map_err(|_| {
DeError::custom(format_args!(
"a timestamp which can be represented in a DateTime but received '{value}'"
))
})?;
DateTime::from_timestamp(value, 0).ok_or_else(|| {
DeError::custom(format_args!(
"a timestamp which can be represented in a DateTime but received '{value}'"
))
})
}

// as conversions are necessary for floats
#[allow(clippy::as_conversions)]
fn visit_f64<E>(self, value: f64) -> Result<Self::Value, E>
where
E: DeError,
Expand Down Expand Up @@ -127,12 +139,13 @@ pub mod datetime_utc_ts_seconds_from_any {
}
[seconds, subseconds] => {
if let Ok(seconds) = seconds.parse() {
let subseclen = subseconds.chars().count() as u32;
if subseclen > 9 {
return Err(DeError::custom(format_args!(
let subseclen =
match u32::try_from(subseconds.chars().count()) {
Ok(subseclen) if subseclen <= 9 => subseclen,
_ => return Err(DeError::custom(format_args!(
"DateTimes only support nanosecond precision but '{value}' has more than 9 digits."
)));
}
))),
};

if let Ok(mut subseconds) = subseconds.parse() {
// convert subseconds to nanoseconds (10^-9), require 9 places for nanoseconds
Expand Down
16 changes: 9 additions & 7 deletions serde_with/src/content/de.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,19 @@ impl Content<'_> {
fn unexpected(&self) -> Unexpected<'_> {
match *self {
Content::Bool(b) => Unexpected::Bool(b),
Content::U8(n) => Unexpected::Unsigned(n as u64),
Content::U16(n) => Unexpected::Unsigned(n as u64),
Content::U32(n) => Unexpected::Unsigned(n as u64),
Content::U8(n) => Unexpected::Unsigned(u64::from(n)),
Content::U16(n) => Unexpected::Unsigned(u64::from(n)),
Content::U32(n) => Unexpected::Unsigned(u64::from(n)),
Content::U64(n) => Unexpected::Unsigned(n),
// TODO generate better unexpected error
Content::U128(_) => Unexpected::Other("u128"),
Content::I8(n) => Unexpected::Signed(n as i64),
Content::I16(n) => Unexpected::Signed(n as i64),
Content::I32(n) => Unexpected::Signed(n as i64),
Content::I8(n) => Unexpected::Signed(i64::from(n)),
Content::I16(n) => Unexpected::Signed(i64::from(n)),
Content::I32(n) => Unexpected::Signed(i64::from(n)),
Content::I64(n) => Unexpected::Signed(n),
// TODO generate better unexpected error
Content::I128(_) => Unexpected::Other("i128"),
Content::F32(f) => Unexpected::Float(f as f64),
Content::F32(f) => Unexpected::Float(f64::from(f)),
Content::F64(f) => Unexpected::Float(f),
Content::Char(c) => Unexpected::Char(c),
Content::String(ref s) => Unexpected::Str(s),
Expand Down
4 changes: 2 additions & 2 deletions serde_with/src/de/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1839,7 +1839,7 @@ impl<'de> DeserializeAs<'de, bool> for BoolFromInt<Strict> {
0 => Ok(false),
1 => Ok(true),
unexp => Err(DeError::invalid_value(
Unexpected::Unsigned(unexp as u64),
Unexpected::Unsigned(u64::from(unexp)),
&"0 or 1",
)),
}
Expand All @@ -1853,7 +1853,7 @@ impl<'de> DeserializeAs<'de, bool> for BoolFromInt<Strict> {
0 => Ok(false),
1 => Ok(true),
unexp => Err(DeError::invalid_value(
Unexpected::Signed(unexp as i64),
Unexpected::Signed(i64::from(unexp)),
&"0 or 1",
)),
}
Expand Down
2 changes: 1 addition & 1 deletion serde_with/src/ser/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1024,7 +1024,7 @@ impl<STRICTNESS: Strictness> SerializeAs<bool> for BoolFromInt<STRICTNESS> {
where
S: Serializer,
{
serializer.serialize_u8(*source as u8)
serializer.serialize_u8(u8::from(*source))
}
}

Expand Down
5 changes: 5 additions & 0 deletions serde_with/src/time_0_3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@
//!
//! [time]: https://docs.rs/time/0.3/
// Serialization of large numbers can result in overflows
// The time calculations are prone to this, so lint here extra
// https://github.com/jonasbb/serde_with/issues/771
#![warn(clippy::as_conversions)]

use crate::{
formats::{Flexible, Format, Strict, Strictness},
prelude::*,
Expand Down
12 changes: 7 additions & 5 deletions serde_with/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@ where
_size_hint_from_bounds(iter.size_hint())
}

pub(crate) const NANOS_PER_SEC: u32 = 1_000_000_000;
pub(crate) const NANOS_PER_SEC: u128 = 1_000_000_000;
pub(crate) const NANOS_PER_SEC_F64: f64 = 1_000_000_000.0;
// pub(crate) const NANOS_PER_MILLI: u32 = 1_000_000;
// pub(crate) const NANOS_PER_MICRO: u32 = 1_000;
// pub(crate) const MILLIS_PER_SEC: u64 = 1_000;
// pub(crate) const MICROS_PER_SEC: u64 = 1_000_000;
pub(crate) const U64_MAX: u128 = u64::MAX as u128;

pub(crate) struct MapIter<'de, A, K, V> {
pub(crate) access: A,
Expand Down Expand Up @@ -114,10 +116,10 @@ where
}

pub(crate) fn duration_signed_from_secs_f64(secs: f64) -> Result<DurationSigned, &'static str> {
const MAX_NANOS_F64: f64 = ((u64::MAX as u128 + 1) * (NANOS_PER_SEC as u128)) as f64;
const MAX_NANOS_F64: f64 = ((U64_MAX + 1) * NANOS_PER_SEC) as f64;
// TODO why are the seconds converted to nanoseconds first?
// Does it make sense to just truncate the value?
let mut nanos = secs * (NANOS_PER_SEC as f64);
let mut nanos = secs * NANOS_PER_SEC_F64;
if !nanos.is_finite() {
return Err("got non-finite value when converting float to duration");
}
Expand All @@ -132,8 +134,8 @@ pub(crate) fn duration_signed_from_secs_f64(secs: f64) -> Result<DurationSigned,
let nanos = nanos as u128;
Ok(DurationSigned::new(
sign,
(nanos / (NANOS_PER_SEC as u128)) as u64,
(nanos % (NANOS_PER_SEC as u128)) as u32,
(nanos / NANOS_PER_SEC) as u64,
(nanos % NANOS_PER_SEC) as u32,
))
}

Expand Down
39 changes: 32 additions & 7 deletions serde_with/src/utils/duration.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
//! Internal Helper types
// Serialization of large numbers can result in overflows
// The time calculations are prone to this, so lint here extra
// https://github.com/jonasbb/serde_with/issues/771
#![warn(clippy::as_conversions)]

use crate::{
formats::{Flexible, Format, Strict, Strictness},
prelude::*,
Expand Down Expand Up @@ -156,7 +161,11 @@ where
where
S: Serializer,
{
let mut secs = source.sign.apply(source.duration.as_secs() as i64);
let mut secs = source
.sign
.apply(i64::try_from(source.duration.as_secs()).map_err(|_| {
SerError::custom("The Duration of Timestamp is outside the supported range.")
})?);

// Properly round the value
if source.duration.subsec_millis() >= 500 {
Expand All @@ -178,6 +187,8 @@ where
where
S: Serializer,
{
// as conversions are necessary for floats
#[allow(clippy::as_conversions)]
let mut secs = source.sign.apply(source.duration.as_secs() as f64);

// Properly round the value
Expand All @@ -201,7 +212,11 @@ where
where
S: Serializer,
{
let mut secs = source.sign.apply(source.duration.as_secs() as i64);
let mut secs = source
.sign
.apply(i64::try_from(source.duration.as_secs()).map_err(|_| {
SerError::custom("The Duration of Timestamp is outside the supported range.")
})?);

// Properly round the value
if source.duration.subsec_millis() >= 500 {
Expand Down Expand Up @@ -307,11 +322,12 @@ impl Visitor<'_> for DurationVisitorFlexible {
where
E: DeError,
{
if value >= 0 {
Ok(DurationSigned::new(Sign::Positive, value as u64, 0))
let sign = if value >= 0 {
Sign::Positive
} else {
Ok(DurationSigned::new(Sign::Negative, (-value) as u64, 0))
}
Sign::Negative
};
Ok(DurationSigned::new(sign, value.unsigned_abs(), 0))
}

fn visit_u64<E>(self, secs: u64) -> Result<Self::Value, E>
Expand Down Expand Up @@ -503,7 +519,16 @@ fn parse_float_into_time_parts(mut value: &str) -> Result<(Sign, u64, u32), Pars
let seconds = parts.next().expect("Float contains exactly one part");
if let Ok(seconds) = seconds.parse() {
let subseconds = parts.next().expect("Float contains exactly one part");
let subseclen = subseconds.chars().count() as u32;
let subseclen = u32::try_from(subseconds.chars().count()).map_err(|_| {
#[cfg(feature = "alloc")]
return ParseFloatError::Custom(alloc::format!(
"Duration and Timestamps with no more than 9 digits precision, but '{value}' has more"
));
#[cfg(not(feature = "alloc"))]
return ParseFloatError::Custom(
"Duration and Timestamps with no more than 9 digits precision",
);
})?;
if subseclen > 9 {
#[cfg(feature = "alloc")]
return Err(ParseFloatError::Custom(alloc::format!(
Expand Down
2 changes: 1 addition & 1 deletion serde_with/tests/serde_as/enum_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ fn bytes_debug_readable(bytes: &[u8]) -> String {
}
b'\\' => result.push_str("\\\\"),
_ => {
result.push(byte as char);
result.push(char::from(byte));
}
}
}
Expand Down

0 comments on commit 4c273b2

Please sign in to comment.