Skip to content

Commit f71b3bd

Browse files
pitdickerpitdicker
committed
Fix all methods that would panic on out-of-range local datetime
1 parent daefe53 commit f71b3bd

File tree

4 files changed

+132
-27
lines changed

4 files changed

+132
-27
lines changed

‎src/datetime/mod.rs‎

Lines changed: 93 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,14 @@ impl<Tz: TimeZone> DateTime<Tz> {
168168
/// [`NaiveDate`] is a more well-defined type, and has more traits implemented on it,
169169
/// so should be preferred to [`Date`] any time you truly want to operate on Dates.
170170
///
171+
/// # Panics
172+
///
173+
/// [`DateTime`] internally stores the date and time in UTC with a [`NaiveDateTime`]. This
174+
/// method will panic if the offset from UTC would push the local datetime outside of the
175+
/// representable range of a [`DateTime`].
176+
///
177+
/// # Example
178+
///
171179
/// ```
172180
/// use chrono::prelude::*;
173181
///
@@ -352,7 +360,7 @@ impl<Tz: TimeZone> DateTime<Tz> {
352360
/// See [`NaiveDate::checked_add_months`] for more details on behavior
353361
#[must_use]
354362
pub fn checked_add_months(self, rhs: Months) -> Option<DateTime<Tz>> {
355-
self.naive_local()
363+
self.overflowing_naive_local()
356364
.checked_add_months(rhs)?
357365
.and_local_timezone(Tz::from_offset(&self.offset))
358366
.single()
@@ -377,7 +385,7 @@ impl<Tz: TimeZone> DateTime<Tz> {
377385
/// See [`NaiveDate::checked_sub_months`] for more details on behavior
378386
#[must_use]
379387
pub fn checked_sub_months(self, rhs: Months) -> Option<DateTime<Tz>> {
380-
self.naive_local()
388+
self.overflowing_naive_local()
381389
.checked_sub_months(rhs)?
382390
.and_local_timezone(Tz::from_offset(&self.offset))
383391
.single()
@@ -388,7 +396,7 @@ impl<Tz: TimeZone> DateTime<Tz> {
388396
/// Returns `None` if the resulting date would be out of range.
389397
#[must_use]
390398
pub fn checked_add_days(self, days: Days) -> Option<Self> {
391-
self.naive_local()
399+
self.overflowing_naive_local()
392400
.checked_add_days(days)?
393401
.and_local_timezone(TimeZone::from_offset(&self.offset))
394402
.single()
@@ -399,7 +407,7 @@ impl<Tz: TimeZone> DateTime<Tz> {
399407
/// Returns `None` if the resulting date would be out of range.
400408
#[must_use]
401409
pub fn checked_sub_days(self, days: Days) -> Option<Self> {
402-
self.naive_local()
410+
self.overflowing_naive_local()
403411
.checked_sub_days(days)?
404412
.and_local_timezone(TimeZone::from_offset(&self.offset))
405413
.single()
@@ -421,10 +429,30 @@ impl<Tz: TimeZone> DateTime<Tz> {
421429
}
422430

423431
/// Returns a view to the naive local datetime.
432+
///
433+
/// # Panics
434+
///
435+
/// [`DateTime`] internally stores the date and time in UTC with a [`NaiveDateTime`]. This
436+
/// method will panic if the offset from UTC would push the local datetime outside of the
437+
/// representable range of a [`NaiveDateTime`].
424438
#[inline]
425439
#[must_use]
426440
pub fn naive_local(&self) -> NaiveDateTime {
427-
self.datetime + self.offset.fix()
441+
self.datetime
442+
.checked_add_offset(self.offset.fix())
443+
.expect("Local time out of range for `NaiveDateTime`")
444+
}
445+
446+
/// Returns a view to the naive local datetime.
447+
///
448+
/// FOR INTERNAL USE ONLY.
449+
/// This makes use of the buffer space outside of the representable range of values of
450+
/// `NaiveDateTime`. The result can be user as intermediate value, but should never be exposed
451+
/// outside chrono.
452+
#[inline]
453+
#[must_use]
454+
pub(crate) fn overflowing_naive_local(&self) -> NaiveDateTime {
455+
self.datetime.unchecked_add_offset(self.offset.fix())
428456
}
429457

430458
/// 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
548576
where
549577
F: FnMut(NaiveDateTime) -> Option<NaiveDateTime>,
550578
{
551-
f(dt.naive_local()).and_then(|datetime| dt.timezone().from_local_datetime(&datetime).single())
579+
f(dt.overflowing_naive_local())
580+
.and_then(|datetime| dt.timezone().from_local_datetime(&datetime).single())
552581
}
553582

554583
impl DateTime<FixedOffset> {
@@ -635,8 +664,12 @@ where
635664
#[must_use]
636665
pub fn to_rfc2822(&self) -> String {
637666
let mut result = String::with_capacity(32);
638-
crate::format::write_rfc2822(&mut result, self.naive_local(), self.offset.fix())
639-
.expect("writing rfc2822 datetime to string should never fail");
667+
crate::format::write_rfc2822(
668+
&mut result,
669+
self.overflowing_naive_local(),
670+
self.offset.fix(),
671+
)
672+
.expect("writing rfc2822 datetime to string should never fail");
640673
result
641674
}
642675

@@ -646,8 +679,12 @@ where
646679
#[must_use]
647680
pub fn to_rfc3339(&self) -> String {
648681
let mut result = String::with_capacity(32);
649-
crate::format::write_rfc3339(&mut result, self.naive_local(), self.offset.fix())
650-
.expect("writing rfc3339 datetime to string should never fail");
682+
crate::format::write_rfc3339(
683+
&mut result,
684+
self.overflowing_naive_local(),
685+
self.offset.fix(),
686+
)
687+
.expect("writing rfc3339 datetime to string should never fail");
651688
result
652689
}
653690

@@ -730,7 +767,7 @@ where
730767
I: Iterator<Item = B> + Clone,
731768
B: Borrow<Item<'a>>,
732769
{
733-
let local = self.naive_local();
770+
let local = self.overflowing_naive_local();
734771
DelayedFormat::new_with_offset(Some(local.date()), Some(local.time()), &self.offset, items)
735772
}
736773

@@ -799,93 +836,124 @@ where
799836
impl<Tz: TimeZone> Datelike for DateTime<Tz> {
800837
#[inline]
801838
fn year(&self) -> i32 {
802-
self.naive_local().year()
839+
self.overflowing_naive_local().year()
803840
}
804841
#[inline]
805842
fn month(&self) -> u32 {
806-
self.naive_local().month()
843+
self.overflowing_naive_local().month()
807844
}
808845
#[inline]
809846
fn month0(&self) -> u32 {
810-
self.naive_local().month0()
847+
self.overflowing_naive_local().month0()
811848
}
812849
#[inline]
813850
fn day(&self) -> u32 {
814-
self.naive_local().day()
851+
self.overflowing_naive_local().day()
815852
}
816853
#[inline]
817854
fn day0(&self) -> u32 {
818-
self.naive_local().day0()
855+
self.overflowing_naive_local().day0()
819856
}
820857
#[inline]
821858
fn ordinal(&self) -> u32 {
822-
self.naive_local().ordinal()
859+
self.overflowing_naive_local().ordinal()
823860
}
824861
#[inline]
825862
fn ordinal0(&self) -> u32 {
826-
self.naive_local().ordinal0()
863+
self.overflowing_naive_local().ordinal0()
827864
}
828865
#[inline]
829866
fn weekday(&self) -> Weekday {
830-
self.naive_local().weekday()
867+
self.overflowing_naive_local().weekday()
831868
}
832869
#[inline]
833870
fn iso_week(&self) -> IsoWeek {
834-
self.naive_local().iso_week()
871+
self.overflowing_naive_local().iso_week()
835872
}
836873

874+
// Note on short-circuiting.
875+
//
876+
// The `with_*` methods have an interesting property: if the local `NaiveDateTime` would be
877+
// out-of-range, there is only exactly one year/month/day/ordinal they can be set to that would
878+
// result in a valid `DateTime`: the one that is already there.
879+
// This is thanks to the restriction that offset is always less then 24h.
880+
//
881+
// To prevent creating an out-of-range `NaiveDateTime` all the following methods short-circuit
882+
// when possible.
883+
837884
#[inline]
838885
fn with_year(&self, year: i32) -> Option<DateTime<Tz>> {
886+
if self.year() == year {
887+
return Some(self.clone()); // See note on short-circuiting above.
888+
}
839889
map_local(self, |datetime| datetime.with_year(year))
840890
}
841891

842892
#[inline]
843893
fn with_month(&self, month: u32) -> Option<DateTime<Tz>> {
894+
if self.month() == month {
895+
return Some(self.clone()); // See note on short-circuiting above.
896+
}
844897
map_local(self, |datetime| datetime.with_month(month))
845898
}
846899

847900
#[inline]
848901
fn with_month0(&self, month0: u32) -> Option<DateTime<Tz>> {
902+
if self.month0() == month0 {
903+
return Some(self.clone()); // See note on short-circuiting above.
904+
}
849905
map_local(self, |datetime| datetime.with_month0(month0))
850906
}
851907

852908
#[inline]
853909
fn with_day(&self, day: u32) -> Option<DateTime<Tz>> {
910+
if self.day() == day {
911+
return Some(self.clone()); // See note on short-circuiting above.
912+
}
854913
map_local(self, |datetime| datetime.with_day(day))
855914
}
856915

857916
#[inline]
858917
fn with_day0(&self, day0: u32) -> Option<DateTime<Tz>> {
918+
if self.day0() == day0 {
919+
return Some(self.clone()); // See note on short-circuiting above.
920+
}
859921
map_local(self, |datetime| datetime.with_day0(day0))
860922
}
861923

862924
#[inline]
863925
fn with_ordinal(&self, ordinal: u32) -> Option<DateTime<Tz>> {
926+
if self.ordinal() == ordinal {
927+
return Some(self.clone()); // See note on short-circuiting above.
928+
}
864929
map_local(self, |datetime| datetime.with_ordinal(ordinal))
865930
}
866931

867932
#[inline]
868933
fn with_ordinal0(&self, ordinal0: u32) -> Option<DateTime<Tz>> {
934+
if self.ordinal0() == ordinal0 {
935+
return Some(self.clone()); // See note on short-circuiting above.
936+
}
869937
map_local(self, |datetime| datetime.with_ordinal0(ordinal0))
870938
}
871939
}
872940

873941
impl<Tz: TimeZone> Timelike for DateTime<Tz> {
874942
#[inline]
875943
fn hour(&self) -> u32 {
876-
self.naive_local().hour()
944+
self.overflowing_naive_local().hour()
877945
}
878946
#[inline]
879947
fn minute(&self) -> u32 {
880-
self.naive_local().minute()
948+
self.overflowing_naive_local().minute()
881949
}
882950
#[inline]
883951
fn second(&self) -> u32 {
884-
self.naive_local().second()
952+
self.overflowing_naive_local().second()
885953
}
886954
#[inline]
887955
fn nanosecond(&self) -> u32 {
888-
self.naive_local().nanosecond()
956+
self.overflowing_naive_local().nanosecond()
889957
}
890958

891959
#[inline]
@@ -1055,7 +1123,7 @@ impl<Tz: TimeZone> Sub<Days> for DateTime<Tz> {
10551123

10561124
impl<Tz: TimeZone> fmt::Debug for DateTime<Tz> {
10571125
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
1058-
self.naive_local().fmt(f)?;
1126+
self.overflowing_naive_local().fmt(f)?;
10591127
self.offset.fmt(f)
10601128
}
10611129
}

‎src/naive/date.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,7 @@ impl NaiveDate {
577577
/// ```
578578
#[must_use]
579579
pub fn checked_add_months(self, months: Months) -> Option<Self> {
580-
if months.0 == 0 {
580+
if months.0 == 0 && self >= Self::MIN && self <= Self::MAX {
581581
return Some(self);
582582
}
583583

@@ -608,7 +608,7 @@ impl NaiveDate {
608608
/// ```
609609
#[must_use]
610610
pub fn checked_sub_months(self, months: Months) -> Option<Self> {
611-
if months.0 == 0 {
611+
if months.0 == 0 && self >= Self::MIN && self <= Self::MAX {
612612
return Some(self);
613613
}
614614

‎src/naive/datetime/mod.rs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -641,6 +641,17 @@ impl NaiveDateTime {
641641
self.date.add_days(days, false).map(|date| NaiveDateTime { date, time })
642642
}
643643

644+
/// Adds given [`FixedOffset`] to the current datetime.
645+
/// The resulting value may be outside the valid range of [`NaiveDateTime`].
646+
///
647+
/// FOR INTERNAL USE ONLY.
648+
#[must_use]
649+
pub(crate) fn unchecked_add_offset(self, rhs: FixedOffset) -> NaiveDateTime {
650+
let (time, days) = self.time.overflowing_add_offset(rhs);
651+
let date = self.date.add_days(days, true).unwrap();
652+
NaiveDateTime { date, time }
653+
}
654+
644655
/// Subtracts given `Duration` from the current date and time.
645656
///
646657
/// As a part of Chrono's [leap second handling](./struct.NaiveTime.html#leap-second-handling),

‎src/naive/datetime/tests.rs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,3 +435,29 @@ fn test_checked_sub_offset() {
435435
// out of range
436436
assert!(NaiveDateTime::MAX.checked_sub_offset(negative_offset).is_none());
437437
}
438+
439+
#[test]
440+
fn test_unchecked_add_offset() {
441+
let ymdhmsm = |y, m, d, h, mn, s, mi| {
442+
NaiveDate::from_ymd_opt(y, m, d).unwrap().and_hms_milli_opt(h, mn, s, mi).unwrap()
443+
};
444+
let positive_offset = FixedOffset::east_opt(2 * 60 * 60).unwrap();
445+
// regular date
446+
let dt = ymdhmsm(2023, 5, 5, 20, 10, 0, 0);
447+
assert_eq!(dt.unchecked_add_offset(positive_offset), ymdhmsm(2023, 5, 5, 22, 10, 0, 0));
448+
// leap second is preserved
449+
let dt = ymdhmsm(2023, 6, 30, 23, 59, 59, 1_000);
450+
assert_eq!(dt.unchecked_add_offset(positive_offset), ymdhmsm(2023, 7, 1, 1, 59, 59, 1_000));
451+
// out of range
452+
assert!(NaiveDateTime::MAX.unchecked_add_offset(positive_offset) > NaiveDateTime::MAX);
453+
454+
let negative_offset = FixedOffset::west_opt(2 * 60 * 60).unwrap();
455+
// regular date
456+
let dt = ymdhmsm(2023, 5, 5, 20, 10, 0, 0);
457+
assert_eq!(dt.unchecked_add_offset(negative_offset), ymdhmsm(2023, 5, 5, 18, 10, 0, 0));
458+
// leap second is preserved
459+
let dt = ymdhmsm(2023, 6, 30, 23, 59, 59, 1_000);
460+
assert_eq!(dt.unchecked_add_offset(negative_offset), ymdhmsm(2023, 6, 30, 21, 59, 59, 1_000));
461+
// out of range
462+
assert!(NaiveDateTime::MIN.unchecked_add_offset(negative_offset) < NaiveDateTime::MIN);
463+
}

0 commit comments

Comments
 (0)