Auto merge of #55527 - sgeisler:time-checked-add, r=sfackler

Implement checked_add_duration for SystemTime

[Original discussion on the rust user forum](https://users.rust-lang.org/t/std-systemtime-misses-a-checked-add-function/21785)

Since `SystemTime` is opaque there is no way to check if the result of an addition will be in bounds. That makes the `Add<Duration>` trait completely unusable with untrusted data. This is a big problem because adding a `Duration` to `UNIX_EPOCH` is the standard way of constructing a `SystemTime` from a unix timestamp.

This PR implements `checked_add_duration(&self, &Duration) -> Option<SystemTime>` for `std::time::SystemTime` and as a prerequisite also for all platform specific time structs. This also led to the refactoring of many `add_duration(&self, &Duration) -> SystemTime` functions to avoid redundancy (they now unwrap the result of `checked_add_duration`).

Some basic unit tests for the newly introduced function were added too.

I wasn't sure which stabilization attribute to add to the newly introduced function, so I just chose `#[stable(feature = "time_checked_add", since = "1.32.0")]` for now to make it compile. Please let me know how I should change it or if I violated any other conventions.

P.S.: I could only test on Linux so far, so I don't necessarily expect it to compile for all platforms.
This commit is contained in:
bors 2018-11-25 19:01:35 +00:00
commit 6acbb5b65c
6 changed files with 78 additions and 22 deletions

View File

@ -19,10 +19,14 @@ pub struct Instant {
t: abi::timestamp, t: abi::timestamp,
} }
pub fn dur2intervals(dur: &Duration) -> abi::timestamp { fn checked_dur2intervals(dur: &Duration) -> Option<abi::timestamp> {
dur.as_secs() dur.as_secs()
.checked_mul(NSEC_PER_SEC) .checked_mul(NSEC_PER_SEC)
.and_then(|nanos| nanos.checked_add(dur.subsec_nanos() as abi::timestamp)) .and_then(|nanos| nanos.checked_add(dur.subsec_nanos() as abi::timestamp))
}
pub fn dur2intervals(dur: &Duration) -> abi::timestamp {
checked_dur2intervals(dur)
.expect("overflow converting duration to nanoseconds") .expect("overflow converting duration to nanoseconds")
} }
@ -92,11 +96,14 @@ impl SystemTime {
} }
pub fn add_duration(&self, other: &Duration) -> SystemTime { pub fn add_duration(&self, other: &Duration) -> SystemTime {
SystemTime { self.checked_add_duration(other)
t: self.t .expect("overflow when adding duration to instant")
.checked_add(dur2intervals(other)) }
.expect("overflow when adding duration to instant"),
} pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
checked_dur2intervals(other)
.and_then(|d| self.t.checked_add(d))
.map(|t| SystemTime {t})
} }
pub fn sub_duration(&self, other: &Duration) -> SystemTime { pub fn sub_duration(&self, other: &Duration) -> SystemTime {

View File

@ -42,27 +42,29 @@ impl Timespec {
} }
fn add_duration(&self, other: &Duration) -> Timespec { fn add_duration(&self, other: &Duration) -> Timespec {
self.checked_add_duration(other).expect("overflow when adding duration to time")
}
fn checked_add_duration(&self, other: &Duration) -> Option<Timespec> {
let mut secs = other let mut secs = other
.as_secs() .as_secs()
.try_into() // <- target type would be `i64` .try_into() // <- target type would be `i64`
.ok() .ok()
.and_then(|secs| self.t.tv_sec.checked_add(secs)) .and_then(|secs| self.t.tv_sec.checked_add(secs))?;
.expect("overflow when adding duration to time");
// Nano calculations can't overflow because nanos are <1B which fit // Nano calculations can't overflow because nanos are <1B which fit
// in a u32. // in a u32.
let mut nsec = other.subsec_nanos() + self.t.tv_nsec as u32; let mut nsec = other.subsec_nanos() + self.t.tv_nsec as u32;
if nsec >= NSEC_PER_SEC as u32 { if nsec >= NSEC_PER_SEC as u32 {
nsec -= NSEC_PER_SEC as u32; nsec -= NSEC_PER_SEC as u32;
secs = secs.checked_add(1).expect("overflow when adding \ secs = secs.checked_add(1)?;
duration to time");
} }
Timespec { Some(Timespec {
t: syscall::TimeSpec { t: syscall::TimeSpec {
tv_sec: secs, tv_sec: secs,
tv_nsec: nsec as i32, tv_nsec: nsec as i32,
}, },
} })
} }
fn sub_duration(&self, other: &Duration) -> Timespec { fn sub_duration(&self, other: &Duration) -> Timespec {
@ -180,6 +182,10 @@ impl SystemTime {
SystemTime { t: self.t.add_duration(other) } SystemTime { t: self.t.add_duration(other) }
} }
pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
self.t.checked_add_duration(other).map(|t| SystemTime { t })
}
pub fn sub_duration(&self, other: &Duration) -> SystemTime { pub fn sub_duration(&self, other: &Duration) -> SystemTime {
SystemTime { t: self.t.sub_duration(other) } SystemTime { t: self.t.sub_duration(other) }
} }

View File

@ -43,27 +43,29 @@ impl Timespec {
} }
fn add_duration(&self, other: &Duration) -> Timespec { fn add_duration(&self, other: &Duration) -> Timespec {
self.checked_add_duration(other).expect("overflow when adding duration to time")
}
fn checked_add_duration(&self, other: &Duration) -> Option<Timespec> {
let mut secs = other let mut secs = other
.as_secs() .as_secs()
.try_into() // <- target type would be `libc::time_t` .try_into() // <- target type would be `libc::time_t`
.ok() .ok()
.and_then(|secs| self.t.tv_sec.checked_add(secs)) .and_then(|secs| self.t.tv_sec.checked_add(secs))?;
.expect("overflow when adding duration to time");
// Nano calculations can't overflow because nanos are <1B which fit // Nano calculations can't overflow because nanos are <1B which fit
// in a u32. // in a u32.
let mut nsec = other.subsec_nanos() + self.t.tv_nsec as u32; let mut nsec = other.subsec_nanos() + self.t.tv_nsec as u32;
if nsec >= NSEC_PER_SEC as u32 { if nsec >= NSEC_PER_SEC as u32 {
nsec -= NSEC_PER_SEC as u32; nsec -= NSEC_PER_SEC as u32;
secs = secs.checked_add(1).expect("overflow when adding \ secs = secs.checked_add(1)?;
duration to time");
} }
Timespec { Some(Timespec {
t: libc::timespec { t: libc::timespec {
tv_sec: secs, tv_sec: secs,
tv_nsec: nsec as _, tv_nsec: nsec as _,
}, },
} })
} }
fn sub_duration(&self, other: &Duration) -> Timespec { fn sub_duration(&self, other: &Duration) -> Timespec {
@ -201,6 +203,10 @@ mod inner {
SystemTime { t: self.t.add_duration(other) } SystemTime { t: self.t.add_duration(other) }
} }
pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
self.t.checked_add_duration(other).map(|t| SystemTime { t })
}
pub fn sub_duration(&self, other: &Duration) -> SystemTime { pub fn sub_duration(&self, other: &Duration) -> SystemTime {
SystemTime { t: self.t.sub_duration(other) } SystemTime { t: self.t.sub_duration(other) }
} }
@ -325,6 +331,10 @@ mod inner {
SystemTime { t: self.t.add_duration(other) } SystemTime { t: self.t.add_duration(other) }
} }
pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
self.t.checked_add_duration(other).map(|t| SystemTime { t })
}
pub fn sub_duration(&self, other: &Duration) -> SystemTime { pub fn sub_duration(&self, other: &Duration) -> SystemTime {
SystemTime { t: self.t.sub_duration(other) } SystemTime { t: self.t.sub_duration(other) }
} }

View File

@ -51,6 +51,10 @@ impl SystemTime {
SystemTime(self.0 + *other) SystemTime(self.0 + *other)
} }
pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
self.0.checked_add(*other).map(|d| SystemTime(d))
}
pub fn sub_duration(&self, other: &Duration) -> SystemTime { pub fn sub_duration(&self, other: &Duration) -> SystemTime {
SystemTime(self.0 - *other) SystemTime(self.0 - *other)
} }

View File

@ -128,9 +128,13 @@ impl SystemTime {
} }
pub fn add_duration(&self, other: &Duration) -> SystemTime { pub fn add_duration(&self, other: &Duration) -> SystemTime {
let intervals = self.intervals().checked_add(dur2intervals(other)) self.checked_add_duration(other).expect("overflow when adding duration to time")
.expect("overflow when adding duration to time"); }
SystemTime::from_intervals(intervals)
pub fn checked_add_duration(&self, other: &Duration) -> Option<SystemTime> {
checked_dur2intervals(other)
.and_then(|d| self.intervals().checked_add(d))
.map(|i| SystemTime::from_intervals(i))
} }
pub fn sub_duration(&self, other: &Duration) -> SystemTime { pub fn sub_duration(&self, other: &Duration) -> SystemTime {
@ -180,11 +184,15 @@ impl Hash for SystemTime {
} }
} }
fn dur2intervals(d: &Duration) -> i64 { fn checked_dur2intervals(d: &Duration) -> Option<i64> {
d.as_secs() d.as_secs()
.checked_mul(INTERVALS_PER_SEC) .checked_mul(INTERVALS_PER_SEC)
.and_then(|i| i.checked_add(d.subsec_nanos() as u64 / 100)) .and_then(|i| i.checked_add(d.subsec_nanos() as u64 / 100))
.and_then(|i| i.try_into().ok()) .and_then(|i| i.try_into().ok())
}
fn dur2intervals(d: &Duration) -> i64 {
checked_dur2intervals(d)
.expect("overflow when converting duration to intervals") .expect("overflow when converting duration to intervals")
} }

View File

@ -357,6 +357,14 @@ impl SystemTime {
pub fn elapsed(&self) -> Result<Duration, SystemTimeError> { pub fn elapsed(&self) -> Result<Duration, SystemTimeError> {
SystemTime::now().duration_since(*self) SystemTime::now().duration_since(*self)
} }
/// Returns `Some(t)` where `t` is the time `self + duration` if `t` can be represented as
/// `SystemTime` (which means it's inside the bounds of the underlying data structure), `None`
/// otherwise.
#[unstable(feature = "time_checked_add", issue = "55940")]
pub fn checked_add(&self, duration: Duration) -> Option<SystemTime> {
self.0.checked_add_duration(&duration).map(|t| SystemTime(t))
}
} }
#[stable(feature = "time2", since = "1.8.0")] #[stable(feature = "time2", since = "1.8.0")]
@ -561,6 +569,19 @@ mod tests {
let one_second_from_epoch2 = UNIX_EPOCH + Duration::new(0, 500_000_000) let one_second_from_epoch2 = UNIX_EPOCH + Duration::new(0, 500_000_000)
+ Duration::new(0, 500_000_000); + Duration::new(0, 500_000_000);
assert_eq!(one_second_from_epoch, one_second_from_epoch2); assert_eq!(one_second_from_epoch, one_second_from_epoch2);
// checked_add_duration will not panic on overflow
let mut maybe_t = Some(SystemTime::UNIX_EPOCH);
let max_duration = Duration::from_secs(u64::max_value());
// in case `SystemTime` can store `>= UNIX_EPOCH + max_duration`.
for _ in 0..2 {
maybe_t = maybe_t.and_then(|t| t.checked_add(max_duration));
}
assert_eq!(maybe_t, None);
// checked_add_duration calculates the right time and will work for another year
let year = Duration::from_secs(60 * 60 * 24 * 365);
assert_eq!(a + year, a.checked_add(year).unwrap());
} }
#[test] #[test]