From 2805dd733bd85825054f1971e760ff28c27e31ca Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Mon, 5 Apr 2021 17:27:13 +0200 Subject: [PATCH] serde: Improve error handling in time modules --- ruma-serde/src/time/ms_since_unix_epoch.rs | 20 +++++++++---------- .../src/time/opt_ms_since_unix_epoch.rs | 11 +++++++--- ruma-serde/src/time/opt_s_since_unix_epoch.rs | 11 +++++++--- ruma-serde/src/time/s_since_unix_epoch.rs | 20 +++++++++---------- 4 files changed, 36 insertions(+), 26 deletions(-) diff --git a/ruma-serde/src/time/ms_since_unix_epoch.rs b/ruma-serde/src/time/ms_since_unix_epoch.rs index 6abb8fcb..6f074f5e 100644 --- a/ruma-serde/src/time/ms_since_unix_epoch.rs +++ b/ruma-serde/src/time/ms_since_unix_epoch.rs @@ -2,14 +2,14 @@ //! since the UNIX epoch. Delegates to `js_int::UInt` to ensure integer size is within bounds. use std::{ - convert::TryFrom, + convert::TryInto, time::{Duration, SystemTime, UNIX_EPOCH}, }; use js_int::UInt; use serde::{ - de::{Deserialize, Deserializer}, - ser::{Error, Serialize, Serializer}, + de::{self, Deserialize, Deserializer}, + ser::{self, Serialize, Serializer}, }; /// Serialize a SystemTime. @@ -20,12 +20,10 @@ pub fn serialize(time: &SystemTime, serializer: S) -> Result where S: Serializer, { - // If this unwrap fails, the system this is executed is completely broken. - let time_since_epoch = time.duration_since(UNIX_EPOCH).unwrap(); - match UInt::try_from(time_since_epoch.as_millis()) { - Ok(uint) => uint.serialize(serializer), - Err(err) => Err(S::Error::custom(err)), - } + let time_since_epoch = time.duration_since(UNIX_EPOCH).map_err(ser::Error::custom)?; + let uint: UInt = time_since_epoch.as_millis().try_into().map_err(ser::Error::custom)?; + + uint.serialize(serializer) } /// Deserializes a SystemTime. @@ -37,7 +35,9 @@ where D: Deserializer<'de>, { let millis = UInt::deserialize(deserializer)?; - Ok(UNIX_EPOCH + Duration::from_millis(millis.into())) + UNIX_EPOCH + .checked_add(Duration::from_millis(millis.into())) + .ok_or_else(|| de::Error::custom("input too large for SystemTime")) } #[cfg(test)] diff --git a/ruma-serde/src/time/opt_ms_since_unix_epoch.rs b/ruma-serde/src/time/opt_ms_since_unix_epoch.rs index 100b84c9..f7eb7746 100644 --- a/ruma-serde/src/time/opt_ms_since_unix_epoch.rs +++ b/ruma-serde/src/time/opt_ms_since_unix_epoch.rs @@ -6,7 +6,7 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH}; use js_int::UInt; use serde::{ - de::{Deserialize, Deserializer}, + de::{self, Deserialize, Deserializer}, ser::{Serialize, Serializer}, }; @@ -32,8 +32,13 @@ pub fn deserialize<'de, D>(deserializer: D) -> Result, D::Err where D: Deserializer<'de>, { - Ok(Option::::deserialize(deserializer)? - .map(|millis| UNIX_EPOCH + Duration::from_millis(millis.into()))) + Option::::deserialize(deserializer)? + .map(|millis| { + UNIX_EPOCH + .checked_add(Duration::from_millis(millis.into())) + .ok_or_else(|| de::Error::custom("input too large for SystemTime")) + }) + .transpose() } #[cfg(test)] diff --git a/ruma-serde/src/time/opt_s_since_unix_epoch.rs b/ruma-serde/src/time/opt_s_since_unix_epoch.rs index b7711b62..5a19ba11 100644 --- a/ruma-serde/src/time/opt_s_since_unix_epoch.rs +++ b/ruma-serde/src/time/opt_s_since_unix_epoch.rs @@ -6,7 +6,7 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH}; use js_int::UInt; use serde::{ - de::{Deserialize, Deserializer}, + de::{self, Deserialize, Deserializer}, ser::{Serialize, Serializer}, }; @@ -32,8 +32,13 @@ pub fn deserialize<'de, D>(deserializer: D) -> Result, D::Err where D: Deserializer<'de>, { - Ok(Option::::deserialize(deserializer)? - .map(|secs| UNIX_EPOCH + Duration::from_secs(secs.into()))) + Option::::deserialize(deserializer)? + .map(|secs| { + UNIX_EPOCH + .checked_add(Duration::from_secs(secs.into())) + .ok_or_else(|| de::Error::custom("input too large for SystemTime")) + }) + .transpose() } #[cfg(test)] diff --git a/ruma-serde/src/time/s_since_unix_epoch.rs b/ruma-serde/src/time/s_since_unix_epoch.rs index 75c43acd..103b7a3f 100644 --- a/ruma-serde/src/time/s_since_unix_epoch.rs +++ b/ruma-serde/src/time/s_since_unix_epoch.rs @@ -2,14 +2,14 @@ //! the UNIX epoch. Delegates to `js_int::UInt` to ensure integer size is within bounds. use std::{ - convert::TryFrom, + convert::TryInto, time::{Duration, SystemTime, UNIX_EPOCH}, }; use js_int::UInt; use serde::{ - de::{Deserialize, Deserializer}, - ser::{Error, Serialize, Serializer}, + de::{self, Deserialize, Deserializer}, + ser::{self, Serialize, Serializer}, }; /// Serialize a SystemTime. @@ -20,12 +20,10 @@ pub fn serialize(time: &SystemTime, serializer: S) -> Result where S: Serializer, { - // If this unwrap fails, the system this is executed is completely broken. - let time_since_epoch = time.duration_since(UNIX_EPOCH).unwrap(); - match UInt::try_from(time_since_epoch.as_secs()) { - Ok(uint) => uint.serialize(serializer), - Err(err) => Err(S::Error::custom(err)), - } + let time_since_epoch = time.duration_since(UNIX_EPOCH).map_err(ser::Error::custom)?; + let uint: UInt = time_since_epoch.as_secs().try_into().map_err(ser::Error::custom)?; + + uint.serialize(serializer) } /// Deserializes a SystemTime. @@ -37,7 +35,9 @@ where D: Deserializer<'de>, { let secs = UInt::deserialize(deserializer)?; - Ok(UNIX_EPOCH + Duration::from_secs(secs.into())) + UNIX_EPOCH + .checked_add(Duration::from_secs(secs.into())) + .ok_or_else(|| de::Error::custom("input too large for SystemTime")) } #[cfg(test)]