Eliminate overflow in Duration constructors

time-rs · Sep 25, 2022 · 8cb9c44 · 8cb9c44
1 parent d0bab6a
commit 8cb9c44
Show file tree

Hide file tree

Showing 5 changed files with 92 additions and 17 deletions.
diff --git a/src/duration.rs b/src/duration.rs
@@ -227,13 +227,18 @@ impl Duration {
     /// assert_eq!(Duration::new(1, 2_000_000_000), 3.seconds());
     /// ```
     pub const fn new(mut seconds: i64, mut nanoseconds: i32) -> Self {
-        seconds += nanoseconds as i64 / 1_000_000_000;
+        seconds = expect_opt!(
+            seconds.checked_add(nanoseconds as i64 / 1_000_000_000),
+            "overflow constructing `time::Duration`"
+        );
         nanoseconds %= 1_000_000_000;
 
         if seconds > 0 && nanoseconds < 0 {
+            // `seconds` cannot overflow here because it is positive.
             seconds -= 1;
             nanoseconds += 1_000_000_000;
         } else if seconds < 0 && nanoseconds > 0 {
+            // `seconds` cannot overflow here because it is negative.
             seconds += 1;
             nanoseconds -= 1_000_000_000;
         }
@@ -249,7 +254,10 @@ impl Duration {
     /// assert_eq!(Duration::weeks(1), 604_800.seconds());
     /// ```
     pub const fn weeks(weeks: i64) -> Self {
-        Self::seconds(weeks * 604_800)
+        Self::seconds(expect_opt!(
+            weeks.checked_mul(604_800),
+            "overflow constructing `time::Duration`"
+        ))
     }
 
     /// Create a new `Duration` with the given number of days. Equivalent to
@@ -260,7 +268,10 @@ impl Duration {
     /// assert_eq!(Duration::days(1), 86_400.seconds());
     /// ```
     pub const fn days(days: i64) -> Self {
-        Self::seconds(days * 86_400)
+        Self::seconds(expect_opt!(
+            days.checked_mul(86_400),
+            "overflow constructing `time::Duration`"
+        ))
     }
 
     /// Create a new `Duration` with the given number of hours. Equivalent to
@@ -271,7 +282,10 @@ impl Duration {
     /// assert_eq!(Duration::hours(1), 3_600.seconds());
     /// ```
     pub const fn hours(hours: i64) -> Self {
-        Self::seconds(hours * 3_600)
+        Self::seconds(expect_opt!(
+            hours.checked_mul(3_600),
+            "overflow constructing `time::Duration`"
+        ))
     }
 
     /// Create a new `Duration` with the given number of minutes. Equivalent to
@@ -282,7 +296,10 @@ impl Duration {
     /// assert_eq!(Duration::minutes(1), 60.seconds());
     /// ```
     pub const fn minutes(minutes: i64) -> Self {
-        Self::seconds(minutes * 60)
+        Self::seconds(expect_opt!(
+            minutes.checked_mul(60),
+            "overflow constructing `time::Duration`"
+        ))
     }
 
     /// Create a new `Duration` with the given number of seconds.
@@ -303,6 +320,9 @@ impl Duration {
     /// assert_eq!(Duration::seconds_f64(-0.5), -0.5.seconds());
     /// ```
     pub fn seconds_f64(seconds: f64) -> Self {
+        if seconds > i64::MAX as f64 || seconds < i64::MIN as f64 {
+            crate::expect_failed("overflow constructing `time::Duration`");
+        }
         Self::new_unchecked(seconds as _, ((seconds % 1.) * 1_000_000_000.) as _)
     }
 
@@ -314,6 +334,9 @@ impl Duration {
     /// assert_eq!(Duration::seconds_f32(-0.5), (-0.5).seconds());
     /// ```
     pub fn seconds_f32(seconds: f32) -> Self {
+        if seconds > i64::MAX as f32 || seconds < i64::MIN as f32 {
+            crate::expect_failed("overflow constructing `time::Duration`");
+        }
         Self::new_unchecked(seconds as _, ((seconds % 1.) * 1_000_000_000.) as _)
     }
 
@@ -364,10 +387,14 @@ impl Duration {
     /// As the input range cannot be fully mapped to the output, this should only be used where it's
     /// known to result in a valid value.
     pub(crate) const fn nanoseconds_i128(nanoseconds: i128) -> Self {
-        Self::new_unchecked(
-            (nanoseconds / 1_000_000_000) as _,
-            (nanoseconds % 1_000_000_000) as _,
-        )
+        let seconds = nanoseconds / 1_000_000_000;
+        let nanoseconds = nanoseconds % 1_000_000_000;
+
+        if seconds > i64::MAX as i128 || seconds < i64::MIN as i128 {
+            crate::expect_failed("overflow constructing `time::Duration`");
+        }
+
+        Self::new_unchecked(seconds as _, nanoseconds as _)
     }
     // endregion constructors
 

diff --git a/src/lib.rs b/src/lib.rs
@@ -283,6 +283,18 @@ macro_rules! const_try_opt {
         }
     };
 }
+
+/// Try to unwrap an expression, panicking if not possible.
+///
+/// This is similar to `$e.expect($message)`, but is usable in `const` contexts.
+macro_rules! expect_opt {
+    ($e:expr, $message:literal) => {
+        match $e {
+            Some(value) => value,
+            None => crate::expect_failed($message),
+        }
+    };
+}
 // endregion macros
 
 mod date;
@@ -334,3 +346,11 @@ pub use crate::weekday::Weekday;
 
 /// An alias for [`std::result::Result`] with a generic error from the time crate.
 pub type Result<T> = core::result::Result<T, Error>;
+
+/// This is a separate function to reduce the code size of `expect_opt!`.
+#[inline(never)]
+#[cold]
+#[track_caller]
+const fn expect_failed(message: &str) -> ! {
+    panic!("{}", message)
+}
diff --git a/tests/integration/duration.rs b/tests/integration/duration.rs
@@ -82,6 +82,9 @@ fn new() {
     assert_eq!(Duration::new(1, -1_400_000_000), (-400).milliseconds());
     assert_eq!(Duration::new(2, -1_400_000_000), 600.milliseconds());
     assert_eq!(Duration::new(3, -1_400_000_000), 1_600.milliseconds());
+
+    assert_panic!(Duration::new(i64::MAX, 1_000_000_000));
+    assert_panic!(Duration::new(i64::MIN, -1_000_000_000));
 }
 
 #[test]
@@ -90,6 +93,9 @@ fn weeks() {
     assert_eq!(Duration::weeks(2), (2 * 604_800).seconds());
     assert_eq!(Duration::weeks(-1), (-604_800).seconds());
     assert_eq!(Duration::weeks(-2), (2 * -604_800).seconds());
+
+    assert_panic!(Duration::weeks(i64::MAX));
+    assert_panic!(Duration::weeks(i64::MIN));
 }
 
 #[test]
@@ -106,6 +112,9 @@ fn days() {
     assert_eq!(Duration::days(2), (2 * 86_400).seconds());
     assert_eq!(Duration::days(-1), (-86_400).seconds());
     assert_eq!(Duration::days(-2), (2 * -86_400).seconds());
+
+    assert_panic!(Duration::days(i64::MAX));
+    assert_panic!(Duration::days(i64::MIN));
 }
 
 #[test]
@@ -122,6 +131,9 @@ fn hours() {
     assert_eq!(Duration::hours(2), (2 * 3_600).seconds());
     assert_eq!(Duration::hours(-1), (-3_600).seconds());
     assert_eq!(Duration::hours(-2), (2 * -3_600).seconds());
+
+    assert_panic!(Duration::hours(i64::MAX));
+    assert_panic!(Duration::hours(i64::MIN));
 }
 
 #[test]
@@ -138,6 +150,9 @@ fn minutes() {
     assert_eq!(Duration::minutes(2), (2 * 60).seconds());
     assert_eq!(Duration::minutes(-1), (-60).seconds());
     assert_eq!(Duration::minutes(-2), (2 * -60).seconds());
+
+    assert_panic!(Duration::minutes(i64::MAX));
+    assert_panic!(Duration::minutes(i64::MIN));
 }
 
 #[test]
@@ -168,6 +183,9 @@ fn whole_seconds() {
 fn seconds_f64() {
     assert_eq!(Duration::seconds_f64(0.5), 0.5.seconds());
     assert_eq!(Duration::seconds_f64(-0.5), (-0.5).seconds());
+
+    assert_panic!(Duration::seconds_f64(f64::MAX));
+    assert_panic!(Duration::seconds_f64(f64::MIN));
 }
 
 #[test]
@@ -185,6 +203,9 @@ fn as_seconds_f64() {
 fn seconds_f32() {
     assert_eq!(Duration::seconds_f32(0.5), 0.5.seconds());
     assert_eq!(Duration::seconds_f32(-0.5), (-0.5).seconds());
+
+    assert_panic!(Duration::seconds_f32(f32::MAX));
+    assert_panic!(Duration::seconds_f32(f32::MIN));
 }
 
 #[test]
@@ -600,6 +621,9 @@ fn std_sub_assign_overflow() {
 fn mul_int() {
     assert_eq!(1.seconds() * 2, 2.seconds());
     assert_eq!(1.seconds() * -2, (-2).seconds());
+
+    assert_panic!(Duration::MAX * 2);
+    assert_panic!(Duration::MIN * 2);
 }
 
 #[test]

diff --git a/tests/integration/main.rs b/tests/integration/main.rs
@@ -71,6 +71,16 @@ macro_rules! modifier {
     (@value $field:ident $value:expr) => ($value);
 }
 
+/// Assert that the given expression panics.
+macro_rules! assert_panic {
+    ($($x:tt)*) => {
+        assert!(std::panic::catch_unwind(|| {
+            $($x)*
+        })
+        .is_err())
+    }
+}
+
 mod date;
 mod derives;
 mod duration;

diff --git a/tests/integration/offset_date_time.rs b/tests/integration/offset_date_time.rs
@@ -51,14 +51,8 @@ fn to_offset() {
 
 #[test]
 fn to_offset_panic() {
-    assert!(
-        std::panic::catch_unwind(|| { PrimitiveDateTime::MAX.assume_utc().to_offset(offset!(+1)) })
-            .is_err()
-    );
-    assert!(
-        std::panic::catch_unwind(|| { PrimitiveDateTime::MIN.assume_utc().to_offset(offset!(-1)) })
-            .is_err()
-    );
+    assert_panic!(PrimitiveDateTime::MAX.assume_utc().to_offset(offset!(+1)));
+    assert_panic!(PrimitiveDateTime::MIN.assume_utc().to_offset(offset!(-1)));
 }
 
 #[test]