From 2c6c5a229852db5dff1662b3a80d9a396939412f Mon Sep 17 00:00:00 2001 From: Tom Date: Wed, 8 Jan 2025 13:59:48 +0000 Subject: [PATCH] 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 Auto-Submit: Ian Lance Taylor LUCI-TryBot-Result: Go LUCI Reviewed-by: Ian Lance Taylor --- rate/rate.go | 11 +++++++++-- rate/rate_test.go | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) 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 @@ func (limit Limit) durationFromTokens(tokens float64) time.Duration { 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 @@ func TestSetAfterZeroLimit(t *testing.T) { // 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") + } +}