Skip to content

Commit

Permalink
Fix creating a DateTime with NaiveDateTime::{MIN, MAX}
Browse files Browse the repository at this point in the history
  • Loading branch information
pitdicker committed May 4, 2023
1 parent 88c23e6 commit 87b45ba
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 9 deletions.
17 changes: 16 additions & 1 deletion src/offset/local/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ mod tests {
use super::Local;
use crate::offset::local::TzInfo;
use crate::offset::TimeZone;
use crate::{Datelike, Duration, FixedOffset, LocalResult, NaiveDate, Utc};
use crate::{Datelike, Duration, FixedOffset, LocalResult, NaiveDate, NaiveDateTime, Utc};

#[test]
fn verify_correct_offsets() {
Expand Down Expand Up @@ -342,6 +342,21 @@ mod tests {
assert_eq!(Local.with_ymd_and_hms(2999, 12, 28, 0, 0, 0).unwrap().day(), 28);
}

#[test]
fn test_local_min_max_dates() {
let local_max = Local.from_utc_datetime(&NaiveDateTime::MAX);
assert_eq!(local_max.naive_utc(), NaiveDateTime::MAX);
let local_min = Local.from_utc_datetime(&NaiveDateTime::MIN);
assert_eq!(local_min.naive_utc(), NaiveDateTime::MIN);

if let Some(local_max) = Local.from_local_datetime(&NaiveDateTime::MAX).single() {
assert_eq!(local_max.naive_local(), NaiveDateTime::MAX);
}
if let Some(local_min) = Local.from_local_datetime(&NaiveDateTime::MIN).single() {
assert_eq!(local_min.naive_local(), NaiveDateTime::MIN);
}
}

#[test]
fn test_leap_second() {
// issue #123
Expand Down
8 changes: 5 additions & 3 deletions src/offset/local/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use std::{cell::RefCell, collections::hash_map, env, fs, hash::Hasher, time::Sys

use super::tz_info::TimeZone;
use super::{DateTime, Local, NaiveDateTime};
use crate::offset::local_time_min_offset;
use crate::{Datelike, LocalResult};

pub(super) fn naive_to_local(d: &NaiveDateTime, local: bool) -> LocalResult<DateTime<Local>> {
Expand Down Expand Up @@ -152,9 +153,10 @@ impl Cache {

// we pass through the year as the year of a local point in time must either be valid in that locale, or
// the entire time was skipped in which case we will return LocalResult::None anyway.
self.zone
let local_result_offset = self
.zone
.find_local_offset_from_local(d.timestamp(), d.year())
.expect("unable to select local time type")
.map(|offset| DateTime::from_utc(d - offset, offset))
.expect("unable to select local time type");
local_time_min_offset(local_result_offset, &d).unwrap_or(LocalResult::None)
}
}
7 changes: 5 additions & 2 deletions src/offset/local/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ use winapi::um::minwinbase::SYSTEMTIME;
use winapi::um::timezoneapi::{GetTimeZoneInformationForYear, TIME_ZONE_INFORMATION};

use super::{FixedOffset, Local, TzInfo};
use crate::{DateTime, Datelike, LocalResult, NaiveDate, NaiveDateTime, NaiveTime, Weekday};
use crate::offset::local_time_min_offset;
use crate::{
DateTime, Datelike, Duration, LocalResult, NaiveDate, NaiveDateTime, NaiveTime, Weekday,
};

pub(super) fn naive_to_local(d: &NaiveDateTime, local: bool) -> LocalResult<DateTime<Local>> {
let tz_info = match TzInfo::get_current_for_year(d.year()) {
Expand Down Expand Up @@ -95,7 +98,7 @@ impl TzInfo {
(Some(_), Some(_)) => self.lookup_with_dst_transitions(local_time),
_ => LocalResult::Single(self.std_offset),
};
local_result_offset.map(|offset| DateTime::from_utc(*local_time - offset, offset))
local_time_min_offset(local_result_offset, local_time).unwrap_or(LocalResult::None)
}
}

Expand Down
61 changes: 58 additions & 3 deletions src/offset/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use crate::format::{parse, ParseResult, Parsed, StrftimeItems};
use crate::naive::{NaiveDate, NaiveDateTime, NaiveTime};
use crate::Weekday;
#[allow(deprecated)]
use crate::{Date, DateTime};
use crate::{Date, DateTime, Duration, Timelike};

mod fixed;
pub use self::fixed::FixedOffset;
Expand Down Expand Up @@ -472,8 +472,8 @@ pub trait TimeZone: Sized + Clone {
/// Converts the local `NaiveDateTime` to the timezone-aware `DateTime` if possible.
#[allow(clippy::wrong_self_convention)]
fn from_local_datetime(&self, local: &NaiveDateTime) -> LocalResult<DateTime<Self>> {
self.offset_from_local_datetime(local)
.map(|offset| DateTime::from_utc(*local - offset.fix(), offset))
let local_result_offset = self.offset_from_local_datetime(local);
local_time_min_offset(local_result_offset, local).unwrap_or(LocalResult::None)
}

/// Creates the offset for given UTC `NaiveDate`. This cannot fail.
Expand All @@ -499,10 +499,65 @@ pub trait TimeZone: Sized + Clone {
}
}

/// Helper function to map `LocalResult<FixedOffset>` to `LocalResult<DateTime<Local>>`.
/// Returns `None` on out-of-range.
#[allow(dead_code)]
pub(crate) fn local_time_min_offset<Tz: TimeZone>(
local_result: LocalResult<Tz::Offset>,
local_time: &NaiveDateTime,
) -> Option<LocalResult<DateTime<Tz>>> {
// We temporarily extract and later add back the nanosecond part because
// `DateTime::checked_sub_signed()` normalizes leap_seconds, which we want to preserve.
fn try_apply_offset<Tz: TimeZone>(
local_time: &NaiveDateTime,
offset: Tz::Offset,
) -> Option<DateTime<Tz>> {
let nanos = local_time.nanosecond();
let local_time = local_time.with_nanosecond(0).unwrap();
let o = Duration::seconds(offset.fix().local_minus_utc() as i64);
let utc_time = local_time.checked_sub_signed(o)?.with_nanosecond(nanos).unwrap();
Some(DateTime::from_utc(utc_time, offset))
}
Some(match local_result {
LocalResult::None => LocalResult::None,
LocalResult::Single(offset) => LocalResult::Single(try_apply_offset(local_time, offset)?),
LocalResult::Ambiguous(o1, o2) => LocalResult::Ambiguous(
try_apply_offset(local_time, o1)?,
try_apply_offset(local_time, o2)?,
),
})
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_fixed_offset_min_max_dates() {
for offset_hour in -23..=23 {
dbg!(offset_hour);
let offset = FixedOffset::east_opt(offset_hour * 60 * 60).unwrap();

let local_max = offset.from_utc_datetime(&NaiveDateTime::MAX);
assert_eq!(local_max.naive_utc(), NaiveDateTime::MAX);
let local_min = offset.from_utc_datetime(&NaiveDateTime::MIN);
assert_eq!(local_min.naive_utc(), NaiveDateTime::MIN);

let local_max = offset.from_local_datetime(&NaiveDateTime::MAX);
if offset_hour >= 0 {
assert_eq!(local_max.unwrap().naive_local(), NaiveDateTime::MAX);
} else {
assert_eq!(local_max, LocalResult::None);
}
let local_min = offset.from_local_datetime(&NaiveDateTime::MIN);
if offset_hour <= 0 {
assert_eq!(local_min.unwrap().naive_local(), NaiveDateTime::MIN);
} else {
assert_eq!(local_min, LocalResult::None);
}
}
}

#[test]
fn test_negative_millis() {
let dt = Utc.timestamp_millis_opt(-1000).unwrap();
Expand Down

0 comments on commit 87b45ba

Please sign in to comment.