diff --git a/Cargo.lock b/Cargo.lock index 1bbc24d6..e580c259 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -896,6 +896,7 @@ dependencies = [ "indexmap 2.1.0", "jsonschema", "mime", + "num-traits", "pretty_assertions", "regex", "rmp-serde", diff --git a/Cargo.toml b/Cargo.toml index f1a516fe..b4afba52 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,6 +48,8 @@ explicit_auto_deref = "allow" manual-unwrap-or-default = "allow" # alloc_instead_of_core = "warn" +as_conversions = "warn" +# arithmetic_side_effects = "warn" # Checks for usage of `cloned()` on an `Iterator` or `Option` where `copied()` could be used instead. cloned_instead_of_copied = "warn" # Checks for literal calls to `Default::default()`. diff --git a/serde_with/Cargo.toml b/serde_with/Cargo.toml index b98481b9..226aace1 100644 --- a/serde_with/Cargo.toml +++ b/serde_with/Cargo.toml @@ -136,6 +136,7 @@ hashbrown_0_15 = {package = "hashbrown", version = "0.15.0", optional = true, de hex = {version = "0.4.3", optional = true, default-features = false} indexmap_1 = {package = "indexmap", version = "1.8", optional = true, default-features = false, features = ["serde-1"]} indexmap_2 = {package = "indexmap", version = "2.0", optional = true, default-features = false, features = ["serde"]} +num-traits = { version = "0.2.19", features = ["std"] } schemars_0_8 = {package = "schemars", version = "0.8.16", optional = true, default-features = false} serde = {version = "1.0.152", default-features = false} serde_derive = "1.0.152" diff --git a/serde_with/src/chrono_0_4.rs b/serde_with/src/chrono_0_4.rs index 66fee5b6..40a25b0e 100644 --- a/serde_with/src/chrono_0_4.rs +++ b/serde_with/src/chrono_0_4.rs @@ -57,6 +57,7 @@ fn unix_epoch_naive() -> NaiveDateTime { #[cfg(feature = "std")] pub mod datetime_utc_ts_seconds_from_any { use super::*; + use num_traits::ToPrimitive as _; /// Deserialize a Unix timestamp with optional subsecond precision into a `DateTime`. pub fn deserialize<'de, D>(deserializer: D) -> Result, D::Error> @@ -87,20 +88,26 @@ pub mod datetime_utc_ts_seconds_from_any { where E: DeError, { - DateTime::from_timestamp(value as i64, 0).ok_or_else(|| { - DeError::custom(format_args!( + i64::try_from(value) + .ok() + .and_then(|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}'" )) - }) + }) } fn visit_f64(self, value: f64) -> Result where E: DeError, { - let seconds = value.trunc() as i64; - let nsecs = (value.fract() * 1_000_000_000_f64).abs() as u32; - DateTime::from_timestamp(seconds, nsecs).ok_or_else(|| { + fn f64_to_value(value: f64) -> Option> { + let seconds = value.trunc().to_i64()?; + let nsecs = (value.fract() * 1_000_000_000_f64).abs().to_u32()?; + DateTime::from_timestamp(seconds, nsecs) + } + f64_to_value(value).ok_or_else(|| { DeError::custom(format_args!( "a timestamp which can be represented in a DateTime but received '{value}'" )) @@ -127,12 +134,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 a81a9e28..d6ca76f9 100644 --- a/serde_with/src/content/de.rs +++ b/serde_with/src/content/de.rs @@ -14,6 +14,7 @@ //! In the future this can hopefully be replaced by a public type in `serde` itself. //! +use self::utils::{get_unexpected_i128, get_unexpected_u128}; use crate::{ prelude::*, utils::{size_hint_cautious, size_hint_from_bounds}, @@ -59,20 +60,20 @@ pub(crate) enum Content<'de> { impl Content<'_> { #[cold] - fn unexpected(&self) -> Unexpected<'_> { + fn unexpected<'a>(&'a self, buf: &'a mut [u8; 58]) -> Unexpected<'a> { 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), - 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::U128(n) => get_unexpected_u128(n, buf), + 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), - Content::I128(_) => Unexpected::Other("i128"), - Content::F32(f) => Unexpected::Float(f as f64), + Content::I128(n) => get_unexpected_i128(n, buf), + 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), @@ -325,7 +326,8 @@ where { #[cold] fn invalid_type(self, exp: &dyn Expected) -> E { - DeError::invalid_type(self.content.unexpected(), exp) + let mut buf = [0; 58]; + DeError::invalid_type(self.content.unexpected(&mut buf), exp) } fn deserialize_integer(self, visitor: V) -> Result @@ -767,7 +769,11 @@ where } s @ Content::String(_) | s @ Content::Str(_) => (s, None), other => { - return Err(DeError::invalid_type(other.unexpected(), &"string or map")); + let mut buf = [0; 58]; + return Err(DeError::invalid_type( + other.unexpected(&mut buf), + &"string or map", + )); } }; @@ -913,7 +919,13 @@ where SeqDeserializer::new(v, self.is_human_readable), visitor, ), - Some(other) => Err(DeError::invalid_type(other.unexpected(), &"tuple variant")), + Some(other) => { + let mut buf = [0; 58]; + Err(DeError::invalid_type( + other.unexpected(&mut buf), + &"tuple variant", + )) + } None => Err(DeError::invalid_type( Unexpected::UnitVariant, &"tuple variant", @@ -938,7 +950,13 @@ where SeqDeserializer::new(v, self.is_human_readable), visitor, ), - Some(other) => Err(DeError::invalid_type(other.unexpected(), &"struct variant")), + Some(other) => { + let mut buf = [0; 58]; + Err(DeError::invalid_type( + other.unexpected(&mut buf), + &"struct variant", + )) + } None => Err(DeError::invalid_type( Unexpected::UnitVariant, &"struct variant", @@ -1129,7 +1147,8 @@ where { #[cold] fn invalid_type(self, exp: &dyn Expected) -> E { - DeError::invalid_type(self.content.unexpected(), exp) + let mut buf = [0; 58]; + DeError::invalid_type(self.content.unexpected(&mut buf), exp) } fn deserialize_integer(self, visitor: V) -> Result @@ -1542,7 +1561,11 @@ where } ref s @ Content::String(_) | ref s @ Content::Str(_) => (s, None), ref other => { - return Err(DeError::invalid_type(other.unexpected(), &"string or map")); + let mut buf = [0; 58]; + return Err(DeError::invalid_type( + other.unexpected(&mut buf), + &"string or map", + )); } }; @@ -1670,7 +1693,13 @@ where SeqRefDeserializer::new(v, self.is_human_readable), visitor, ), - Some(other) => Err(DeError::invalid_type(other.unexpected(), &"tuple variant")), + Some(other) => { + let mut buf = [0; 58]; + Err(DeError::invalid_type( + other.unexpected(&mut buf), + &"tuple variant", + )) + } None => Err(DeError::invalid_type( Unexpected::UnitVariant, &"tuple variant", @@ -1695,7 +1724,13 @@ where SeqRefDeserializer::new(v, self.is_human_readable), visitor, ), - Some(other) => Err(DeError::invalid_type(other.unexpected(), &"struct variant")), + Some(other) => { + let mut buf = [0; 58]; + Err(DeError::invalid_type( + other.unexpected(&mut buf), + &"struct variant", + )) + } None => Err(DeError::invalid_type( Unexpected::UnitVariant, &"struct variant", diff --git a/serde_with/src/de/impls.rs b/serde_with/src/de/impls.rs index 93b370d5..4d0de784 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 { 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 { 0 => Ok(false), 1 => Ok(true), unexp => Err(DeError::invalid_value( - Unexpected::Signed(unexp as i64), + Unexpected::Signed(i64::from(unexp)), &"0 or 1", )), } @@ -1891,10 +1891,13 @@ impl<'de> DeserializeAs<'de, bool> for BoolFromInt { match v { 0 => Ok(false), 1 => Ok(true), - unexp => Err(DeError::invalid_value( - Unexpected::Unsigned(unexp as u64), - &"0 or 1", - )), + unexp => { + let mut buf: [u8; 58] = [0u8; 58]; + Err(DeError::invalid_value( + crate::utils::get_unexpected_u128(unexp, &mut buf), + &self, + )) + } } } @@ -1905,10 +1908,13 @@ impl<'de> DeserializeAs<'de, bool> for BoolFromInt { match v { 0 => Ok(false), 1 => Ok(true), - unexp => Err(DeError::invalid_value( - Unexpected::Signed(unexp as i64), - &"0 or 1", - )), + unexp => { + let mut buf: [u8; 58] = [0u8; 58]; + Err(DeError::invalid_value( + crate::utils::get_unexpected_i128(unexp, &mut buf), + &"0 or 1", + )) + } } } } diff --git a/serde_with/src/ser/impls.rs b/serde_with/src/ser/impls.rs index 7fc32900..ea995af5 100644 --- a/serde_with/src/ser/impls.rs +++ b/serde_with/src/ser/impls.rs @@ -1024,7 +1024,7 @@ impl SerializeAs for BoolFromInt { where S: Serializer, { - serializer.serialize_u8(*source as u8) + serializer.serialize_u8(u8::from(*source)) } } diff --git a/serde_with/src/utils.rs b/serde_with/src/utils.rs index 256ef948..e6122770 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 { - 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(&arr) }) } + +/// Writer that writes into a `&mut [u8]` while checking the length of the buffer +struct BufWriter<'a> { + bytes: &'a mut [u8], + offset: usize, +} + +impl<'a> BufWriter<'a> { + fn new(bytes: &'a mut [u8]) -> Self { + BufWriter { bytes, offset: 0 } + } + + fn into_str(self) -> &'a str { + let slice = &self.bytes[..self.offset]; + core::str::from_utf8(slice) + .unwrap_or("Failed to extract valid string from BufWriter. This should never happen.") + } +} + +impl<'a> core::fmt::Write for BufWriter<'a> { + fn write_str(&mut self, s: &str) -> fmt::Result { + if s.len() > self.bytes.len() - self.offset { + Err(fmt::Error) + } else { + self.bytes[self.offset..self.offset + s.len()].copy_from_slice(s.as_bytes()); + self.offset += s.len(); + Ok(()) + } + } +} + +// 58 chars is long enough for any i128 and u128 value +pub(crate) fn get_unexpected_i128(value: i128, buf: &mut [u8; 58]) -> Unexpected<'_> { + let mut writer = BufWriter::new(buf); + fmt::Write::write_fmt(&mut writer, format_args!("integer `{value}` as i128")).unwrap(); + Unexpected::Other(writer.into_str()) +} + +// 58 chars is long enough for any i128 and u128 value +pub(crate) fn get_unexpected_u128(value: u128, buf: &mut [u8; 58]) -> Unexpected<'_> { + let mut writer = BufWriter::new(buf); + fmt::Write::write_fmt(&mut writer, format_args!("integer `{value}` as u128")).unwrap(); + Unexpected::Other(writer.into_str()) +} diff --git a/serde_with/src/utils/duration.rs b/serde_with/src/utils/duration.rs index 27332bbb..93bb4056 100644 --- a/serde_with/src/utils/duration.rs +++ b/serde_with/src/utils/duration.rs @@ -23,13 +23,17 @@ impl Sign { *self == Sign::Negative } - pub(crate) fn apply(&self, value: T) -> T - where - T: core::ops::Neg, - { + pub(crate) fn apply_f64(&self, value: f64) -> f64 { match *self { Sign::Positive => value, - Sign::Negative => value.neg(), + Sign::Negative => -value, + } + } + + pub(crate) fn apply_i64(&self, value: i64) -> Option { + match *self { + Sign::Positive => Some(value), + Sign::Negative => value.checked_neg(), } } } @@ -48,6 +52,16 @@ impl DurationSigned { } } + pub(crate) fn checked_mul(mut self, rhs: u32) -> Option { + self.duration = self.duration.checked_mul(rhs)?; + Some(self) + } + + pub(crate) fn checked_div(mut self, rhs: u32) -> Option { + self.duration = self.duration.checked_div(rhs)?; + Some(self) + } + #[cfg(any(feature = "chrono_0_4", feature = "time_0_3"))] pub(crate) fn with_duration(sign: Sign, duration: Duration) -> Self { Self { sign, duration } @@ -102,24 +116,6 @@ impl From<&SystemTime> for DurationSigned { } } -impl core::ops::Mul for DurationSigned { - type Output = DurationSigned; - - fn mul(mut self, rhs: u32) -> Self::Output { - self.duration *= rhs; - self - } -} - -impl core::ops::Div for DurationSigned { - type Output = DurationSigned; - - fn div(mut self, rhs: u32) -> Self::Output { - self.duration /= rhs; - self - } -} - impl SerializeAs for DurationSeconds where STRICTNESS: Strictness, @@ -156,9 +152,16 @@ where where S: Serializer, { - let mut secs = source.sign.apply(source.duration.as_secs() as i64); + let mut secs = source + .sign + // TODO BUG771 the as i64 can fail + .apply_i64(source.duration.as_secs() as i64) + .ok_or_else(|| { + S::Error::custom("Failed to serialize value as the value cannot be represented.") + })?; // Properly round the value + // TODO check for overflows BUG771 if source.duration.subsec_millis() >= 500 { if source.sign.is_positive() { secs += 1; @@ -178,7 +181,7 @@ where where S: Serializer, { - let mut secs = source.sign.apply(source.duration.as_secs() as f64); + let mut secs = source.sign.apply_f64(source.duration.as_secs() as f64); // Properly round the value if source.duration.subsec_millis() >= 500 { @@ -201,7 +204,12 @@ where where S: Serializer, { - let mut secs = source.sign.apply(source.duration.as_secs() as i64); + let mut secs = source + .sign + .apply_i64(source.duration.as_secs() as i64) + .ok_or_else(|| { + S::Error::custom("Failed to serialize value as the value cannot be represented.") + })?; // Properly round the value if source.duration.subsec_millis() >= 500 { @@ -225,7 +233,7 @@ where { source .sign - .apply(source.duration.as_secs_f64()) + .apply_f64(source.duration.as_secs_f64()) .serialize(serializer) } } @@ -241,7 +249,7 @@ where { source .sign - .apply(source.duration.as_secs_f64()) + .apply_f64(source.duration.as_secs_f64()) .to_string() .serialize(serializer) } @@ -261,7 +269,8 @@ macro_rules! duration_impls { where S: Serializer, { - $inner::::serialize_as(&(*source * $factor), serializer) + let value = source.checked_mul($factor).ok_or_else(|| S::Error::custom("Failed to serialize value as the value cannot be represented."))?; + $inner::::serialize_as(&value, serializer) } } @@ -276,7 +285,8 @@ macro_rules! duration_impls { D: Deserializer<'de>, { let dur = $inner::::deserialize_as(deserializer)?; - Ok(dur / $factor) + let dur = dur.checked_div($factor).ok_or_else(|| D::Error::custom("Failed to deserialize value as the value cannot be represented."))?; + Ok(dur) } } @@ -307,11 +317,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(self, secs: u64) -> Result diff --git a/serde_with/tests/chrono_0_4.rs b/serde_with/tests/chrono_0_4.rs index 8817f384..01c15624 100644 --- a/serde_with/tests/chrono_0_4.rs +++ b/serde_with/tests/chrono_0_4.rs @@ -5,7 +5,8 @@ extern crate alloc; mod utils; use crate::utils::{ - check_deserialization, check_error_deserialization, check_serialization, is_equal, + check_deserialization, check_error_deserialization, check_error_serialization, + check_serialization, is_equal, }; use alloc::collections::BTreeMap; use chrono_0_4::{DateTime, Duration, Local, NaiveDateTime, Utc}; @@ -743,3 +744,55 @@ fn test_naive_datetime_smoketest() { NaiveDateTime, "TimestampSecondsWithFrac", zero - Duration::seconds(1), {expect![[r#"-1.0"#]]}; }; } + +#[test] +fn bug771_extreme_nanoseconds() { + #[serde_as] + #[derive(Debug, Serialize)] + struct S(#[serde_as(as = "serde_with::TimestampNanoSeconds")] DateTime); + + check_error_serialization( + S(DateTime::::MIN_UTC), + expect!["Failed to serialize value as the value cannot be represented."], + ); + check_error_serialization( + S("1385-06-12T00:25:26.290448384Z".parse().unwrap()), + expect!["Failed to serialize value as the value cannot be represented."], + ); + check_error_serialization( + S("1385-06-12T00:25:26.290448385Z".parse().unwrap()), + expect!["Failed to serialize value as the value cannot be represented."], + ); + check_error_serialization( + S("1677-09-21T00:12:43.145224191Z".parse().unwrap()), + expect!["Failed to serialize value as the value cannot be represented."], + ); + check_error_serialization( + S("1677-09-21T00:12:43.145224192Z".parse().unwrap()), + expect!["Failed to serialize value as the value cannot be represented."], + ); + check_serialization( + S("1677-09-21T00:12:43.145224193Z".parse().unwrap()), + expect!["-9223372036854775807"], + ); + check_serialization( + S("2262-04-11T23:47:16.854775807Z".parse().unwrap()), + expect!["9223372036854775807"], + ); + check_error_serialization( + S("2262-04-11T23:47:16.854775808Z".parse().unwrap()), + expect!["Failed to serialize value as the value cannot be represented."], + ); + check_error_serialization( + S("2554-07-21T23:34:33.709551615Z".parse().unwrap()), + expect!["Failed to serialize value as the value cannot be represented."], + ); + check_error_serialization( + S("2554-07-21T23:34:33.709551616Z".parse().unwrap()), + expect!["Failed to serialize value as the value cannot be represented."], + ); + check_error_serialization( + S(DateTime::::MAX_UTC), + expect!["Failed to serialize value as the value cannot be represented."], + ); +} diff --git a/serde_with/tests/serde_as/enum_map.rs b/serde_with/tests/serde_as/enum_map.rs index ada33e93..65ece6f2 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)); } } }