Skip to content

Commit f4f784c

Browse files
authored
Remove use of ops::Mul and ops::Neg since they can fail (#811)
2 parents 9c06ffa + 90d7312 commit f4f784c

File tree

1 file changed

+37
-32
lines changed

1 file changed

+37
-32
lines changed

serde_with/src/utils/duration.rs

Lines changed: 37 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,17 @@ impl Sign {
2828
*self == Sign::Negative
2929
}
3030

31-
pub(crate) fn apply<T>(&self, value: T) -> T
32-
where
33-
T: core::ops::Neg<Output = T>,
34-
{
31+
pub(crate) fn apply_f64(&self, value: f64) -> f64 {
3532
match *self {
3633
Sign::Positive => value,
37-
Sign::Negative => value.neg(),
34+
Sign::Negative => -value,
35+
}
36+
}
37+
38+
pub(crate) fn apply_i64(&self, value: i64) -> Option<i64> {
39+
match *self {
40+
Sign::Positive => Some(value),
41+
Sign::Negative => value.checked_neg(),
3842
}
3943
}
4044
}
@@ -53,6 +57,16 @@ impl DurationSigned {
5357
}
5458
}
5559

60+
pub(crate) fn checked_mul(mut self, rhs: u32) -> Option<Self> {
61+
self.duration = self.duration.checked_mul(rhs)?;
62+
Some(self)
63+
}
64+
65+
pub(crate) fn checked_div(mut self, rhs: u32) -> Option<Self> {
66+
self.duration = self.duration.checked_div(rhs)?;
67+
Some(self)
68+
}
69+
5670
#[cfg(any(feature = "chrono_0_4", feature = "time_0_3"))]
5771
pub(crate) fn with_duration(sign: Sign, duration: Duration) -> Self {
5872
Self { sign, duration }
@@ -107,24 +121,6 @@ impl From<&SystemTime> for DurationSigned {
107121
}
108122
}
109123

110-
impl core::ops::Mul<u32> for DurationSigned {
111-
type Output = DurationSigned;
112-
113-
fn mul(mut self, rhs: u32) -> Self::Output {
114-
self.duration *= rhs;
115-
self
116-
}
117-
}
118-
119-
impl core::ops::Div<u32> for DurationSigned {
120-
type Output = DurationSigned;
121-
122-
fn div(mut self, rhs: u32) -> Self::Output {
123-
self.duration /= rhs;
124-
self
125-
}
126-
}
127-
128124
impl<STRICTNESS> SerializeAs<DurationSigned> for DurationSeconds<u64, STRICTNESS>
129125
where
130126
STRICTNESS: Strictness,
@@ -163,11 +159,15 @@ where
163159
{
164160
let mut secs = source
165161
.sign
166-
.apply(i64::try_from(source.duration.as_secs()).map_err(|_| {
162+
.apply_i64(i64::try_from(source.duration.as_secs()).map_err(|_| {
167163
SerError::custom("The Duration of Timestamp is outside the supported range.")
168-
})?);
164+
})?)
165+
.ok_or_else(|| {
166+
S::Error::custom("The Duration of Timestamp is outside the supported range.")
167+
})?;
169168

170169
// Properly round the value
170+
// TODO check for overflows BUG771
171171
if source.duration.subsec_millis() >= 500 {
172172
if source.sign.is_positive() {
173173
secs += 1;
@@ -189,7 +189,7 @@ where
189189
{
190190
// as conversions are necessary for floats
191191
#[allow(clippy::as_conversions)]
192-
let mut secs = source.sign.apply(source.duration.as_secs() as f64);
192+
let mut secs = source.sign.apply_f64(source.duration.as_secs() as f64);
193193

194194
// Properly round the value
195195
if source.duration.subsec_millis() >= 500 {
@@ -214,9 +214,12 @@ where
214214
{
215215
let mut secs = source
216216
.sign
217-
.apply(i64::try_from(source.duration.as_secs()).map_err(|_| {
217+
.apply_i64(i64::try_from(source.duration.as_secs()).map_err(|_| {
218218
SerError::custom("The Duration of Timestamp is outside the supported range.")
219-
})?);
219+
})?)
220+
.ok_or_else(|| {
221+
S::Error::custom("The Duration of Timestamp is outside the supported range.")
222+
})?;
220223

221224
// Properly round the value
222225
if source.duration.subsec_millis() >= 500 {
@@ -240,7 +243,7 @@ where
240243
{
241244
source
242245
.sign
243-
.apply(source.duration.as_secs_f64())
246+
.apply_f64(source.duration.as_secs_f64())
244247
.serialize(serializer)
245248
}
246249
}
@@ -256,7 +259,7 @@ where
256259
{
257260
source
258261
.sign
259-
.apply(source.duration.as_secs_f64())
262+
.apply_f64(source.duration.as_secs_f64())
260263
.to_string()
261264
.serialize(serializer)
262265
}
@@ -276,7 +279,8 @@ macro_rules! duration_impls {
276279
where
277280
S: Serializer,
278281
{
279-
$inner::<FORMAT, STRICTNESS>::serialize_as(&(*source * $factor), serializer)
282+
let value = source.checked_mul($factor).ok_or_else(|| S::Error::custom("Failed to serialize value as the value cannot be represented."))?;
283+
$inner::<FORMAT, STRICTNESS>::serialize_as(&value, serializer)
280284
}
281285
}
282286

@@ -291,7 +295,8 @@ macro_rules! duration_impls {
291295
D: Deserializer<'de>,
292296
{
293297
let dur = $inner::<FORMAT, STRICTNESS>::deserialize_as(deserializer)?;
294-
Ok(dur / $factor)
298+
let dur = dur.checked_div($factor).ok_or_else(|| D::Error::custom("Failed to deserialize value as the value cannot be represented."))?;
299+
Ok(dur)
295300
}
296301
}
297302

0 commit comments

Comments
 (0)