From 0861c47e54f3386e0734f9ab0f61376ff4acf2d3 Mon Sep 17 00:00:00 2001 From: Witold Duranek Date: Tue, 3 Jan 2023 16:16:05 +0100 Subject: [PATCH] fix no rate limiting if average is 0 --- pkg/middlewares/ratelimiter/rate_limiter.go | 12 +++++------- pkg/middlewares/ratelimiter/rate_limiter_test.go | 13 +++++++++++++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/pkg/middlewares/ratelimiter/rate_limiter.go b/pkg/middlewares/ratelimiter/rate_limiter.go index 312595bfcf..e066797876 100644 --- a/pkg/middlewares/ratelimiter/rate_limiter.go +++ b/pkg/middlewares/ratelimiter/rate_limiter.go @@ -79,10 +79,12 @@ func New(ctx context.Context, next http.Handler, config dynamic.RateLimit, name period = time.Second } - // if config.Average == 0, in that case, - // the value of maxDelay does not matter since the reservation will (buggily) give us a delay of 0 anyway. + // Initialized at rate.Inf to enforce no rate limiting when config.Average == 0 + rtl := float64(rate.Inf) + // No need to set any particular value for maxDelay as the reservation's delay + // will be <= 0 in the Inf case (i.e. the average == 0 case). var maxDelay time.Duration - var rtl float64 + if config.Average > 0 { rtl = float64(config.Average*int64(time.Second)) / float64(period) // maxDelay does not scale well for rates below 1, @@ -153,10 +155,6 @@ func (rl *rateLimiter) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - // time/rate is bugged, since a rate.Limiter with a 0 Limit not only allows a Reservation to take place, - // but also gives a 0 delay below (because of a division by zero, followed by a multiplication that flips into the negatives), - // regardless of the current load. - // However, for now we take advantage of this behavior to provide the no-limit ratelimiter when config.Average is 0. res := bucket.Reserve() if !res.OK() { http.Error(w, "No bursty traffic allowed", http.StatusTooManyRequests) diff --git a/pkg/middlewares/ratelimiter/rate_limiter_test.go b/pkg/middlewares/ratelimiter/rate_limiter_test.go index ac73fc2ecc..6705617745 100644 --- a/pkg/middlewares/ratelimiter/rate_limiter_test.go +++ b/pkg/middlewares/ratelimiter/rate_limiter_test.go @@ -15,6 +15,7 @@ import ( "github.com/traefik/traefik/v2/pkg/config/dynamic" "github.com/traefik/traefik/v2/pkg/testhelpers" "github.com/vulcand/oxy/v2/utils" + "golang.org/x/time/rate" ) func TestNewRateLimiter(t *testing.T) { @@ -25,7 +26,16 @@ func TestNewRateLimiter(t *testing.T) { expectedSourceIP string requestHeader string expectedError string + expectedRTL rate.Limit }{ + { + desc: "no ratelimit on Average == 0", + config: dynamic.RateLimit{ + Average: 0, + Burst: 10, + }, + expectedRTL: rate.Inf, + }, { desc: "maxDelay computation", config: dynamic.RateLimit{ @@ -120,6 +130,9 @@ func TestNewRateLimiter(t *testing.T) { assert.NoError(t, err) assert.Equal(t, test.requestHeader, hd) } + if test.expectedRTL != 0 { + assert.Equal(t, test.expectedRTL, rtl.rate) + } }) } }