diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 2ca0f327..29e132fe 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -30,6 +30,8 @@ jobs: USER: "test-user" - name: Upload to codecov.io - uses: codecov/codecov-action@v3 + uses: codecov/codecov-action@v4 with: fail_ci_if_error: true + env: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} diff --git a/Cargo.toml b/Cargo.toml index fbc77abd..85932203 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -58,7 +58,7 @@ harness = false [dependencies] arc-swap = "1.6" -chrono = { version = "0.4.23", optional = true, features = ["clock"], default-features = false } +chrono = { version = "0.4.35", optional = true, features = ["clock"], default-features = false } flate2 = { version = "1.0", optional = true } fnv = "1.0" humantime = { version = "2.1", optional = true } diff --git a/src/append/rolling_file/policy/compound/trigger/time.rs b/src/append/rolling_file/policy/compound/trigger/time.rs index 4568a524..4d2f931a 100644 --- a/src/append/rolling_file/policy/compound/trigger/time.rs +++ b/src/append/rolling_file/policy/compound/trigger/time.rs @@ -2,8 +2,6 @@ //! //! Requires the `time_trigger` feature. -#[cfg(test)] -use chrono::NaiveDateTime; use chrono::{DateTime, Datelike, Duration, Local, TimeZone, Timelike}; #[cfg(test)] use mock_instant::{SystemTime, UNIX_EPOCH}; @@ -13,6 +11,7 @@ use serde::de; #[cfg(feature = "config_parsing")] use std::fmt; use std::sync::RwLock; +use thiserror::Error; use crate::append::rolling_file::{policy::compound::trigger::Trigger, LogFile}; #[cfg(feature = "config_parsing")] @@ -73,6 +72,12 @@ impl Default for TimeTriggerInterval { } } +#[derive(Debug, Error)] +enum TimeTriggerIntervalError { + #[error("The '{0}' value specified as a time trigger is out of bounds, ensure it fits within an i64: : '{1:?}'")] + OutOfBounds(String, TimeTriggerInterval), +} + #[cfg(feature = "config_parsing")] impl<'de> serde::Deserialize<'de> for TimeTriggerInterval { fn deserialize(d: D) -> Result @@ -175,45 +180,54 @@ impl<'de> serde::Deserialize<'de> for TimeTriggerInterval { impl TimeTrigger { /// Returns a new trigger which rolls the log once it has passed the /// specified time. - pub fn new(config: TimeTriggerConfig) -> TimeTrigger { + pub fn new(config: TimeTriggerConfig) -> anyhow::Result { #[cfg(test)] let current = { let now: std::time::Duration = SystemTime::now() .duration_since(UNIX_EPOCH) .expect("system time before Unix epoch"); - NaiveDateTime::from_timestamp_opt(now.as_secs() as i64, now.subsec_nanos()) + DateTime::from_timestamp(now.as_secs() as i64, now.subsec_nanos()) .unwrap() + .naive_local() .and_local_timezone(Local) .unwrap() }; #[cfg(not(test))] let current = Local::now(); - let next_time = TimeTrigger::get_next_time(current, config.interval, config.modulate); + // TODO: Design an implement error handling at a project level scope instead of panic + let next_time = TimeTrigger::get_next_time(current, config.interval, config.modulate)?; let next_roll_time = if config.max_random_delay > 0 { let random_delay = rand::thread_rng().gen_range(0..config.max_random_delay); - next_time + Duration::seconds(random_delay as i64) + // This is a valid unwrap because chrono::Duration::try_milliseconds accepts an i64 + // and we're providing a known valid value. We can trust the Option will always return + // us a valid number. It's acceptable to use i64::MAX here because it is replacement of + // a randomly generated value. If a user enters a value greater than i64::MAX, then + // i64::MAX is an acceptable value. + next_time + + Duration::try_seconds(random_delay as i64) + .unwrap_or(Duration::try_milliseconds(i64::MAX).unwrap()) } else { next_time }; - TimeTrigger { + Ok(TimeTrigger { config, next_roll_time: RwLock::new(next_roll_time), - } + }) } fn get_next_time( current: DateTime, interval: TimeTriggerInterval, modulate: bool, - ) -> DateTime { + ) -> Result, TimeTriggerIntervalError> { let year = current.year(); if let TimeTriggerInterval::Year(n) = interval { let n = n as i32; let increment = if modulate { n - year % n } else { n }; let year_new = year + increment; - return Local.with_ymd_and_hms(year_new, 1, 1, 0, 0, 0).unwrap(); + return Ok(Local.with_ymd_and_hms(year_new, 1, 1, 0, 0, 0).unwrap()); } if let TimeTriggerInterval::Month(n) = interval { @@ -224,26 +238,35 @@ impl TimeTrigger { let num_months_new = num_months + increment; let year_new = (num_months_new / 12) as i32; let month_new = (num_months_new) % 12 + 1; - return Local + return Ok(Local .with_ymd_and_hms(year_new, month_new, 1, 0, 0, 0) - .unwrap(); + .unwrap()); } let month = current.month(); let day = current.day(); if let TimeTriggerInterval::Week(n) = interval { let week0 = current.iso_week().week0() as i64; - let weekday = current.weekday().num_days_from_monday() as i64; // Monday is the first day of the week let time = Local.with_ymd_and_hms(year, month, day, 0, 0, 0).unwrap(); let increment = if modulate { n - week0 % n } else { n }; - return time + Duration::weeks(increment) - Duration::days(weekday); + + // Unwrap instead of propogate the try_days call as this is a known safe value + // generated by the chrono library as a value between 0-6. + let weekday = current.weekday().num_days_from_monday() as i64; // Monday is the first day of the week + let dur = Duration::try_weeks(increment).ok_or( + TimeTriggerIntervalError::OutOfBounds("Weeks".to_owned(), interval), + )? - Duration::try_days(weekday).unwrap(); + return Ok(time + dur); } if let TimeTriggerInterval::Day(n) = interval { let ordinal0 = current.ordinal0() as i64; let time = Local.with_ymd_and_hms(year, month, day, 0, 0, 0).unwrap(); let increment = if modulate { n - ordinal0 % n } else { n }; - return time + Duration::days(increment); + let dur = Duration::try_days(increment).ok_or( + TimeTriggerIntervalError::OutOfBounds("Days".to_owned(), interval), + )?; + return Ok(time + dur); } let hour = current.hour(); @@ -252,7 +275,10 @@ impl TimeTrigger { .with_ymd_and_hms(year, month, day, hour, 0, 0) .unwrap(); let increment = if modulate { n - (hour as i64) % n } else { n }; - return time + Duration::hours(increment); + let dur = Duration::try_hours(increment).ok_or( + TimeTriggerIntervalError::OutOfBounds("Hours".to_owned(), interval), + )?; + return Ok(time + dur); } let min = current.minute(); @@ -261,7 +287,10 @@ impl TimeTrigger { .with_ymd_and_hms(year, month, day, hour, min, 0) .unwrap(); let increment = if modulate { n - (min as i64) % n } else { n }; - return time + Duration::minutes(increment); + let dur = Duration::try_minutes(increment).ok_or( + TimeTriggerIntervalError::OutOfBounds("Minutes".to_owned(), interval), + )?; + return Ok(time + dur); } let sec = current.second(); @@ -270,7 +299,10 @@ impl TimeTrigger { .with_ymd_and_hms(year, month, day, hour, min, sec) .unwrap(); let increment = if modulate { n - (sec as i64) % n } else { n }; - return time + Duration::seconds(increment); + let dur = Duration::try_seconds(increment).ok_or( + TimeTriggerIntervalError::OutOfBounds("Seconds".to_owned(), interval), + )?; + return Ok(time + dur); } panic!("Should not reach here!"); } @@ -283,8 +315,9 @@ impl Trigger for TimeTrigger { let now = SystemTime::now() .duration_since(UNIX_EPOCH) .expect("system time before Unix epoch"); - NaiveDateTime::from_timestamp_opt(now.as_secs() as i64, now.subsec_nanos()) + DateTime::from_timestamp(now.as_secs() as i64, now.subsec_nanos()) .unwrap() + .naive_local() .and_local_timezone(Local) .unwrap() }; @@ -294,7 +327,7 @@ impl Trigger for TimeTrigger { let mut next_roll_time = self.next_roll_time.write().unwrap(); let is_trigger = current >= *next_roll_time; if is_trigger { - let tmp = TimeTrigger::new(self.config); + let tmp = TimeTrigger::new(self.config)?; let time_new = tmp.next_roll_time.read().unwrap(); *next_roll_time = *time_new; } @@ -333,7 +366,7 @@ impl Deserialize for TimeTriggerDeserializer { config: TimeTriggerConfig, _: &Deserializers, ) -> anyhow::Result> { - Ok(Box::new(TimeTrigger::new(config))) + Ok(Box::new(TimeTrigger::new(config)?)) } } @@ -361,7 +394,7 @@ mod test { max_random_delay: 0, }; - let trigger = TimeTrigger::new(config); + let trigger = TimeTrigger::new(config).unwrap(); MockClock::advance_system_time(Duration::from_millis(millis / 2)); let result1 = trigger.trigger(&logfile).unwrap(); @@ -491,6 +524,18 @@ mod test { max_random_delay: 0, }; let trigger = TimeTrigger::new(config); - assert!(trigger.is_pre_process()); + assert!(trigger.unwrap().is_pre_process()); + } + + #[test] + fn test_days_overflow() { + let config = TimeTriggerConfig { + interval: TimeTriggerInterval::Day(i64::MAX), + modulate: false, + max_random_delay: 0, + }; + + let trigger = TimeTrigger::new(config); + assert!(trigger.is_err()); } }