Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

protect overflow when computing expiration #56

Merged
merged 3 commits into from
Dec 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down
6 changes: 4 additions & 2 deletions src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 2 additions & 6 deletions src/common/time_atomic64.rs → src/common/atomic_time.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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<Option<Instant>>,
Expand Down
30 changes: 30 additions & 0 deletions src/common/time.rs
Original file line number Diff line number Diff line change
@@ -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<Self> 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<Instant> {
self.0.checked_add(duration).map(Instant)
}
}
32 changes: 32 additions & 0 deletions src/future/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -68,6 +70,17 @@ where
/// Builds a `Cache<K, V>`.
pub fn build(self) -> Cache<K, V, RandomState> {
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,
Expand Down Expand Up @@ -144,6 +157,7 @@ impl<C> CacheBuilder<C> {
mod tests {
use super::CacheBuilder;

use super::Cache;
use std::time::Duration;

#[tokio::test]
Expand Down Expand Up @@ -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<Cache<char, String>> = 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<Cache<char, String>> = CacheBuilder::new(100);
let duration = Duration::from_secs(thousand_years_secs);
builder.time_to_idle(duration + Duration::from_secs(1)).build();
}
}
3 changes: 2 additions & 1 deletion src/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

use crate::common::{
deque::DeqNode,
time::{AtomicInstant, Instant},
atomic_time::AtomicInstant,
time::Instant,
AccessTime,
};

Expand Down
17 changes: 14 additions & 3 deletions src/sync/base_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()
}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
32 changes: 32 additions & 0 deletions src/sync/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down Expand Up @@ -92,6 +94,17 @@ where
/// calling this method.
pub fn build(self) -> Cache<K, V, RandomState> {
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,
Expand Down Expand Up @@ -212,6 +225,7 @@ impl<C> CacheBuilder<C> {
#[cfg(test)]
mod tests {
use super::CacheBuilder;
use super::Cache;

use std::time::Duration;

Expand Down Expand Up @@ -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<Cache<char, String>> = 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<Cache<char, String>> = CacheBuilder::new(100);
let duration = Duration::from_secs(thousand_years_secs);
builder.time_to_idle(duration + Duration::from_secs(1)).build();
}
}
32 changes: 32 additions & 0 deletions src/unsync/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -63,6 +65,17 @@ where
/// Builds a `Cache<K, V>`.
pub fn build(self) -> Cache<K, V, RandomState> {
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,
Expand Down Expand Up @@ -122,6 +135,7 @@ impl<C> CacheBuilder<C> {
#[cfg(test)]
mod tests {
use super::CacheBuilder;
use super::Cache;

use std::time::Duration;

Expand Down Expand Up @@ -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<Cache<char, String>> = 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<Cache<char, String>> = CacheBuilder::new(100);
let duration = Duration::from_secs(thousand_years_secs);
builder.time_to_idle(duration + Duration::from_secs(1)).build();
}
}
16 changes: 12 additions & 4 deletions src/unsync/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand Down Expand Up @@ -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()
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down