Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix out-of-range panic in NaiveWeek::last_day #1070

Merged
merged 2 commits into from
May 19, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 31 additions & 14 deletions src/naive/date.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,13 @@ impl NaiveWeek {
#[inline]
#[must_use]
pub fn first_day(&self) -> NaiveDate {
let start = self.start.num_days_from_monday();
let end = self.date.weekday().num_days_from_monday();
let days = if start > end { 7 - start + end } else { end - start };
self.date - Duration::days(days.into())
let start = self.start.num_days_from_monday() as i32;
let ref_day = self.date.weekday().num_days_from_monday() as i32;
// Calculate the number of days to subtract from `self.date`.
// Do not construct an intermediate date beyond `self.date`, because that may be out of
// range if `date` is close to `NaiveDate::MAX`.
let days = start - ref_day - if start > ref_day { 7 } else { 0 };
self.date.diff_days(days as i64).unwrap()
}

/// Returns a date representing the last day of the week.
Expand All @@ -99,7 +102,13 @@ impl NaiveWeek {
#[inline]
#[must_use]
pub fn last_day(&self) -> NaiveDate {
self.first_day() + Duration::days(6)
let end = self.start.pred().num_days_from_monday() as i32;
let ref_day = self.date.weekday().num_days_from_monday() as i32;
// Calculate the number of days to add to `self.date`.
// Do not construct an intermediate date before `self.date` (like with `first_day()`),
// because that may be out of range if `date` is close to `NaiveDate::MIN`.
let days = end - ref_day + if end < ref_day { 7 } else { 0 };
self.date.diff_days(days as i64).unwrap()
}

/// Returns a [`RangeInclusive<T>`] representing the whole week bounded by
Expand Down Expand Up @@ -3019,23 +3028,31 @@ mod tests {
fn test_naiveweek() {
let date = NaiveDate::from_ymd_opt(2022, 5, 18).unwrap();
let asserts = vec![
(Weekday::Mon, "2022-05-16", "2022-05-22"),
(Weekday::Tue, "2022-05-17", "2022-05-23"),
(Weekday::Wed, "2022-05-18", "2022-05-24"),
(Weekday::Thu, "2022-05-12", "2022-05-18"),
(Weekday::Fri, "2022-05-13", "2022-05-19"),
(Weekday::Sat, "2022-05-14", "2022-05-20"),
(Weekday::Sun, "2022-05-15", "2022-05-21"),
(Weekday::Mon, "Mon 2022-05-16", "Sun 2022-05-22"),
(Weekday::Tue, "Tue 2022-05-17", "Mon 2022-05-23"),
(Weekday::Wed, "Wed 2022-05-18", "Tue 2022-05-24"),
(Weekday::Thu, "Thu 2022-05-12", "Wed 2022-05-18"),
(Weekday::Fri, "Fri 2022-05-13", "Thu 2022-05-19"),
(Weekday::Sat, "Sat 2022-05-14", "Fri 2022-05-20"),
(Weekday::Sun, "Sun 2022-05-15", "Sat 2022-05-21"),
];
for (start, first_day, last_day) in asserts {
let week = date.week(start);
let days = week.days();
assert_eq!(Ok(week.first_day()), NaiveDate::parse_from_str(first_day, "%Y-%m-%d"));
assert_eq!(Ok(week.last_day()), NaiveDate::parse_from_str(last_day, "%Y-%m-%d"));
assert_eq!(Ok(week.first_day()), NaiveDate::parse_from_str(first_day, "%a %Y-%m-%d"));
djc marked this conversation as resolved.
Show resolved Hide resolved
assert_eq!(Ok(week.last_day()), NaiveDate::parse_from_str(last_day, "%a %Y-%m-%d"));
assert!(days.contains(&date));
}
}

#[test]
fn test_naiveweek_min_max() {
let date_max = NaiveDate::MAX;
assert!(date_max.week(Weekday::Mon).first_day() <= date_max);
let date_min = NaiveDate::MIN;
assert!(date_min.week(Weekday::Mon).last_day() >= date_min);
}

#[test]
fn test_weeks_from() {
// tests per: https://github.com/chronotope/chrono/issues/961
Expand Down