diff options
| author | bors <bors@rust-lang.org> | 2018-11-25 19:01:35 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2018-11-25 19:01:35 +0000 |
| commit | 6acbb5b65c06d82c867a94c54ce51dab4707ac61 (patch) | |
| tree | 20692d5500836bc0d65c20e89d6b5c8d1c087629 /src/libstd | |
| parent | 5bd451b26580de465d59ed5389209ed191b7dbdd (diff) | |
| parent | f2106d0746cdbd04ddad44c35b4e13eeced2a546 (diff) | |
| download | rust-6acbb5b65c06d82c867a94c54ce51dab4707ac61.tar.gz rust-6acbb5b65c06d82c867a94c54ce51dab4707ac61.zip | |
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.
Diffstat (limited to 'src/libstd')
| -rw-r--r-- | src/libstd/sys/cloudabi/time.rs | 19 | ||||
| -rw-r--r-- | src/libstd/sys/redox/time.rs | 18 | ||||
| -rw-r--r-- | src/libstd/sys/unix/time.rs | 22 | ||||
| -rw-r--r-- | src/libstd/sys/wasm/time.rs | 4 | ||||
| -rw-r--r-- | src/libstd/sys/windows/time.rs | 16 | ||||
| -rw-r--r-- | src/libstd/time.rs | 21 |
6 files changed, 78 insertions, 22 deletions
diff --git a/src/libstd/sys/cloudabi/time.rs b/src/libstd/sys/cloudabi/time.rs index 3d66998b9f5..a442d1e4ad7 100644 --- a/src/libstd/sys/cloudabi/time.rs +++ b/src/libstd/sys/cloudabi/time.rs @@ -19,10 +19,14 @@ pub struct Instant { t: abi::timestamp, } -pub fn dur2intervals(dur: &Duration) -> abi::timestamp { +fn checked_dur2intervals(dur: &Duration) -> Option<abi::timestamp> { dur.as_secs() .checked_mul(NSEC_PER_SEC) .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") } @@ -92,11 +96,14 @@ impl SystemTime { } pub fn add_duration(&self, other: &Duration) -> SystemTime { - SystemTime { - t: self.t - .checked_add(dur2intervals(other)) - .expect("overflow when adding duration to instant"), - } + self.checked_add_duration(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 { diff --git a/src/libstd/sys/redox/time.rs b/src/libstd/sys/redox/time.rs index aac6d2704e7..beff8d287e7 100644 --- a/src/libstd/sys/redox/time.rs +++ b/src/libstd/sys/redox/time.rs @@ -42,27 +42,29 @@ impl 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 .as_secs() .try_into() // <- target type would be `i64` .ok() - .and_then(|secs| self.t.tv_sec.checked_add(secs)) - .expect("overflow when adding duration to time"); + .and_then(|secs| self.t.tv_sec.checked_add(secs))?; // Nano calculations can't overflow because nanos are <1B which fit // in a u32. let mut nsec = other.subsec_nanos() + self.t.tv_nsec as u32; if nsec >= NSEC_PER_SEC as u32 { nsec -= NSEC_PER_SEC as u32; - secs = secs.checked_add(1).expect("overflow when adding \ - duration to time"); + secs = secs.checked_add(1)?; } - Timespec { + Some(Timespec { t: syscall::TimeSpec { tv_sec: secs, tv_nsec: nsec as i32, }, - } + }) } fn sub_duration(&self, other: &Duration) -> Timespec { @@ -180,6 +182,10 @@ impl SystemTime { 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 { SystemTime { t: self.t.sub_duration(other) } } diff --git a/src/libstd/sys/unix/time.rs b/src/libstd/sys/unix/time.rs index af51f8a8e25..1f9539c36e0 100644 --- a/src/libstd/sys/unix/time.rs +++ b/src/libstd/sys/unix/time.rs @@ -43,27 +43,29 @@ impl 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 .as_secs() .try_into() // <- target type would be `libc::time_t` .ok() - .and_then(|secs| self.t.tv_sec.checked_add(secs)) - .expect("overflow when adding duration to time"); + .and_then(|secs| self.t.tv_sec.checked_add(secs))?; // Nano calculations can't overflow because nanos are <1B which fit // in a u32. let mut nsec = other.subsec_nanos() + self.t.tv_nsec as u32; if nsec >= NSEC_PER_SEC as u32 { nsec -= NSEC_PER_SEC as u32; - secs = secs.checked_add(1).expect("overflow when adding \ - duration to time"); + secs = secs.checked_add(1)?; } - Timespec { + Some(Timespec { t: libc::timespec { tv_sec: secs, tv_nsec: nsec as _, }, - } + }) } fn sub_duration(&self, other: &Duration) -> Timespec { @@ -201,6 +203,10 @@ mod inner { 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 { SystemTime { t: self.t.sub_duration(other) } } @@ -325,6 +331,10 @@ mod inner { 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 { SystemTime { t: self.t.sub_duration(other) } } diff --git a/src/libstd/sys/wasm/time.rs b/src/libstd/sys/wasm/time.rs index e52435e6339..991e8176edf 100644 --- a/src/libstd/sys/wasm/time.rs +++ b/src/libstd/sys/wasm/time.rs @@ -51,6 +51,10 @@ impl SystemTime { 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 { SystemTime(self.0 - *other) } diff --git a/src/libstd/sys/windows/time.rs b/src/libstd/sys/windows/time.rs index 54bcbc76b1a..c809a0b98ac 100644 --- a/src/libstd/sys/windows/time.rs +++ b/src/libstd/sys/windows/time.rs @@ -128,9 +128,13 @@ impl SystemTime { } pub fn add_duration(&self, other: &Duration) -> SystemTime { - let intervals = self.intervals().checked_add(dur2intervals(other)) - .expect("overflow when adding duration to time"); - SystemTime::from_intervals(intervals) + self.checked_add_duration(other).expect("overflow when adding duration to time") + } + + 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 { @@ -180,11 +184,15 @@ impl Hash for SystemTime { } } -fn dur2intervals(d: &Duration) -> i64 { +fn checked_dur2intervals(d: &Duration) -> Option<i64> { d.as_secs() .checked_mul(INTERVALS_PER_SEC) .and_then(|i| i.checked_add(d.subsec_nanos() as u64 / 100)) .and_then(|i| i.try_into().ok()) +} + +fn dur2intervals(d: &Duration) -> i64 { + checked_dur2intervals(d) .expect("overflow when converting duration to intervals") } diff --git a/src/libstd/time.rs b/src/libstd/time.rs index 58be1972a81..5d0d501615f 100644 --- a/src/libstd/time.rs +++ b/src/libstd/time.rs @@ -357,6 +357,14 @@ impl SystemTime { pub fn elapsed(&self) -> Result<Duration, SystemTimeError> { 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")] @@ -561,6 +569,19 @@ mod tests { let one_second_from_epoch2 = UNIX_EPOCH + Duration::new(0, 500_000_000) + Duration::new(0, 500_000_000); 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] |
