diff --git a/README.md b/README.md index 13b8e08b..12a0ba3c 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,11 @@ exceeded. - Time to live - Time to idle +## A note on expiration policies +The cache builders will panic if configured with either time to live/ time to idle +higher than 1000 years. This is done to protect against overflow when computing key +expiration. + ## Usage @@ -313,6 +318,7 @@ available on crates.io, such as the [aHash][ahash-crate] crate. [ahash-crate]: https://crates.io/crates/ahash + ## Minimum Supported Rust Versions This crate's minimum supported Rust versions (MSRV) are the followings: diff --git a/src/common.rs b/src/common.rs index 4051ca9c..45a42b15 100644 --- a/src/common.rs +++ b/src/common.rs @@ -8,8 +8,10 @@ pub(crate) mod unsafe_weak_pointer; // https://github.com/rust-lang/rust/issues/32976 // #[cfg_attr(target_has_atomic = "64", path = "common/time_atomic64.rs")] -#[cfg_attr(feature = "atomic64", path = "common/time_atomic64.rs")] -#[cfg_attr(not(feature = "atomic64"), path = "common/time_compat.rs")] +#[cfg_attr(feature = "atomic64", path = "common/atomic_time.rs")] +#[cfg_attr(not(feature = "atomic64"), path = "common/atomic_time_compat.rs")] +pub(crate) mod atomic_time; + pub(crate) mod time; use time::Instant; diff --git a/src/common/time_atomic64.rs b/src/common/atomic_time.rs similarity index 79% rename from src/common/time_atomic64.rs rename to src/common/atomic_time.rs index 50c8686d..d65602ae 100644 --- a/src/common/time_atomic64.rs +++ b/src/common/atomic_time.rs @@ -1,10 +1,6 @@ use std::sync::atomic::{AtomicU64, Ordering}; -pub(crate) type Instant = quanta::Instant; -pub(crate) type Clock = quanta::Clock; - -#[cfg(test)] -pub(crate) type Mock = quanta::Mock; +use super::time::Instant; pub(crate) struct AtomicInstant { instant: AtomicU64, @@ -37,6 +33,6 @@ impl AtomicInstant { } pub(crate) fn set_instant(&self, instant: Instant) { - self.instant.store(instant.as_u64(), Ordering::Release); + self.instant.store(instant.0.as_u64(), Ordering::Release); } } diff --git a/src/common/time_compat.rs b/src/common/atomic_time_compat.rs similarity index 82% rename from src/common/time_compat.rs rename to src/common/atomic_time_compat.rs index 5de71bac..1cfeca38 100644 --- a/src/common/time_compat.rs +++ b/src/common/atomic_time_compat.rs @@ -1,10 +1,5 @@ use parking_lot::RwLock; - -pub(crate) type Instant = quanta::Instant; -pub(crate) type Clock = quanta::Clock; - -#[cfg(test)] -pub(crate) type Mock = quanta::Mock; +use super::time::Instant; pub(crate) struct AtomicInstant { instant: RwLock>, diff --git a/src/common/time.rs b/src/common/time.rs new file mode 100644 index 00000000..390ffffd --- /dev/null +++ b/src/common/time.rs @@ -0,0 +1,30 @@ +use std::time::Duration; + +pub(crate) type Clock = quanta::Clock; +#[cfg(test)] +pub(crate) type Mock = quanta::Mock; + +/// a wrapper type over qunta::Instant to force checked additions and prevent +/// unintentioal overflow. The type preserve the Copy semnatics for the wrapped +#[derive(PartialEq, PartialOrd, Clone, Copy)] +pub(crate) struct Instant(pub quanta::Instant); + +pub(crate) trait CheckedTimeOps { + fn checked_add(&self, duration: Duration)-> Option where Self: Sized; +} + +impl Instant { + pub(crate) fn new(instant: quanta::Instant) -> Instant { + Instant(instant) + } + + pub(crate) fn now()-> Instant { + Instant(quanta::Instant::now()) + } +} + +impl CheckedTimeOps for Instant { + fn checked_add(&self, duration: Duration) -> Option { + self.0.checked_add(duration).map(Instant) + } +} \ No newline at end of file diff --git a/src/future/builder.rs b/src/future/builder.rs index 5cbf2e68..c3bcd88c 100644 --- a/src/future/builder.rs +++ b/src/future/builder.rs @@ -7,6 +7,8 @@ use std::{ time::Duration, }; +const YEAR_SECONDS: u64 = 365 * 24 * 3600; + /// Builds a [`Cache`][cache-struct] with various configuration knobs. /// /// [cache-struct]: ./struct.Cache.html @@ -68,6 +70,17 @@ where /// Builds a `Cache`. pub fn build(self) -> Cache { let build_hasher = RandomState::default(); + self.time_to_live.map(|d| if Duration::from_secs(1_000 * YEAR_SECONDS) < d { + panic!("time_to_live is longer than 1000 years"); + } else { + d + } + ); + self.time_to_idle.map(|d| if Duration::from_secs(1_000 * YEAR_SECONDS) < d { + panic!("time_to_idle is longer than 1000 years"); + } else { + d + }); Cache::with_everything( self.max_capacity, self.initial_capacity, @@ -144,6 +157,7 @@ impl CacheBuilder { mod tests { use super::CacheBuilder; + use super::Cache; use std::time::Duration; #[tokio::test] @@ -172,4 +186,22 @@ mod tests { cache.insert('a', "Alice").await; assert_eq!(cache.get(&'a'), Some("Alice")); } + + #[tokio::test] + #[should_panic(expected = "time_to_live is longer than 1000 years")] + async fn build_cache_too_long_ttl() { + let thousand_years_secs: u64 = 1000 * 365 * 24 * 3600; + let builder: CacheBuilder> = CacheBuilder::new(100); + let duration = Duration::from_secs(thousand_years_secs); + builder.time_to_live(duration + Duration::from_secs(1)).build(); + } + + #[tokio::test] + #[should_panic(expected = "time_to_idle is longer than 1000 years")] + async fn build_cache_too_long_tti() { + let thousand_years_secs: u64 = 1000 * 365 * 24 * 3600; + let builder: CacheBuilder> = CacheBuilder::new(100); + let duration = Duration::from_secs(thousand_years_secs); + builder.time_to_idle(duration + Duration::from_secs(1)).build(); + } } diff --git a/src/sync.rs b/src/sync.rs index 03999861..c3e761f8 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -2,7 +2,8 @@ use crate::common::{ deque::DeqNode, - time::{AtomicInstant, Instant}, + atomic_time::AtomicInstant, + time::Instant, AccessTime, }; diff --git a/src/sync/base_cache.rs b/src/sync/base_cache.rs index 88f88b0a..a3aa2e49 100644 --- a/src/sync/base_cache.rs +++ b/src/sync/base_cache.rs @@ -8,7 +8,8 @@ use crate::{ common::{ deque::{CacheRegion, DeqNode, Deque}, frequency_sketch::FrequencySketch, - time::{AtomicInstant, Clock, Instant}, + atomic_time::AtomicInstant, + time::{Clock, Instant, CheckedTimeOps}, AccessTime, }, PredicateError, @@ -518,11 +519,13 @@ where #[inline] fn current_time_from_expiration_clock(&self) -> Instant { if self.has_expiration_clock.load(Ordering::Relaxed) { + Instant::new( self.expiration_clock .read() .as_ref() .expect("Cannot get the expiration clock") .now() + ) } else { Instant::now() } @@ -1046,7 +1049,11 @@ fn is_expired_entry_ao( } } if let Some(tti) = time_to_idle { - return ts + *tti <= now; + let checked_add = ts.checked_add(*tti); + if checked_add.is_none() { + panic!("ttl overflow") + } + return checked_add.unwrap() <= now; } } false @@ -1066,7 +1073,11 @@ fn is_expired_entry_wo( } } if let Some(ttl) = time_to_live { - return ts + *ttl <= now; + let checked_add = ts.checked_add(*ttl); + if checked_add.is_none() { + panic!("ttl overflow"); + } + return checked_add.unwrap() <= now; } } false diff --git a/src/sync/builder.rs b/src/sync/builder.rs index 18082202..4897676a 100644 --- a/src/sync/builder.rs +++ b/src/sync/builder.rs @@ -7,6 +7,8 @@ use std::{ time::Duration, }; +const YEAR_SECONDS: u64 = 365 * 24 *3600; + /// Builds a [`Cache`][cache-struct] or [`SegmentedCache`][seg-cache-struct] /// with various configuration knobs. /// @@ -92,6 +94,17 @@ where /// calling this method. pub fn build(self) -> Cache { let build_hasher = RandomState::default(); + self.time_to_live.map(|d| if Duration::from_secs(1_000 * YEAR_SECONDS) < d { + panic!("time_to_live is longer than 1000 years"); + } else { + d + } + ); + self.time_to_idle.map(|d| if Duration::from_secs(1_000 * YEAR_SECONDS) < d { + panic!("time_to_idle is longer than 1000 years"); + } else { + d + }); Cache::with_everything( self.max_capacity, self.initial_capacity, @@ -212,6 +225,7 @@ impl CacheBuilder { #[cfg(test)] mod tests { use super::CacheBuilder; + use super::Cache; use std::time::Duration; @@ -269,4 +283,22 @@ mod tests { cache.insert('b', "Bob"); assert_eq!(cache.get(&'b'), Some("Bob")); } + + #[tokio::test] + #[should_panic(expected = "time_to_live is longer than 1000 years")] + async fn build_cache_too_long_ttl() { + let thousand_years_secs: u64 = 1000 * 365 * 24 * 3600; + let builder: CacheBuilder> = CacheBuilder::new(100); + let duration = Duration::from_secs(thousand_years_secs); + builder.time_to_live(duration + Duration::from_secs(1)).build(); + } + + #[tokio::test] + #[should_panic(expected = "time_to_idle is longer than 1000 years")] + async fn build_cache_too_long_tti() { + let thousand_years_secs: u64 = 1000 * 365 * 24 * 3600; + let builder: CacheBuilder> = CacheBuilder::new(100); + let duration = Duration::from_secs(thousand_years_secs); + builder.time_to_idle(duration + Duration::from_secs(1)).build(); + } } diff --git a/src/unsync/builder.rs b/src/unsync/builder.rs index ef38ef0d..9a5d128b 100644 --- a/src/unsync/builder.rs +++ b/src/unsync/builder.rs @@ -7,6 +7,8 @@ use std::{ time::Duration, }; +const YEAR_SECONDS: u64 = 365 * 24 * 3600; + /// Builds a [`Cache`][cache-struct] with various configuration knobs. /// /// [cache-struct]: ./struct.Cache.html @@ -63,6 +65,17 @@ where /// Builds a `Cache`. pub fn build(self) -> Cache { let build_hasher = RandomState::default(); + self.time_to_live.map(|d| if Duration::from_secs(1_000 * YEAR_SECONDS) < d { + panic!("time_to_live is longer than 1000 years"); + } else { + d + } + ); + self.time_to_idle.map(|d| if Duration::from_secs(1_000 * YEAR_SECONDS) < d { + panic!("time_to_idle is longer than 1000 years"); + } else { + d + }); Cache::with_everything( self.max_capacity, self.initial_capacity, @@ -122,6 +135,7 @@ impl CacheBuilder { #[cfg(test)] mod tests { use super::CacheBuilder; + use super::Cache; use std::time::Duration; @@ -149,4 +163,22 @@ mod tests { cache.insert('a', "Alice"); assert_eq!(cache.get(&'a'), Some(&"Alice")); } + + #[tokio::test] + #[should_panic(expected = "time_to_live is longer than 1000 years")] + async fn build_cache_too_long_ttl() { + let thousand_years_secs: u64 = 1000 * 365 * 24 * 3600; + let builder: CacheBuilder> = CacheBuilder::new(100); + let duration = Duration::from_secs(thousand_years_secs); + builder.time_to_live(duration + Duration::from_secs(1)).build(); + } + + #[tokio::test] + #[should_panic(expected = "time_to_idle is longer than 1000 years")] + async fn build_cache_too_long_tti() { + let thousand_years_secs: u64 = 1000 * 365 * 24 * 3600; + let builder: CacheBuilder> = CacheBuilder::new(100); + let duration = Duration::from_secs(thousand_years_secs); + builder.time_to_idle(duration + Duration::from_secs(1)).build(); + } } diff --git a/src/unsync/cache.rs b/src/unsync/cache.rs index 207106d4..52bfe7bf 100644 --- a/src/unsync/cache.rs +++ b/src/unsync/cache.rs @@ -2,7 +2,7 @@ use super::{deques::Deques, KeyDate, KeyHashDate, ValueEntry}; use crate::common::{ deque::{CacheRegion, DeqNode, Deque}, frequency_sketch::FrequencySketch, - time::{Clock, Instant}, + time::{Clock, Instant, CheckedTimeOps}, AccessTime, }; @@ -342,7 +342,7 @@ where #[inline] fn current_time_from_expiration_clock(&self) -> Instant { if let Some(clock) = &self.expiration_clock { - clock.now() + Instant::new(clock.now()) } else { Instant::now() } @@ -355,7 +355,11 @@ where now: Instant, ) -> bool { if let (Some(ts), Some(tti)) = (entry.last_accessed(), time_to_idle) { - return ts + *tti <= now; + let checked_add = ts.checked_add(*tti); + if checked_add.is_none() { + panic!("ttl overflow") + } + return checked_add.unwrap() <= now; } false } @@ -367,7 +371,11 @@ where now: Instant, ) -> bool { if let (Some(ts), Some(ttl)) = (entry.last_modified(), time_to_live) { - return ts + *ttl <= now; + let checked_add = ts.checked_add(*ttl); + if checked_add.is_none() { + panic!("ttl overflow") + } + return checked_add.unwrap() <= now; } false }