Skip to content

Commit

Permalink
fix no rate limiting if average is 0
Browse files Browse the repository at this point in the history
  • Loading branch information
witalisoft committed Jan 3, 2023
1 parent e1e8676 commit 0861c47
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 7 deletions.
12 changes: 5 additions & 7 deletions pkg/middlewares/ratelimiter/rate_limiter.go
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
13 changes: 13 additions & 0 deletions pkg/middlewares/ratelimiter/rate_limiter_test.go
Expand Up @@ -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) {
Expand All @@ -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{
Expand Down Expand Up @@ -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)
}
})
}
}
Expand Down

0 comments on commit 0861c47

Please sign in to comment.