Skip to content

Commit b3d7ac0

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

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> {
@@ -669,8 +698,12 @@ where
669698
#[must_use]
670699
pub fn to_rfc2822(&self) -> String {
671700
let mut result = String::with_capacity(32);
672-
crate::format::write_rfc2822(&mut result, self.naive_local(), self.offset.fix())
673-
.expect("writing rfc2822 datetime to string should never fail");
701+
crate::format::write_rfc2822(
702+
&mut result,
703+
self.overflowing_naive_local(),
704+
self.offset.fix(),
705+
)
706+
.expect("writing rfc2822 datetime to string should never fail");
674707
result
675708
}
676709

@@ -680,8 +713,12 @@ where
680713
#[must_use]
681714
pub fn to_rfc3339(&self) -> String {
682715
let mut result = String::with_capacity(32);
683-
crate::format::write_rfc3339(&mut result, self.naive_local(), self.offset.fix())
684-
.expect("writing rfc3339 datetime to string should never fail");
716+
crate::format::write_rfc3339(
717+
&mut result,
718+
self.overflowing_naive_local(),
719+
self.offset.fix(),
720+
)
721+
.expect("writing rfc3339 datetime to string should never fail");
685722
result
686723
}
687724

@@ -764,7 +801,7 @@ where
764801
I: Iterator<Item = B> + Clone,
765802
B: Borrow<Item<'a>>,
766803
{
767-
let local = self.naive_local();
804+
let local = self.overflowing_naive_local();
768805
DelayedFormat::new_with_offset(Some(local.date()), Some(local.time()), &self.offset, items)
769806
}
770807

@@ -833,93 +870,124 @@ where
833870
impl<Tz: TimeZone> Datelike for DateTime<Tz> {
834871
#[inline]
835872
fn year(&self) -> i32 {
836-
self.naive_local().year()
873+
self.overflowing_naive_local().year()
837874
}
838875
#[inline]
839876
fn month(&self) -> u32 {
840-
self.naive_local().month()
877+
self.overflowing_naive_local().month()
841878
}
842879
#[inline]
843880
fn month0(&self) -> u32 {
844-
self.naive_local().month0()
881+
self.overflowing_naive_local().month0()
845882
}
846883
#[inline]
847884
fn day(&self) -> u32 {
848-
self.naive_local().day()
885+
self.overflowing_naive_local().day()
849886
}
850887
#[inline]
851888
fn day0(&self) -> u32 {
852-
self.naive_local().day0()
889+
self.overflowing_naive_local().day0()
853890
}
854891
#[inline]
855892
fn ordinal(&self) -> u32 {
856-
self.naive_local().ordinal()
893+
self.overflowing_naive_local().ordinal()
857894
}
858895
#[inline]
859896
fn ordinal0(&self) -> u32 {
860-
self.naive_local().ordinal0()
897+
self.overflowing_naive_local().ordinal0()
861898
}
862899
#[inline]
863900
fn weekday(&self) -> Weekday {
864-
self.naive_local().weekday()
901+
self.overflowing_naive_local().weekday()
865902
}
866903
#[inline]
867904
fn iso_week(&self) -> IsoWeek {
868-
self.naive_local().iso_week()
905+
self.overflowing_naive_local().iso_week()
869906
}
870907

908+
// Note on short-circuiting.
909+
//
910+
// The `with_*` methods have an interesting property: if the local `NaiveDateTime` would be
911+
// out-of-range, there is only exactly one year/month/day/ordinal they can be set to that would
912+
// result in a valid `DateTime`: the one that is already there.
913+
// This is thanks to the restriction that offset is always less then 24h.
914+
//
915+
// To prevent creating an out-of-range `NaiveDateTime` all the following methods short-circuit
916+
// when possible.
917+
871918
#[inline]
872919
fn with_year(&self, year: i32) -> Option<DateTime<Tz>> {
920+
if self.year() == year {
921+
return Some(self.clone()); // See note on short-circuiting above.
922+
}
873923
map_local(self, |datetime| datetime.with_year(year))
874924
}
875925

876926
#[inline]
877927
fn with_month(&self, month: u32) -> Option<DateTime<Tz>> {
928+
if self.month() == month {
929+
return Some(self.clone()); // See note on short-circuiting above.
930+
}
878931
map_local(self, |datetime| datetime.with_month(month))
879932
}
880933

881934
#[inline]
882935
fn with_month0(&self, month0: u32) -> Option<DateTime<Tz>> {
936+
if self.month0() == month0 {
937+
return Some(self.clone()); // See note on short-circuiting above.
938+
}
883939
map_local(self, |datetime| datetime.with_month0(month0))
884940
}
885941

886942
#[inline]
887943
fn with_day(&self, day: u32) -> Option<DateTime<Tz>> {
944+
if self.day() == day {
945+
return Some(self.clone()); // See note on short-circuiting above.
946+
}
888947
map_local(self, |datetime| datetime.with_day(day))
889948
}
890949

891950
#[inline]
892951
fn with_day0(&self, day0: u32) -> Option<DateTime<Tz>> {
952+
if self.day0() == day0 {
953+
return Some(self.clone()); // See note on short-circuiting above.
954+
}
893955
map_local(self, |datetime| datetime.with_day0(day0))
894956
}
895957

896958
#[inline]
897959
fn with_ordinal(&self, ordinal: u32) -> Option<DateTime<Tz>> {
960+
if self.ordinal() == ordinal {
961+
return Some(self.clone()); // See note on short-circuiting above.
962+
}
898963
map_local(self, |datetime| datetime.with_ordinal(ordinal))
899964
}
900965

901966
#[inline]
902967
fn with_ordinal0(&self, ordinal0: u32) -> Option<DateTime<Tz>> {
968+
if self.ordinal0() == ordinal0 {
969+
return Some(self.clone()); // See note on short-circuiting above.
970+
}
903971
map_local(self, |datetime| datetime.with_ordinal0(ordinal0))
904972
}
905973
}
906974

907975
impl<Tz: TimeZone> Timelike for DateTime<Tz> {
908976
#[inline]
909977
fn hour(&self) -> u32 {
910-
self.naive_local().hour()
978+
self.overflowing_naive_local().hour()
911979
}
912980
#[inline]
913981
fn minute(&self) -> u32 {
914-
self.naive_local().minute()
982+
self.overflowing_naive_local().minute()
915983
}
916984
#[inline]
917985
fn second(&self) -> u32 {
918-
self.naive_local().second()
986+
self.overflowing_naive_local().second()
919987
}
920988
#[inline]
921989
fn nanosecond(&self) -> u32 {
922-
self.naive_local().nanosecond()
990+
self.overflowing_naive_local().nanosecond()
923991
}
924992

925993
#[inline]
@@ -1089,7 +1157,7 @@ impl<Tz: TimeZone> Sub<Days> for DateTime<Tz> {
10891157

10901158
impl<Tz: TimeZone> fmt::Debug for DateTime<Tz> {
10911159
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
1092-
self.naive_local().fmt(f)?;
1160+
self.overflowing_naive_local().fmt(f)?;
10931161
self.offset.fmt(f)
10941162
}
10951163
}

‎src/naive/date.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -605,7 +605,7 @@ impl NaiveDate {
605605
/// ```
606606
#[must_use]
607607
pub fn checked_add_months(self, months: Months) -> Option<Self> {
608-
if months.0 == 0 {
608+
if months.0 == 0 && self >= Self::MIN && self <= Self::MAX {
609609
return Some(self);
610610
}
611611

@@ -636,7 +636,7 @@ impl NaiveDate {
636636
/// ```
637637
#[must_use]
638638
pub fn checked_sub_months(self, months: Months) -> Option<Self> {
639-
if months.0 == 0 {
639+
if months.0 == 0 && self >= Self::MIN && self <= Self::MAX {
640640
return Some(self);
641641
}
642642

‎src/naive/datetime/mod.rs‎

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

670+
/// Adds given [`FixedOffset`] to the current datetime.
671+
/// The resulting value may be outside the valid range of [`NaiveDateTime`].
672+
///
673+
/// FOR INTERNAL USE ONLY.
674+
#[must_use]
675+
pub(crate) fn unchecked_add_offset(self, rhs: FixedOffset) -> NaiveDateTime {
676+
let (time, days) = self.time.overflowing_add_offset(rhs);
677+
let date = self.date.add_days(days, true).unwrap();
678+
NaiveDateTime { date, time }
679+
}
680+
670681
/// Subtracts given `Duration` from the current date and time.
671682
///
672683
/// 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
@@ -443,3 +443,29 @@ fn test_checked_sub_offset() {
443443
// out of range
444444
assert!(NaiveDateTime::MAX.checked_sub_offset(negative_offset).is_none());
445445
}
446+
447+
#[test]
448+
fn test_unchecked_add_offset() {
449+
let ymdhmsm = |y, m, d, h, mn, s, mi| {
450+
NaiveDate::from_ymd_opt(y, m, d).unwrap().and_hms_milli_opt(h, mn, s, mi).unwrap()
451+
};
452+
let positive_offset = FixedOffset::east_opt(2 * 60 * 60).unwrap();
453+
// regular date
454+
let dt = ymdhmsm(2023, 5, 5, 20, 10, 0, 0);
455+
assert_eq!(dt.unchecked_add_offset(positive_offset), ymdhmsm(2023, 5, 5, 22, 10, 0, 0));
456+
// leap second is preserved
457+
let dt = ymdhmsm(2023, 6, 30, 23, 59, 59, 1_000);
458+
assert_eq!(dt.unchecked_add_offset(positive_offset), ymdhmsm(2023, 7, 1, 1, 59, 59, 1_000));
459+
// out of range
460+
assert!(NaiveDateTime::MAX.unchecked_add_offset(positive_offset) > NaiveDateTime::MAX);
461+
462+
let negative_offset = FixedOffset::west_opt(2 * 60 * 60).unwrap();
463+
// regular date
464+
let dt = ymdhmsm(2023, 5, 5, 20, 10, 0, 0);
465+
assert_eq!(dt.unchecked_add_offset(negative_offset), ymdhmsm(2023, 5, 5, 18, 10, 0, 0));
466+
// leap second is preserved
467+
let dt = ymdhmsm(2023, 6, 30, 23, 59, 59, 1_000);
468+
assert_eq!(dt.unchecked_add_offset(negative_offset), ymdhmsm(2023, 6, 30, 21, 59, 59, 1_000));
469+
// out of range
470+
assert!(NaiveDateTime::MIN.unchecked_add_offset(negative_offset) < NaiveDateTime::MIN);
471+
}

0 commit comments

Comments
 (0)