From c624847c842d4795e467e21817ebc3d0bfdd6594 Mon Sep 17 00:00:00 2001 From: Jun Wu Date: Fri, 20 Dec 2019 18:16:10 -0800 Subject: [PATCH] hgtime: limit year in 1900 to 9999 Summary: The Python `datatime` stdlib has limitations: - `datetime.datetime` can only express year 1 to 9999. - `strptime` requires year >= 1900. Limit the `HgTime` to 1900 ..= 9999 range to be consistent. Note: Mercurial has an `i32` range limit, which seems to problematic since `i32::MAX` is Jan 19, 2038. So I kept using `i64`. This addressed some XXX comments about not returning errors. The code change is mostly because added error handling. Reviewed By: sfilipco Differential Revision: D18946333 fbshipit-source-id: 0e4756457b0f13451dc5008ef19d4670a7aaa7fb --- eden/scm/lib/cpython-ext/Cargo.toml | 2 +- eden/scm/lib/hgtime/src/lib.rs | 184 ++++++++++++++++++---------- 2 files changed, 123 insertions(+), 63 deletions(-) diff --git a/eden/scm/lib/cpython-ext/Cargo.toml b/eden/scm/lib/cpython-ext/Cargo.toml index 8c0fb77442..c7badf177b 100644 --- a/eden/scm/lib/cpython-ext/Cargo.toml +++ b/eden/scm/lib/cpython-ext/Cargo.toml @@ -11,7 +11,7 @@ python3 = [] [dependencies] anyhow = "1.0.20" lazy_static = "1" -parking_lot = "0.10" +parking_lot = "0.9" serde = "1" [dependencies.cpython] diff --git a/eden/scm/lib/hgtime/src/lib.rs b/eden/scm/lib/hgtime/src/lib.rs index 91f6c31980..42cf59481d 100644 --- a/eden/scm/lib/hgtime/src/lib.rs +++ b/eden/scm/lib/hgtime/src/lib.rs @@ -11,7 +11,8 @@ use chrono::prelude::*; use chrono::Duration; -use std::ops::{Add, Range, Sub}; +use std::convert::{TryFrom, TryInto}; +use std::ops::{Add, Range, RangeInclusive, Sub}; use std::sync::atomic::{AtomicI32, Ordering}; /// A simple time structure that matches hg's time representation. @@ -19,7 +20,7 @@ use std::sync::atomic::{AtomicI32, Ordering}; /// Internally it's unixtime (in GMT), and offset (GMT -1 = +3600). #[derive(Clone, Copy, Debug, PartialEq)] pub struct HgTime { - pub unixtime: u64, + pub unixtime: i64, pub offset: i32, } @@ -63,12 +64,24 @@ const DEFAULT_FORMATS: [&str; 35] = [ ]; const INVALID_OFFSET: i32 = i32::max_value(); -static DEFAUL_OFFSET: AtomicI32 = AtomicI32::new(INVALID_OFFSET); +static DEFAULT_OFFSET: AtomicI32 = AtomicI32::new(INVALID_OFFSET); impl HgTime { - pub fn now() -> Self { - let now: HgTime = Local::now().into(); - now.use_default_offset() + /// Supported Range. This is to be compatible with Python stdlib. + /// + /// The Python `datetime` library can only express a limited range + /// of dates (0001-01-01 to 9999-12-31). Its strftime requires + /// year >= 1900. + pub const RANGE: RangeInclusive = Self::min_value()..=Self::max_value(); + + /// Return the current time, or `None` if the timestamp is outside + /// [`HgTime::RANGE`]. + pub fn now() -> Option { + Local::now() + .try_into() + .ok() + .map(|t: HgTime| t.use_default_offset()) + .and_then(|t| t.bounded()) } /// Parse a date string. @@ -79,18 +92,19 @@ impl HgTime { /// some additional forms like `2 days ago`. pub fn parse(date: &str) -> Option { match date { - "now" => Some(Self::now()), - "today" => Some(Self::from(Local::today().and_hms(0, 0, 0)).use_default_offset()), - "yesterday" => Some( - Self::from(Local::today().and_hms(0, 0, 0) - Duration::days(1)) - .use_default_offset(), - ), + "now" => Self::now(), + "today" => Self::try_from(Local::today().and_hms(0, 0, 0)) + .ok() + .map(|t| t.use_default_offset()), + "yesterday" => Self::try_from(Local::today().and_hms(0, 0, 0) - Duration::days(1)) + .ok() + .map(|t| t.use_default_offset()), date if date.ends_with(" ago") => { let duration_str = &date[..date.len() - 4]; duration_str .parse::() .ok() - .map(|duration| Self::now() - duration.as_secs()) + .and_then(|duration| Self::now().and_then(|n| n - duration.as_secs())) } _ => Self::parse_absolute(date, default_date_lower), } @@ -105,21 +119,28 @@ impl HgTime { /// - < END pub fn parse_range(date: &str) -> Option> { match date { - "now" => { - let now = Self::now(); - Some(now..now + 1) - } + "now" => Self::now().and_then(|n| (n + 1).map(|m| n..m)), "today" => { let date = Local::today(); - let start = Self::from(date.and_hms(0, 0, 0)).use_default_offset(); - let end = Self::from(date.and_hms(23, 59, 59)).use_default_offset() + 1; - Some(start..end) + let start = Self::try_from(date.and_hms(0, 0, 0)).map(|t| t.use_default_offset()); + let end = + Self::try_from(date.and_hms(23, 59, 59)).map(|t| t.use_default_offset() + 1); + if let (Ok(start), Ok(Some(end))) = (start, end) { + Some(start..end) + } else { + None + } } "yesterday" => { let date = Local::today() - Duration::days(1); - let start = Self::from(date.and_hms(0, 0, 0)).use_default_offset(); - let end = Self::from(date.and_hms(23, 59, 59)).use_default_offset() + 1; - Some(start..end) + let start = Self::try_from(date.and_hms(0, 0, 0)).map(|t| t.use_default_offset()); + let end = + Self::try_from(date.and_hms(23, 59, 59)).map(|t| t.use_default_offset() + 1); + if let (Ok(start), Ok(Some(end))) = (start, end) { + Some(start..end) + } else { + None + } } date if date.starts_with(">") => { Self::parse(&date[1..]).map(|start| start..Self::max_value()) @@ -157,9 +178,10 @@ impl HgTime { let end = Self::parse_absolute(date, default_date_upper::) .or_else(|| Self::parse_absolute(date, default_date_upper::)) .or_else(|| Self::parse_absolute(date, default_date_upper::)) - .or_else(|| Self::parse_absolute(date, default_date_upper::)); + .or_else(|| Self::parse_absolute(date, default_date_upper::)) + .and_then(|end| end + 1); if let (Some(start), Some(end)) = (start, end) { - Some(start..end + 1) + Some(start..end) } else { None } @@ -182,7 +204,7 @@ impl HgTime { if let Ok(unixtime) = parts[0].parse() { if let Ok(offset) = parts[1].parse() { if is_valid_offset(offset) { - return Some(Self { unixtime, offset }); + return Self { unixtime, offset }.bounded(); } } } @@ -238,64 +260,88 @@ impl HgTime { // See https://docs.rs/chrono/0.4.9/chrono/format/strftime/index.html#specifiers let format = format!("{}%#z{}", naive_format, default_format); if let Ok(parsed) = DateTime::parse_from_str(&date_with_defaults, &format) { - return Some(parsed.into()); + if let Ok(parsed) = parsed.try_into() { + return Some(parsed); + } } // Without timezone. let format = format!("{}{}", naive_format, default_format); if let Ok(parsed) = NaiveDateTime::parse_from_str(&date_with_defaults, &format) { - return Some(parsed.into()); + if let Ok(parsed) = parsed.try_into() { + return Some(parsed); + } } } None } - /// Change "offset" to DEFAUL_OFFSET. Useful for tests so they won't be + /// Change "offset" to DEFAULT_OFFSET. Useful for tests so they won't be /// affected by local timezone. fn use_default_offset(mut self) -> Self { - let offset = DEFAUL_OFFSET.load(Ordering::SeqCst); + let offset = DEFAULT_OFFSET.load(Ordering::SeqCst); if is_valid_offset(offset) { self.offset = offset } self } - pub fn min_value() -> Self { + /// See [`HgTime::RANGE`] for details. + pub const fn min_value() -> Self { Self { - unixtime: 0, + unixtime: -2208988800, // 1900-01-01 00:00:00 offset: 0, } } - pub fn max_value() -> Self { + /// See [`HgTime::RANGE`] for details. + pub const fn max_value() -> Self { Self { - unixtime: u64::max_value() >> 2, + unixtime: 253402300799, // 9999-12-31 23:59:59 offset: 0, } } + + /// Return `None` if timestamp is out of [`HgTime::RANGE`]. + pub fn bounded(self) -> Option { + if self < Self::min_value() || self > Self::max_value() { + None + } else { + Some(self) + } + } } impl Add for HgTime { - type Output = Self; + type Output = Option; - fn add(self, seconds: u64) -> Self { - Self { - unixtime: self.unixtime + seconds, - offset: self.offset, - } + fn add(self, seconds: u64) -> Option { + seconds.try_into().ok().and_then(|seconds| { + self.unixtime.checked_add(seconds).and_then(|unixtime| { + Self { + unixtime, + offset: self.offset, + } + .bounded() + }) + }) } } impl Sub for HgTime { - type Output = Self; + type Output = Option; - fn sub(self, seconds: u64) -> Self { - Self { - // XXX: This might silently change negative time to 0. - unixtime: self.unixtime.max(seconds) - seconds, - offset: self.offset, - } + fn sub(self, seconds: u64) -> Option { + seconds.try_into().ok().and_then(|seconds| { + self.unixtime.checked_sub(seconds).and_then(|unixtime| { + Self { + unixtime, + offset: self.offset, + } + .bounded() + }) + }) } } @@ -305,30 +351,40 @@ impl PartialOrd for HgTime { } } -impl From> for HgTime { - fn from(time: DateTime) -> Self { - assert!(time.timestamp() >= 0); - Self { - unixtime: time.timestamp() as u64, - offset: time.offset().fix().utc_minus_local(), +impl TryFrom> for HgTime { + type Error = (); + fn try_from(time: DateTime) -> Result { + if time.timestamp() >= i64::min_value() { + Self { + unixtime: time.timestamp(), + offset: time.offset().fix().utc_minus_local(), + } + .bounded() + .ok_or(()) + } else { + Err(()) } } } -impl From for HgTime { - fn from(time: NaiveDateTime) -> Self { +impl TryFrom for HgTime { + type Error = (); + fn try_from(time: NaiveDateTime) -> Result { let timestamp = time.timestamp(); // Use local offset. (Is there a better way to do this?) - let offset = Self::now().offset; - // XXX: This might silently change negative time to 0. - let unixtime = (timestamp + offset as i64).max(0) as u64; - Self { unixtime, offset } + let offset = HgTime::now().ok_or(())?.offset; + if (timestamp + offset as i64) < 0 { + Err(()) + } else { + let unixtime = timestamp + offset as i64; + HgTime { unixtime, offset }.bounded().ok_or(()) + } } } /// Change default offset (timezone). pub fn set_default_offset(offset: i32) { - DEFAUL_OFFSET.store(offset, Ordering::SeqCst); + DEFAULT_OFFSET.store(offset, Ordering::SeqCst); } fn is_valid_offset(offset: i32) -> bool { @@ -454,6 +510,10 @@ mod tests { assert_eq!(t("2020 GMT"), "1577836800 0"); assert_eq!(t("2020-12"), "1606788000 7200"); assert_eq!(t("2020-13"), "fail"); + assert_eq!(t("1000"), "fail"); // year 1000 < HgTime::min_value() + assert_eq!(t("1"), "fail"); + assert_eq!(t("0"), "fail"); + assert_eq!(t("100000000000000000 1400"), "fail"); assert_eq!(t("Fri, 20 Sep 2019 12:15:13 -0700"), "1569006913 25200"); // date --rfc-2822 assert_eq!(t("Fri, 20 Sep 2019 12:15:13"), "1568988913 7200"); @@ -467,7 +527,7 @@ mod tests { assert_eq!(d("10 minutes ago", Duration::hours(1)), "0"); assert_eq!(d("10 hours ago", Duration::days(1)), "0"); assert_eq!(d("10 h ago", Duration::days(1)), "0"); - assert_eq!(t("9999999 years ago"), "0 7200"); + assert_eq!(t("9999999 years ago"), "fail"); } #[test] @@ -506,7 +566,7 @@ mod tests { fn d(date: &str, duration: Duration) -> String { match HgTime::parse(date) { Some(time) => { - let value = (time.unixtime as i64 - HgTime::now().unixtime as i64).abs() + let value = (time.unixtime as i64 - HgTime::now().unwrap().unixtime as i64).abs() / duration.num_seconds(); format!("{}", value) }