diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a38c001f..177fbbfeb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # master *Please add new entries at the top.* +1. Fixed integer overflow for `DispatchTimeInterval` in FoundationExtensions.swift (#506) # 3.0.0-alpha.1 # 3.0.0-alpha.1 diff --git a/Sources/FoundationExtensions.swift b/Sources/FoundationExtensions.swift index ba0c1b948..07d5a7a6c 100644 --- a/Sources/FoundationExtensions.swift +++ b/Sources/FoundationExtensions.swift @@ -93,7 +93,7 @@ extension DispatchTimeInterval { case let .milliseconds(ms): return TimeInterval(TimeInterval(ms) / 1000.0) case let .microseconds(us): - return TimeInterval(Int64(us) * Int64(NSEC_PER_USEC)) / TimeInterval(NSEC_PER_SEC) + return TimeInterval(Int64(us)) * TimeInterval(NSEC_PER_USEC) / TimeInterval(NSEC_PER_SEC) case let .nanoseconds(ns): return TimeInterval(ns) / TimeInterval(NSEC_PER_SEC) case .never: @@ -120,23 +120,19 @@ extension DispatchTimeInterval { /// Scales a time interval by the given scalar specified in `rhs`. /// - /// - note: This method is only used internally to "scale down" a time - /// interval. Specifically it's used only to scale intervals to 10% - /// of their original value for the default `leeway` parameter in - /// `Scheduler.schedule(after:action:)` schedule and similar - /// other methods. - /// - /// If seconds is over 200,000, 10% is ~2,000, and hence we end up - /// with a value of ~2,000,000,000. Not quite overflowing a signed - /// integer on 32-bit platforms, but close. - /// - /// Even still, 200,000 seconds should be a rarely (if ever) - /// specified interval for our APIs. And even then, folks should be - /// smart and specify their own `leeway` parameter. - /// - /// - returns: Scaled interval in microseconds + /// - returns: Scaled interval in minimal appropriate unit internal static func *(lhs: DispatchTimeInterval, rhs: Double) -> DispatchTimeInterval { let seconds = lhs.timeInterval * rhs - return .microseconds(Int(seconds * 1000 * 1000)) + var result: DispatchTimeInterval = .never + if let integerTimeInterval = Int(exactly: (seconds * 1000 * 1000 * 1000).rounded()) { + result = .nanoseconds(integerTimeInterval) + } else if let integerTimeInterval = Int(exactly: (seconds * 1000 * 1000).rounded()) { + result = .microseconds(integerTimeInterval) + } else if let integerTimeInterval = Int(exactly: (seconds * 1000).rounded()) { + result = .milliseconds(integerTimeInterval) + } else if let integerTimeInterval = Int(exactly: (seconds).rounded()) { + result = .seconds(integerTimeInterval) + } + return result } } diff --git a/Tests/ReactiveSwiftTests/FoundationExtensionsSpec.swift b/Tests/ReactiveSwiftTests/FoundationExtensionsSpec.swift index 3546974d8..c8ed66a8c 100644 --- a/Tests/ReactiveSwiftTests/FoundationExtensionsSpec.swift +++ b/Tests/ReactiveSwiftTests/FoundationExtensionsSpec.swift @@ -94,6 +94,13 @@ class FoundationExtensionsSpec: QuickSpec { expect((DispatchTimeInterval.seconds(5) * 0.5).timeInterval).to(beCloseTo(DispatchTimeInterval.milliseconds(2500).timeInterval)) expect((DispatchTimeInterval.seconds(1) * 0.25).timeInterval).to(beCloseTo(DispatchTimeInterval.milliseconds(250).timeInterval)) } + + it("should not introduce integer overflow upon scale") { + expect((DispatchTimeInterval.seconds(Int.max) * 0.01).timeInterval).to(beCloseTo(10 * DispatchTimeInterval.milliseconds(Int.max).timeInterval, within: 1)) + expect((DispatchTimeInterval.milliseconds(Int.max) * 0.01).timeInterval).to(beCloseTo(10 * DispatchTimeInterval.microseconds(Int.max).timeInterval, within: 1)) + expect((DispatchTimeInterval.microseconds(Int.max) * 0.01).timeInterval).to(beCloseTo(10 * DispatchTimeInterval.nanoseconds(Int.max).timeInterval, within: 1)) + expect((DispatchTimeInterval.seconds(Int.max) * 10).timeInterval) == Double.infinity + } it("should produce the expected TimeInterval values") { expect(DispatchTimeInterval.seconds(1).timeInterval).to(beCloseTo(1.0))