Fix all methods that would panic on out-of-range local datetime

src/datetime/mod.rs

@@ -168,6 +168,14 @@ impl<Tz: TimeZone> DateTime<Tz> {
     /// [`NaiveDate`] is a more well-defined type, and has more traits implemented on it,
     /// so should be preferred to [`Date`] any time you truly want to operate on Dates.
     ///
+    /// # Panics
+    ///
+    /// [`DateTime`] internally stores the date and time in UTC with a [`NaiveDateTime`]. This
+    /// method will panic if the offset from UTC would push the local datetime outside of the
+    /// representable range of a [`DateTime`].
+    ///
+    /// # Example
+    ///
     /// ```
     /// use chrono::prelude::*;
     ///
@@ -352,7 +360,7 @@ impl<Tz: TimeZone> DateTime<Tz> {
     /// See [`NaiveDate::checked_add_months`] for more details on behavior
     #[must_use]
     pub fn checked_add_months(self, rhs: Months) -> Option<DateTime<Tz>> {
-        self.naive_local()
+        self.overflowing_naive_local()
             .checked_add_months(rhs)?
             .and_local_timezone(Tz::from_offset(&self.offset))
             .single()
@@ -377,7 +385,7 @@ impl<Tz: TimeZone> DateTime<Tz> {
     /// See [`NaiveDate::checked_sub_months`] for more details on behavior
     #[must_use]
     pub fn checked_sub_months(self, rhs: Months) -> Option<DateTime<Tz>> {
-        self.naive_local()
+        self.overflowing_naive_local()
             .checked_sub_months(rhs)?
             .and_local_timezone(Tz::from_offset(&self.offset))
             .single()
@@ -388,7 +396,7 @@ impl<Tz: TimeZone> DateTime<Tz> {
     /// Returns `None` if the resulting date would be out of range.
     #[must_use]
     pub fn checked_add_days(self, days: Days) -> Option<Self> {
-        self.naive_local()
+        self.overflowing_naive_local()
             .checked_add_days(days)?
             .and_local_timezone(TimeZone::from_offset(&self.offset))
             .single()
@@ -399,7 +407,7 @@ impl<Tz: TimeZone> DateTime<Tz> {
     /// Returns `None` if the resulting date would be out of range.
     #[must_use]
     pub fn checked_sub_days(self, days: Days) -> Option<Self> {
-        self.naive_local()
+        self.overflowing_naive_local()
             .checked_sub_days(days)?
             .and_local_timezone(TimeZone::from_offset(&self.offset))
             .single()
@@ -421,10 +429,30 @@ impl<Tz: TimeZone> DateTime<Tz> {
     }
 
     /// Returns a view to the naive local datetime.
+    ///
+    /// # Panics
+    ///
+    /// [`DateTime`] internally stores the date and time in UTC with a [`NaiveDateTime`]. This
+    /// method will panic if the offset from UTC would push the local datetime outside of the
+    /// representable range of a [`NaiveDateTime`].
     #[inline]
     #[must_use]
     pub fn naive_local(&self) -> NaiveDateTime {
-        self.datetime + self.offset.fix()
+        self.datetime
+            .checked_add_offset(self.offset.fix())
+            .expect("Local time out of range for `NaiveDateTime`")
+    }
+
+    /// Returns a view to the naive local datetime.
+    ///
+    /// FOR INTERNAL USE ONLY.
+    /// This makes use of the buffer space outside of the representable range of values of
+    /// `NaiveDateTime`. The result can be user as intermediate value, but should never be exposed
+    /// outside chrono.
+    #[inline]
+    #[must_use]
+    pub(crate) fn overflowing_naive_local(&self) -> NaiveDateTime {
+        self.datetime.unchecked_add_offset(self.offset.fix())
     }
 
     /// Retrieve the elapsed years from now to the given [`DateTime`].
@@ -548,7 +576,8 @@ fn map_local<Tz: TimeZone, F>(dt: &DateTime<Tz>, mut f: F) -> Option<DateTime<Tz
 where
     F: FnMut(NaiveDateTime) -> Option<NaiveDateTime>,
 {
-    f(dt.naive_local()).and_then(|datetime| dt.timezone().from_local_datetime(&datetime).single())
+    f(dt.overflowing_naive_local())
+        .and_then(|datetime| dt.timezone().from_local_datetime(&datetime).single())
 }
 
 impl DateTime<FixedOffset> {
@@ -635,8 +664,12 @@ where
     #[must_use]
     pub fn to_rfc2822(&self) -> String {
         let mut result = String::with_capacity(32);
-        crate::format::write_rfc2822(&mut result, self.naive_local(), self.offset.fix())
-            .expect("writing rfc2822 datetime to string should never fail");
+        crate::format::write_rfc2822(
+            &mut result,
+            self.overflowing_naive_local(),
+            self.offset.fix(),
+        )
+        .expect("writing rfc2822 datetime to string should never fail");
         result
     }
 
@@ -646,8 +679,12 @@ where
     #[must_use]
     pub fn to_rfc3339(&self) -> String {
         let mut result = String::with_capacity(32);
-        crate::format::write_rfc3339(&mut result, self.naive_local(), self.offset.fix())
-            .expect("writing rfc3339 datetime to string should never fail");
+        crate::format::write_rfc3339(
+            &mut result,
+            self.overflowing_naive_local(),
+            self.offset.fix(),
+        )
+        .expect("writing rfc3339 datetime to string should never fail");
         result
     }
 
@@ -730,7 +767,7 @@ where
         I: Iterator<Item = B> + Clone,
         B: Borrow<Item<'a>>,
     {
-        let local = self.naive_local();
+        let local = self.overflowing_naive_local();
         DelayedFormat::new_with_offset(Some(local.date()), Some(local.time()), &self.offset, items)
     }
 
@@ -799,93 +836,124 @@ where
 impl<Tz: TimeZone> Datelike for DateTime<Tz> {
     #[inline]
     fn year(&self) -> i32 {
-        self.naive_local().year()
+        self.overflowing_naive_local().year()
     }
     #[inline]
     fn month(&self) -> u32 {
-        self.naive_local().month()
+        self.overflowing_naive_local().month()
     }
     #[inline]
     fn month0(&self) -> u32 {
-        self.naive_local().month0()
+        self.overflowing_naive_local().month0()
     }
     #[inline]
     fn day(&self) -> u32 {
-        self.naive_local().day()
+        self.overflowing_naive_local().day()
     }
     #[inline]
     fn day0(&self) -> u32 {
-        self.naive_local().day0()
+        self.overflowing_naive_local().day0()
     }
     #[inline]
     fn ordinal(&self) -> u32 {
-        self.naive_local().ordinal()
+        self.overflowing_naive_local().ordinal()
     }
     #[inline]
     fn ordinal0(&self) -> u32 {
-        self.naive_local().ordinal0()
+        self.overflowing_naive_local().ordinal0()
     }
     #[inline]
     fn weekday(&self) -> Weekday {
-        self.naive_local().weekday()
+        self.overflowing_naive_local().weekday()
     }
     #[inline]
     fn iso_week(&self) -> IsoWeek {
-        self.naive_local().iso_week()
+        self.overflowing_naive_local().iso_week()
     }
 
+    // Note on short-circuiting.
+    //
+    // The `with_*` methods have an interesting property: if the local `NaiveDateTime` would be
+    // out-of-range, there is only exactly one year/month/day/ordinal they can be set to that would
+    // result in a valid `DateTime`: the one that is already there.
+    // This is thanks to the restriction that offset is always less then 24h.
+    //
+    // To prevent creating an out-of-range `NaiveDateTime` all the following methods short-circuit
+    // when possible.
+
     #[inline]
     fn with_year(&self, year: i32) -> Option<DateTime<Tz>> {
+        if self.year() == year {
+            return Some(self.clone()); // See note on short-circuiting above.
+        }
         map_local(self, |datetime| datetime.with_year(year))
     }
 
     #[inline]
     fn with_month(&self, month: u32) -> Option<DateTime<Tz>> {
+        if self.month() == month {
+            return Some(self.clone()); // See note on short-circuiting above.
+        }
         map_local(self, |datetime| datetime.with_month(month))
     }
 
     #[inline]
     fn with_month0(&self, month0: u32) -> Option<DateTime<Tz>> {
+        if self.month0() == month0 {
+            return Some(self.clone()); // See note on short-circuiting above.
+        }
         map_local(self, |datetime| datetime.with_month0(month0))
     }
 
     #[inline]
     fn with_day(&self, day: u32) -> Option<DateTime<Tz>> {
+        if self.day() == day {
+            return Some(self.clone()); // See note on short-circuiting above.
+        }
         map_local(self, |datetime| datetime.with_day(day))
     }
 
     #[inline]
     fn with_day0(&self, day0: u32) -> Option<DateTime<Tz>> {
+        if self.day0() == day0 {
+            return Some(self.clone()); // See note on short-circuiting above.
+        }
         map_local(self, |datetime| datetime.with_day0(day0))
     }
 
     #[inline]
     fn with_ordinal(&self, ordinal: u32) -> Option<DateTime<Tz>> {
+        if self.ordinal() == ordinal {
+            return Some(self.clone()); // See note on short-circuiting above.
+        }
         map_local(self, |datetime| datetime.with_ordinal(ordinal))
     }
 
     #[inline]
     fn with_ordinal0(&self, ordinal0: u32) -> Option<DateTime<Tz>> {
+        if self.ordinal0() == ordinal0 {
+            return Some(self.clone()); // See note on short-circuiting above.
+        }
         map_local(self, |datetime| datetime.with_ordinal0(ordinal0))
     }
 }
 
 impl<Tz: TimeZone> Timelike for DateTime<Tz> {
     #[inline]
     fn hour(&self) -> u32 {
-        self.naive_local().hour()
+        self.overflowing_naive_local().hour()
     }
     #[inline]
     fn minute(&self) -> u32 {
-        self.naive_local().minute()
+        self.overflowing_naive_local().minute()
     }
     #[inline]
     fn second(&self) -> u32 {
-        self.naive_local().second()
+        self.overflowing_naive_local().second()
     }
     #[inline]
     fn nanosecond(&self) -> u32 {
-        self.naive_local().nanosecond()
+        self.overflowing_naive_local().nanosecond()
     }
 
     #[inline]
@@ -1055,7 +1123,7 @@ impl<Tz: TimeZone> Sub<Days> for DateTime<Tz> {
 
 impl<Tz: TimeZone> fmt::Debug for DateTime<Tz> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        self.naive_local().fmt(f)?;
+        self.overflowing_naive_local().fmt(f)?;
         self.offset.fmt(f)
     }
 }

src/naive/date.rs

@@ -578,7 +578,7 @@ impl NaiveDate {
     /// ```
     #[must_use]
     pub fn checked_add_months(self, months: Months) -> Option<Self> {
-        if months.0 == 0 {
+        if months.0 == 0 && self >= Self::MIN && self <= Self::MAX {
             return Some(self);
         }
 
@@ -609,7 +609,7 @@ impl NaiveDate {
     /// ```
     #[must_use]
     pub fn checked_sub_months(self, months: Months) -> Option<Self> {
-        if months.0 == 0 {
+        if months.0 == 0 && self >= Self::MIN && self <= Self::MAX {
             return Some(self);
         }
 

src/naive/datetime/mod.rs

@@ -641,6 +641,17 @@ impl NaiveDateTime {
         self.date.add_days(days, false).map(|date| NaiveDateTime { date, time })
     }
 
+    /// Adds given [`FixedOffset`] to the current datetime.
+    /// The resulting value may be outside the valid range of [`NaiveDateTime`].
+    ///
+    /// FOR INTERNAL USE ONLY.
+    #[must_use]
+    pub(crate) fn unchecked_add_offset(self, rhs: FixedOffset) -> NaiveDateTime {
+        let (time, days) = self.time.overflowing_add_offset(rhs);
+        let date = self.date.add_days(days, true).unwrap();
+        NaiveDateTime { date, time }
+    }
+
     /// Subtracts given `Duration` from the current date and time.
     ///
     /// As a part of Chrono's [leap second handling](./struct.NaiveTime.html#leap-second-handling),

src/naive/datetime/tests.rs

@@ -435,3 +435,29 @@ fn test_checked_sub_offset() {
     // out of range
     assert!(NaiveDateTime::MAX.checked_sub_offset(negative_offset).is_none());
 }
+
+#[test]
+fn test_unchecked_add_offset() {
+    let ymdhmsm = |y, m, d, h, mn, s, mi| {
+        NaiveDate::from_ymd_opt(y, m, d).unwrap().and_hms_milli_opt(h, mn, s, mi).unwrap()
+    };
+    let positive_offset = FixedOffset::east_opt(2 * 60 * 60).unwrap();
+    // regular date
+    let dt = ymdhmsm(2023, 5, 5, 20, 10, 0, 0);
+    assert_eq!(dt.unchecked_add_offset(positive_offset), ymdhmsm(2023, 5, 5, 22, 10, 0, 0));
+    // leap second is preserved
+    let dt = ymdhmsm(2023, 6, 30, 23, 59, 59, 1_000);
+    assert_eq!(dt.unchecked_add_offset(positive_offset), ymdhmsm(2023, 7, 1, 1, 59, 59, 1_000));
+    // out of range
+    assert!(NaiveDateTime::MAX.unchecked_add_offset(positive_offset) > NaiveDateTime::MAX);
+
+    let negative_offset = FixedOffset::west_opt(2 * 60 * 60).unwrap();
+    // regular date
+    let dt = ymdhmsm(2023, 5, 5, 20, 10, 0, 0);
+    assert_eq!(dt.unchecked_add_offset(negative_offset), ymdhmsm(2023, 5, 5, 18, 10, 0, 0));
+    // leap second is preserved
+    let dt = ymdhmsm(2023, 6, 30, 23, 59, 59, 1_000);
+    assert_eq!(dt.unchecked_add_offset(negative_offset), ymdhmsm(2023, 6, 30, 21, 59, 59, 1_000));
+    // out of range
+    assert!(NaiveDateTime::MIN.unchecked_add_offset(negative_offset) < NaiveDateTime::MIN);
+}