Skip to content

Commit 91fd3f1

Browse files
soxjkeandersio
authored andcommitted
Fixed integer overflows for Foundation extensions (#543)
* - Fixed integer overflows for Foundation extensions (#506) - Added overflow test case - Removed note on value limits for DispatchTimeInterval `*` operator - Updated CHANGELOG.md * - Fixed linux build https://travis-ci.org/ReactiveCocoa/ReactiveSwift/jobs/291102863 - Update CHANGELOG.md * Bringing back `==` syntax for equality matchers in FoundationExtensionsSpec.swift
1 parent 38aeae5 commit 91fd3f1

File tree

3 files changed

+21
-17
lines changed

3 files changed

+21
-17
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# master
22
*Please add new entries at the top.*
3+
1. Fixed integer overflow for `DispatchTimeInterval` in FoundationExtensions.swift (#506)
34

45
# 3.0.0-alpha.1
56
# 3.0.0-alpha.1

Sources/FoundationExtensions.swift

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ extension DispatchTimeInterval {
9393
case let .milliseconds(ms):
9494
return TimeInterval(TimeInterval(ms) / 1000.0)
9595
case let .microseconds(us):
96-
return TimeInterval(Int64(us) * Int64(NSEC_PER_USEC)) / TimeInterval(NSEC_PER_SEC)
96+
return TimeInterval(Int64(us)) * TimeInterval(NSEC_PER_USEC) / TimeInterval(NSEC_PER_SEC)
9797
case let .nanoseconds(ns):
9898
return TimeInterval(ns) / TimeInterval(NSEC_PER_SEC)
9999
case .never:
@@ -120,23 +120,19 @@ extension DispatchTimeInterval {
120120

121121
/// Scales a time interval by the given scalar specified in `rhs`.
122122
///
123-
/// - note: This method is only used internally to "scale down" a time
124-
/// interval. Specifically it's used only to scale intervals to 10%
125-
/// of their original value for the default `leeway` parameter in
126-
/// `Scheduler.schedule(after:action:)` schedule and similar
127-
/// other methods.
128-
///
129-
/// If seconds is over 200,000, 10% is ~2,000, and hence we end up
130-
/// with a value of ~2,000,000,000. Not quite overflowing a signed
131-
/// integer on 32-bit platforms, but close.
132-
///
133-
/// Even still, 200,000 seconds should be a rarely (if ever)
134-
/// specified interval for our APIs. And even then, folks should be
135-
/// smart and specify their own `leeway` parameter.
136-
///
137-
/// - returns: Scaled interval in microseconds
123+
/// - returns: Scaled interval in minimal appropriate unit
138124
internal static func *(lhs: DispatchTimeInterval, rhs: Double) -> DispatchTimeInterval {
139125
let seconds = lhs.timeInterval * rhs
140-
return .microseconds(Int(seconds * 1000 * 1000))
126+
var result: DispatchTimeInterval = .never
127+
if let integerTimeInterval = Int(exactly: (seconds * 1000 * 1000 * 1000).rounded()) {
128+
result = .nanoseconds(integerTimeInterval)
129+
} else if let integerTimeInterval = Int(exactly: (seconds * 1000 * 1000).rounded()) {
130+
result = .microseconds(integerTimeInterval)
131+
} else if let integerTimeInterval = Int(exactly: (seconds * 1000).rounded()) {
132+
result = .milliseconds(integerTimeInterval)
133+
} else if let integerTimeInterval = Int(exactly: (seconds).rounded()) {
134+
result = .seconds(integerTimeInterval)
135+
}
136+
return result
141137
}
142138
}

Tests/ReactiveSwiftTests/FoundationExtensionsSpec.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,13 @@ class FoundationExtensionsSpec: QuickSpec {
9494
expect((DispatchTimeInterval.seconds(5) * 0.5).timeInterval).to(beCloseTo(DispatchTimeInterval.milliseconds(2500).timeInterval))
9595
expect((DispatchTimeInterval.seconds(1) * 0.25).timeInterval).to(beCloseTo(DispatchTimeInterval.milliseconds(250).timeInterval))
9696
}
97+
98+
it("should not introduce integer overflow upon scale") {
99+
expect((DispatchTimeInterval.seconds(Int.max) * 0.01).timeInterval).to(beCloseTo(10 * DispatchTimeInterval.milliseconds(Int.max).timeInterval, within: 1))
100+
expect((DispatchTimeInterval.milliseconds(Int.max) * 0.01).timeInterval).to(beCloseTo(10 * DispatchTimeInterval.microseconds(Int.max).timeInterval, within: 1))
101+
expect((DispatchTimeInterval.microseconds(Int.max) * 0.01).timeInterval).to(beCloseTo(10 * DispatchTimeInterval.nanoseconds(Int.max).timeInterval, within: 1))
102+
expect((DispatchTimeInterval.seconds(Int.max) * 10).timeInterval) == Double.infinity
103+
}
97104

98105
it("should produce the expected TimeInterval values") {
99106
expect(DispatchTimeInterval.seconds(1).timeInterval).to(beCloseTo(1.0))

0 commit comments

Comments
 (0)