Repository: jonasbb/serde_with
Pull request #809 (2024-12-25 20:04:42)

Remove some `as` casts
This removes some potential arithmetic issues

See #771 for reference


Patch:
diff --git a/serde_with/src/chrono_0_4.rs b/serde_with/src/chrono_0_4.rs
index 66fee5b..7be9c13 100644
--- a/serde_with/src/chrono_0_4.rs
+++ b/serde_with/src/chrono_0_4.rs
@@ -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::*,
@@ -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,
@@ -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
diff --git a/serde_with/src/content/de.rs b/serde_with/src/content/de.rs
index a81a9e2..b87cd48 100644
--- a/serde_with/src/content/de.rs
+++ b/serde_with/src/content/de.rs
@@ -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),
diff --git a/serde_with/src/de/impls.rs b/serde_with/src/de/impls.rs
index 93b370d..2819ca3 100644
--- a/serde_with/src/de/impls.rs
+++ b/serde_with/src/de/impls.rs
@@ -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",
                     )),
                 }
@@ -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",
                     )),
                 }
diff --git a/serde_with/src/ser/impls.rs b/serde_with/src/ser/impls.rs
index 7fc3290..ea995af 100644
--- a/serde_with/src/ser/impls.rs
+++ b/serde_with/src/ser/impls.rs
@@ -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))
     }
 }
 
diff --git a/serde_with/src/time_0_3.rs b/serde_with/src/time_0_3.rs
index 6574f46..49f32be 100644
--- a/serde_with/src/time_0_3.rs
+++ b/serde_with/src/time_0_3.rs
@@ -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::*,
diff --git a/serde_with/src/utils.rs b/serde_with/src/utils.rs
index 256ef94..367ce34 100644
--- a/serde_with/src/utils.rs
+++ b/serde_with/src/utils.rs
@@ -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,
@@ -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");
     }
@@ -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,
     ))
 }
 
diff --git a/serde_with/src/utils/duration.rs b/serde_with/src/utils/duration.rs
index 27332bb..91184b0 100644
--- a/serde_with/src/utils/duration.rs
+++ b/serde_with/src/utils/duration.rs
@@ -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::*,
@@ -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 {
@@ -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
@@ -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 {
@@ -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>
@@ -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!(


Test patch:
diff --git a/serde_with/tests/serde_as/enum_map.rs b/serde_with/tests/serde_as/enum_map.rs
index ada33e9..65ece6f 100644
--- a/serde_with/tests/serde_as/enum_map.rs
+++ b/serde_with/tests/serde_as/enum_map.rs
@@ -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));
             }
         }
     }