rate: prevent overflows when calculating durationFromTokens
Currently, there is a conversion from float64 to int64 when returning the duration needed to accumulate the required number of tokens.
When limiters are set with low limits, i.e. 1e-10, the duration needed is greater than math.MaxInt64.
As per the language specifications, in these scenarios the outcome is implementation determined.
This results in overflows on `amd64`, resulting in no wait, effectively jamming the limiter open.
Here we add a check for this scenario, returning InfDuration if the desired duration is greater than math.MaxInt64.
Fixes golang/go#71154
Change-Id: I775aab80fcc8563a59aa399844a64ef70b9eb76a
Reviewed-on: https://go-review.googlesource.com/c/time/+/641336
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
diff --git a/rate/rate.go b/rate/rate.go
index 93a798a..ec5f0cd 100644
--- a/rate/rate.go
+++ b/rate/rate.go
@@ -405,8 +405,15 @@
if limit <= 0 {
return InfDuration
}
- seconds := tokens / float64(limit)
- return time.Duration(float64(time.Second) * seconds)
+
+ duration := (tokens / float64(limit)) * float64(time.Second)
+
+ // Cap the duration to the maximum representable int64 value, to avoid overflow.
+ if duration > float64(math.MaxInt64) {
+ return InfDuration
+ }
+
+ return time.Duration(duration)
}
// tokensFromDuration is a unit conversion function from a time duration to the number of tokens
diff --git a/rate/rate_test.go b/rate/rate_test.go
index c9bc239..8b93903 100644
--- a/rate/rate_test.go
+++ b/rate/rate_test.go
@@ -638,3 +638,20 @@
// We set the limit to 10/s so expect to get another token in 100ms
runWait(t, tt, lim, wait{"wait-after-set-nonzero-after-zero", context.Background(), 1, 1, true})
}
+
+// TestTinyLimit tests that a limiter does not allow more than burst, when the rate is tiny.
+// Prior to resolution of issue 71154, this test
+// would fail on amd64 due to overflow in durationFromTokens.
+func TestTinyLimit(t *testing.T) {
+ lim := NewLimiter(1e-10, 1)
+
+ // The limiter starts with 1 burst token, so the first request should succeed
+ if !lim.Allow() {
+ t.Errorf("Limit(1e-10, 1) want true when first used")
+ }
+
+ // The limiter should not have replenished the token bucket yet, so the second request should fail
+ if lim.Allow() {
+ t.Errorf("Limit(1e-10, 1) want false when already used")
+ }
+}
