From 605495ada5736bae1155beee1247f90460568ddc Mon Sep 17 00:00:00 2001 From: Jacob Pratt Date: Tue, 19 Dec 2023 04:11:59 -0500 Subject: [PATCH] =?UTF-8?q?Expand=20range=20of=20`UtcOffset`=20to=20=C2=B1?= =?UTF-8?q?25:59:59?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This supports the full POSIX range and permits infallible negation. --- tests/parsing.rs | 2 +- tests/serde/mod.rs | 8 ++-- time/src/formatting/formattable.rs | 3 ++ time/src/parsing/mod.rs | 7 ++++ time/src/parsing/parsable.rs | 22 +++++----- time/src/utc_offset.rs | 66 ++++++++++++++++++------------ 6 files changed, 67 insertions(+), 41 deletions(-) diff --git a/tests/parsing.rs b/tests/parsing.rs index a8fe5c40db..99756e5d0e 100644 --- a/tests/parsing.rs +++ b/tests/parsing.rs @@ -494,7 +494,7 @@ fn rfc_3339_err() { )); assert!(matches!( OffsetDateTime::parse("2021-01-01T00:00:00+24:00", &Rfc3339), - Err(error::Parse::TryFromParsed(error::TryFromParsed::ComponentRange(component))) if component.name() == "offset hour" + invalid_component!("offset hour") )); assert!(matches!( OffsetDateTime::parse("2021-01-01T00:00:00+00:60", &Rfc3339), diff --git a/tests/serde/mod.rs b/tests/serde/mod.rs index dc2e1ca75c..3f40c7f77e 100644 --- a/tests/serde/mod.rs +++ b/tests/serde/mod.rs @@ -493,12 +493,12 @@ fn offset_date_time_error() { Token::U8(0), Token::U8(0), Token::U32(0), - Token::I8(24), + Token::I8(26), Token::I8(0), Token::I8(0), Token::TupleEnd, ], - "invalid value: integer `24`, expected a value in the range -23..=23", + "invalid value: integer `26`, expected a value in the range -25..=25", ); // the Deserialize impl does not recognize leap second times as valid assert_de_tokens_error::>( @@ -757,12 +757,12 @@ fn utc_offset_error() { assert_de_tokens_error::>( &[ Token::Tuple { len: 3 }, - Token::I8(24), + Token::I8(26), Token::I8(0), Token::I8(0), Token::TupleEnd, ], - "invalid value: integer `24`, expected a value in the range -23..=23", + "invalid value: integer `26`, expected a value in the range -25..=25", ); } diff --git a/time/src/formatting/formattable.rs b/time/src/formatting/formattable.rs index cad1d482dc..9dc4e6801f 100644 --- a/time/src/formatting/formattable.rs +++ b/time/src/formatting/formattable.rs @@ -217,6 +217,9 @@ impl sealed::Sealed for Rfc3339 { if !(0..10_000).contains(&year) { return Err(error::Format::InvalidComponent("year")); } + if offset.whole_hours().unsigned_abs() > 23 { + return Err(error::Format::InvalidComponent("offset_hour")); + } if offset.seconds_past_minute() != 0 { return Err(error::Format::InvalidComponent("offset_second")); } diff --git a/time/src/parsing/mod.rs b/time/src/parsing/mod.rs index 4a2aa829f1..e65cfd5664 100644 --- a/time/src/parsing/mod.rs +++ b/time/src/parsing/mod.rs @@ -31,6 +31,13 @@ impl<'a, T> ParsedItem<'a, T> { f(self.1)?; Some(self.0) } + + /// Filter the value with the provided function. If the function returns `false`, the value + /// is discarded and `None` is returned. Otherwise, the value is preserved and `Some(self)` is + /// returned. + pub(crate) fn filter(self, f: impl FnOnce(&T) -> bool) -> Option> { + f(&self.1).then_some(self) + } } impl<'a> ParsedItem<'a, ()> { diff --git a/time/src/parsing/parsable.rs b/time/src/parsing/parsable.rs index 5e34325e02..e181ad4d3b 100644 --- a/time/src/parsing/parsable.rs +++ b/time/src/parsing/parsable.rs @@ -557,14 +557,15 @@ impl sealed::Sealed for Rfc3339 { let ParsedItem(input, offset_sign) = sign(input).ok_or(InvalidComponent("offset hour"))?; let input = exactly_n_digits::<2, u8>(input) .and_then(|item| { - item.map(|offset_hour| { - if offset_sign == b'-' { - -(offset_hour as i8) - } else { - offset_hour as _ - } - }) - .consume_value(|value| parsed.set_offset_hour(value)) + item.filter(|&offset_hour| offset_hour <= 23)? + .map(|offset_hour| { + if offset_sign == b'-' { + -(offset_hour as i8) + } else { + offset_hour as _ + } + }) + .consume_value(|value| parsed.set_offset_hour(value)) }) .ok_or(InvalidComponent("offset hour"))?; let input = colon(input).ok_or(InvalidLiteral)?.into_inner(); @@ -635,8 +636,9 @@ impl sealed::Sealed for Rfc3339 { } else { let ParsedItem(input, offset_sign) = sign(input).ok_or(InvalidComponent("offset hour"))?; - let ParsedItem(input, offset_hour) = - exactly_n_digits::<2, u8>(input).ok_or(InvalidComponent("offset hour"))?; + let ParsedItem(input, offset_hour) = exactly_n_digits::<2, u8>(input) + .and_then(|parsed| parsed.filter(|&offset_hour| offset_hour <= 23)) + .ok_or(InvalidComponent("offset hour"))?; let input = colon(input).ok_or(InvalidLiteral)?.into_inner(); let ParsedItem(input, offset_minute) = exactly_n_digits::<2, u8>(input).ok_or(InvalidComponent("offset minute"))?; diff --git a/time/src/utc_offset.rs b/time/src/utc_offset.rs index f4e6eaa2bf..b43ca78f64 100644 --- a/time/src/utc_offset.rs +++ b/time/src/utc_offset.rs @@ -22,15 +22,28 @@ use crate::sys::local_offset_at; use crate::OffsetDateTime; /// The type of the `hours` field of `UtcOffset`. -type Hours = RangedI8<{ -(Hour::per(Day) as i8 - 1) }, { Hour::per(Day) as i8 - 1 }>; +type Hours = RangedI8<-25, 25>; /// The type of the `minutes` field of `UtcOffset`. type Minutes = RangedI8<{ -(Minute::per(Hour) as i8 - 1) }, { Minute::per(Hour) as i8 - 1 }>; /// The type of the `seconds` field of `UtcOffset`. type Seconds = RangedI8<{ -(Second::per(Minute) as i8 - 1) }, { Second::per(Minute) as i8 - 1 }>; +/// The type capable of storing the range of whole seconds that a `UtcOffset` can encompass. +type WholeSeconds = RangedI32< + { + Hours::MIN.get() as i32 * Second::per(Hour) as i32 + + Minutes::MIN.get() as i32 * Second::per(Minute) as i32 + + Seconds::MIN.get() as i32 + }, + { + Hours::MAX.get() as i32 * Second::per(Hour) as i32 + + Minutes::MAX.get() as i32 * Second::per(Minute) as i32 + + Seconds::MAX.get() as i32 + }, +>; /// An offset from UTC. /// -/// This struct can store values up to ±23:59:59. If you need support outside this range, please +/// This struct can store values up to ±25:59:59. If you need support outside this range, please /// file an issue with your use case. // All three components _must_ have the same sign. #[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)] @@ -51,9 +64,7 @@ impl UtcOffset { /// # use time_macros::offset; /// assert_eq!(UtcOffset::UTC, offset!(UTC)); /// ``` - #[allow(clippy::undocumented_unsafe_blocks)] // rust-lang/rust-clippy#11246 - // Safety: All values are in range. - pub const UTC: Self = unsafe { Self::__from_hms_unchecked(0, 0, 0) }; + pub const UTC: Self = Self::from_whole_seconds_ranged(WholeSeconds::new_static::<0>()); // region: constructors /// Create a `UtcOffset` representing an offset of the hours, minutes, and seconds provided, the @@ -62,7 +73,7 @@ impl UtcOffset { /// /// # Safety /// - /// - Hours must be in the range `-23..=23`. + /// - Hours must be in the range `-25..=25`. /// - Minutes must be in the range `-59..=59`. /// - Seconds must be in the range `-59..=59`. /// @@ -169,28 +180,31 @@ impl UtcOffset { /// # Ok::<_, time::Error>(()) /// ``` pub const fn from_whole_seconds(seconds: i32) -> Result { - type WholeSeconds = RangedI32< - { - Hours::MIN.get() as i32 * Second::per(Hour) as i32 - + Minutes::MIN.get() as i32 * Second::per(Minute) as i32 - + Seconds::MIN.get() as i32 - }, - { - Hours::MAX.get() as i32 * Second::per(Hour) as i32 - + Minutes::MAX.get() as i32 * Second::per(Minute) as i32 - + Seconds::MAX.get() as i32 - }, - >; - ensure_ranged!(WholeSeconds: seconds); - - // Safety: The value was checked to be in range. - Ok(unsafe { + Ok(Self::from_whole_seconds_ranged( + ensure_ranged!(WholeSeconds: seconds), + )) + } + + /// Create a `UtcOffset` representing an offset by the number of seconds provided. + // ignore because the function is crate-private + /// ```rust,ignore + /// # use time::UtcOffset; + /// # use deranged::RangedI32; + /// assert_eq!( + /// UtcOffset::from_whole_seconds_ranged(RangedI32::new_static::<3_723>()).as_hms(), + /// (1, 2, 3) + /// ); + /// # Ok::<_, time::Error>(()) + /// ``` + pub(crate) const fn from_whole_seconds_ranged(seconds: WholeSeconds) -> Self { + // Safety: The type of `seconds` guarantees that all values are in range. + unsafe { Self::__from_hms_unchecked( - (seconds / Second::per(Hour) as i32) as _, - ((seconds % Second::per(Hour) as i32) / Minute::per(Hour) as i32) as _, - (seconds % Second::per(Minute) as i32) as _, + (seconds.get() / Second::per(Hour) as i32) as _, + ((seconds.get() % Second::per(Hour) as i32) / Minute::per(Hour) as i32) as _, + (seconds.get() % Second::per(Minute) as i32) as _, ) - }) + } } // endregion constructors