From 5726a07a761962e12a16d16f76b9012c2994ae11 Mon Sep 17 00:00:00 2001 From: Kyle Bloom Date: Fri, 18 Nov 2022 20:24:57 +0000 Subject: [PATCH 1/6] feat: Move from_time to try_from Moves from_time function to TryFrom --- src/result.rs | 17 ++++++++++++++++ src/types.rs | 56 ++++++++++++++++++++++++++++++++++++++++++++++----- src/write.rs | 1 + 3 files changed, 69 insertions(+), 5 deletions(-) diff --git a/src/result.rs b/src/result.rs index 95eb4667..070834a4 100644 --- a/src/result.rs +++ b/src/result.rs @@ -81,3 +81,20 @@ impl From for io::Error { io::Error::new(io::ErrorKind::Other, err) } } + +/// Error type for time parsing +#[derive(Debug)] +pub enum ZipDateTimeError { + /// The date was on or before 1980, or on or after 2107 + DateTimeOutOfBounds, +} + +impl fmt::Display for ZipDateTimeError { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + match self { + ZipDateTimeError::DateTimeOutOfBounds => write!(fmt, "datetime out of bounds"), + } + } +} + +impl Error for ZipDateTimeError {} diff --git a/src/types.rs b/src/types.rs index da4a42e3..609959de 100644 --- a/src/types.rs +++ b/src/types.rs @@ -1,13 +1,15 @@ //! Types that specify what is contained in a ZIP. -#[cfg(doc)] -use {crate::read::ZipFile, crate::write::FileOptions}; - +#[cfg(feature = "time")] +use std::convert::TryFrom; #[cfg(not(any( all(target_arch = "arm", target_pointer_width = "32"), target_arch = "mips", target_arch = "powerpc" )))] use std::sync::atomic; +use std::time::SystemTime; +#[cfg(doc)] +use {crate::read::ZipFile, crate::write::FileOptions}; #[cfg(any( all(target_arch = "arm", target_pointer_width = "32"), @@ -41,6 +43,8 @@ mod atomic { } } +#[cfg(feature = "time")] +use crate::result::ZipDateTimeError; #[cfg(feature = "time")] use time::{error::ComponentRange, Date, Month, OffsetDateTime, PrimitiveDateTime, Time}; @@ -167,6 +171,7 @@ impl DateTime { /// /// Returns `Err` when this object is out of bounds #[allow(clippy::result_unit_err)] + #[deprecated(note = "use `DateTime::try_from()`")] pub fn from_time(dt: OffsetDateTime) -> Result { if dt.year() >= 1980 && dt.year() <= 2107 { Ok(DateTime { @@ -195,8 +200,6 @@ impl DateTime { #[cfg(feature = "time")] /// Converts the DateTime to a OffsetDateTime structure pub fn to_time(&self) -> Result { - use std::convert::TryFrom; - let date = Date::from_calendar_date(self.year as i32, Month::try_from(self.month)?, self.day)?; let time = Time::from_hms(self.hour, self.minute, self.second)?; @@ -254,6 +257,26 @@ impl DateTime { } } +#[cfg(feature = "time")] +impl TryFrom for DateTime { + type Error = ZipDateTimeError; + + fn try_from(dt: OffsetDateTime) -> Result { + if dt.year() >= 1980 && dt.year() <= 2107 { + Ok(DateTime { + year: (dt.year()) as u16, + month: (dt.month()) as u8, + day: dt.day(), + hour: dt.hour(), + minute: dt.minute(), + second: dt.second(), + }) + } else { + Err(ZipDateTimeError::DateTimeOutOfBounds) + } + } +} + pub const DEFAULT_VERSION: u8 = 46; /// A type like `AtomicU64` except it implements `Clone` and has predefined @@ -514,6 +537,27 @@ mod test { assert!(DateTime::from_time(datetime!(2108-01-01 00:00:00 UTC)).is_err()); } + #[cfg(feature = "time")] + #[test] + fn datetime_try_from_bounds() { + use std::convert::TryFrom; + + use super::DateTime; + use time::macros::datetime; + + // 1979-12-31 23:59:59 + assert!(DateTime::try_from(datetime!(1979-12-31 23:59:59 UTC)).is_err()); + + // 1980-01-01 00:00:00 + assert!(DateTime::try_from(datetime!(1980-01-01 00:00:00 UTC)).is_ok()); + + // 2107-12-31 23:59:59 + assert!(DateTime::try_from(datetime!(2107-12-31 23:59:59 UTC)).is_ok()); + + // 2108-01-01 00:00:00 + assert!(DateTime::try_from(datetime!(2108-01-01 00:00:00 UTC)).is_err()); + } + #[test] fn time_conversion() { use super::DateTime; @@ -562,10 +606,12 @@ mod test { #[test] fn time_at_january() { use super::DateTime; + use std::convert::TryFrom; // 2020-01-01 00:00:00 let clock = OffsetDateTime::from_unix_timestamp(1_577_836_800).unwrap(); assert!(DateTime::from_time(clock).is_ok()); + assert!(DateTime::try_from(clock).is_ok()); } } diff --git a/src/write.rs b/src/write.rs index e51fa68f..7953048a 100644 --- a/src/write.rs +++ b/src/write.rs @@ -7,6 +7,7 @@ use crate::spec; use crate::types::{AtomicU64, DateTime, System, ZipFileData, DEFAULT_VERSION}; use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use crc32fast::Hasher; +use std::convert::TryInto; use std::default::Default; use std::io; use std::io::prelude::*; From 3f770178ec0317967edc09bbdc746552f6a20478 Mon Sep 17 00:00:00 2001 From: Kyle Bloom Date: Tue, 31 Jan 2023 13:53:40 +0000 Subject: [PATCH 2/6] fix: Change error type to unit-like struct --- src/result.rs | 16 +++++++--------- src/types.rs | 6 +++--- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/result.rs b/src/result.rs index 070834a4..00d558cb 100644 --- a/src/result.rs +++ b/src/result.rs @@ -84,17 +84,15 @@ impl From for io::Error { /// Error type for time parsing #[derive(Debug)] -pub enum ZipDateTimeError { - /// The date was on or before 1980, or on or after 2107 - DateTimeOutOfBounds, -} +pub struct DateTimeRangeError; -impl fmt::Display for ZipDateTimeError { +impl fmt::Display for DateTimeRangeError { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - match self { - ZipDateTimeError::DateTimeOutOfBounds => write!(fmt, "datetime out of bounds"), - } + write!( + fmt, + "a date could not be represented within the bounds the MS-DOS date range (1980-2107)" + ) } } -impl Error for ZipDateTimeError {} +impl Error for DateTimeRangeError {} diff --git a/src/types.rs b/src/types.rs index 609959de..ac1b9a1d 100644 --- a/src/types.rs +++ b/src/types.rs @@ -44,7 +44,7 @@ mod atomic { } #[cfg(feature = "time")] -use crate::result::ZipDateTimeError; +use crate::result::DateTimeRangeError; #[cfg(feature = "time")] use time::{error::ComponentRange, Date, Month, OffsetDateTime, PrimitiveDateTime, Time}; @@ -259,7 +259,7 @@ impl DateTime { #[cfg(feature = "time")] impl TryFrom for DateTime { - type Error = ZipDateTimeError; + type Error = DateTimeRangeError; fn try_from(dt: OffsetDateTime) -> Result { if dt.year() >= 1980 && dt.year() <= 2107 { @@ -272,7 +272,7 @@ impl TryFrom for DateTime { second: dt.second(), }) } else { - Err(ZipDateTimeError::DateTimeOutOfBounds) + Err(DateTimeRangeError) } } } From c2adaf7ee06eea02ac9c41d2c8335f8ec23608a6 Mon Sep 17 00:00:00 2001 From: Kyle Bloom Date: Tue, 31 Jan 2023 13:54:29 +0000 Subject: [PATCH 3/6] fix: Use try into implementation for from_time --- src/types.rs | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/types.rs b/src/types.rs index ac1b9a1d..b6e79097 100644 --- a/src/types.rs +++ b/src/types.rs @@ -173,18 +173,9 @@ impl DateTime { #[allow(clippy::result_unit_err)] #[deprecated(note = "use `DateTime::try_from()`")] pub fn from_time(dt: OffsetDateTime) -> Result { - if dt.year() >= 1980 && dt.year() <= 2107 { - Ok(DateTime { - year: (dt.year()) as u16, - month: (dt.month()) as u8, - day: dt.day(), - hour: dt.hour(), - minute: dt.minute(), - second: dt.second(), - }) - } else { - Err(()) - } + use std::convert::TryInto; + + dt.try_into().map_err(|_err| ()) } /// Gets the time portion of this datetime in the msdos representation From ab2800b4d8c0ee4a865b4797b75c2cc8bbd5fc82 Mon Sep 17 00:00:00 2001 From: Kyle Bloom Date: Tue, 31 Jan 2023 13:59:45 +0000 Subject: [PATCH 4/6] chore: Move use for TryInto to top --- src/types.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/types.rs b/src/types.rs index b6e79097..72873675 100644 --- a/src/types.rs +++ b/src/types.rs @@ -1,6 +1,6 @@ //! Types that specify what is contained in a ZIP. #[cfg(feature = "time")] -use std::convert::TryFrom; +use std::convert::{TryFrom, TryInto}; #[cfg(not(any( all(target_arch = "arm", target_pointer_width = "32"), target_arch = "mips", @@ -173,8 +173,6 @@ impl DateTime { #[allow(clippy::result_unit_err)] #[deprecated(note = "use `DateTime::try_from()`")] pub fn from_time(dt: OffsetDateTime) -> Result { - use std::convert::TryInto; - dt.try_into().map_err(|_err| ()) } From ccd20c118e68032d4fff7e324b1353968bc93d26 Mon Sep 17 00:00:00 2001 From: Kyle Bloom Date: Tue, 31 Jan 2023 17:58:09 +0000 Subject: [PATCH 5/6] fix: Unused import with time feature --- src/types.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/types.rs b/src/types.rs index 72873675..e07721af 100644 --- a/src/types.rs +++ b/src/types.rs @@ -7,6 +7,7 @@ use std::convert::{TryFrom, TryInto}; target_arch = "powerpc" )))] use std::sync::atomic; +#[cfg(not(feature = "time"))] use std::time::SystemTime; #[cfg(doc)] use {crate::read::ZipFile, crate::write::FileOptions}; From 42eabc9e3316a50ed2c1974f0b04dd0172f74082 Mon Sep 17 00:00:00 2001 From: Marli Frost Date: Wed, 1 Feb 2023 14:01:53 +0000 Subject: [PATCH 6/6] fix: update references to old from_time API --- src/types.rs | 11 ++++++----- src/write.rs | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/types.rs b/src/types.rs index e07721af..ad3a5700 100644 --- a/src/types.rs +++ b/src/types.rs @@ -511,20 +511,22 @@ mod test { #[cfg(feature = "time")] #[test] fn datetime_from_time_bounds() { + use std::convert::TryFrom; + use super::DateTime; use time::macros::datetime; // 1979-12-31 23:59:59 - assert!(DateTime::from_time(datetime!(1979-12-31 23:59:59 UTC)).is_err()); + assert!(DateTime::try_from(datetime!(1979-12-31 23:59:59 UTC)).is_err()); // 1980-01-01 00:00:00 - assert!(DateTime::from_time(datetime!(1980-01-01 00:00:00 UTC)).is_ok()); + assert!(DateTime::try_from(datetime!(1980-01-01 00:00:00 UTC)).is_ok()); // 2107-12-31 23:59:59 - assert!(DateTime::from_time(datetime!(2107-12-31 23:59:59 UTC)).is_ok()); + assert!(DateTime::try_from(datetime!(2107-12-31 23:59:59 UTC)).is_ok()); // 2108-01-01 00:00:00 - assert!(DateTime::from_time(datetime!(2108-01-01 00:00:00 UTC)).is_err()); + assert!(DateTime::try_from(datetime!(2108-01-01 00:00:00 UTC)).is_err()); } #[cfg(feature = "time")] @@ -601,7 +603,6 @@ mod test { // 2020-01-01 00:00:00 let clock = OffsetDateTime::from_unix_timestamp(1_577_836_800).unwrap(); - assert!(DateTime::from_time(clock).is_ok()); assert!(DateTime::try_from(clock).is_ok()); } } diff --git a/src/write.rs b/src/write.rs index 7953048a..14252b4d 100644 --- a/src/write.rs +++ b/src/write.rs @@ -191,7 +191,7 @@ impl Default for FileOptions { compression_method: CompressionMethod::Stored, compression_level: None, #[cfg(feature = "time")] - last_modified_time: DateTime::from_time(OffsetDateTime::now_utc()).unwrap_or_default(), + last_modified_time: OffsetDateTime::now_utc().try_into().unwrap_or_default(), #[cfg(not(feature = "time"))] last_modified_time: DateTime::default(), permissions: None,