Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use WithRequestLimit with 0 to skip rate limit #46

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions context.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,26 @@ const (
requestLimitKey
)

const _NoLimit = -1

func WithIncrement(ctx context.Context, value int) context.Context {
return context.WithValue(ctx, incrementKey, value)
}

func getIncrement(ctx context.Context) int {
if value, ok := ctx.Value(incrementKey).(int); ok {
return value
}
return 1
func getIncrement(ctx context.Context) (int, bool) {
value, ok := ctx.Value(incrementKey).(int)
return value, ok
}
Comment on lines +16 to +19
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on no ctx key, we should by default return 1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do it in the only place where this is used: https://github.com/klaidliadon/httprate/blob/ac748ea5a6b8921aef7a442985c77657a989932f/limiter.go#L90-L97

	increment, ok := getIncrement(r.Context())
	if !ok {
		increment = 1
	}
	// If the increment is 0, we do not limit
	if increment == 0 {
		return false
	}


func WithNoLimit(ctx context.Context) context.Context {
return context.WithValue(ctx, requestLimitKey, _NoLimit)
klaidliadon marked this conversation as resolved.
Show resolved Hide resolved
}

func WithRequestLimit(ctx context.Context, value int) context.Context {
return context.WithValue(ctx, requestLimitKey, value)
}

func getRequestLimit(ctx context.Context) int {
if value, ok := ctx.Value(requestLimitKey).(int); ok {
return value
}
return 0
func getRequestLimit(ctx context.Context) (int, bool) {
value, ok := ctx.Value(requestLimitKey).(int)
return value, ok
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ go 1.17

require github.com/cespare/xxhash/v2 v2.3.0

require golang.org/x/sync v0.7.0 // indirect
require golang.org/x/sync v0.7.0
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
github.com/cespare/xxhash/v2 v2.1.2 h1:YRXhKfTDauu4ajMg1TPgFO5jnlC2HCbmLXMcTG5cbYE=
github.com/cespare/xxhash/v2 v2.1.2/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs=
github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
golang.org/x/sync v0.7.0 h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M=
Expand Down
36 changes: 27 additions & 9 deletions limiter.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,35 @@ func (l *RateLimiter) OnLimit(w http.ResponseWriter, r *http.Request, key string
currentWindow := time.Now().UTC().Truncate(l.windowLength)
ctx := r.Context()

limit := l.requestLimit
if val := getRequestLimit(ctx); val > 0 {
limit = val
setHeader(w, l.headers.Reset, fmt.Sprintf("%d", currentWindow.Add(l.windowLength).Unix()))

limit, ok := getRequestLimit(ctx)
if !ok {
limit = l.requestLimit
}
// If the limit is set to 0, we are always over limit
if limit == 0 {
return true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should be sending some headers back

X-RateLimit-Limit: 0
X-RateLimit-Remaining: 0

}
// If the limit is set to -1, we are never over limit
if limit == _NoLimit {
return false
}

increment, ok := getIncrement(r.Context())
if !ok {
increment = 1
}
// If the increment is 0, we are always on limit
if increment == 0 {
return false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should set X-RateLimit-Increment: 0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

setHeader(w, l.headers.Limit, fmt.Sprintf("%d", limit))
setHeader(w, l.headers.Reset, fmt.Sprintf("%d", currentWindow.Add(l.windowLength).Unix()))

if increment > 1 {
setHeader(w, l.headers.Increment, fmt.Sprintf("%d", increment))
}

l.mu.Lock()
_, rateFloat, err := l.calculateRate(key, limit)
Expand All @@ -88,11 +111,6 @@ func (l *RateLimiter) OnLimit(w http.ResponseWriter, r *http.Request, key string
}
rate := int(math.Round(rateFloat))

increment := getIncrement(r.Context())
if increment > 1 {
setHeader(w, l.headers.Increment, fmt.Sprintf("%d", increment))
}

if rate+increment > limit {
setHeader(w, l.headers.Remaining, fmt.Sprintf("%d", limit-rate))

Expand Down
4 changes: 2 additions & 2 deletions limiter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,8 @@ func TestResponseHeaders(t *testing.T) {
requestsLimit: 5,
increments: []int{0, 0, 0, 0, 0, 0},
respCodes: []int{200, 200, 200, 200, 200, 200},
respLimitHeader: []string{"5", "5", "5", "5", "5", "5"},
respRemainingHeader: []string{"5", "5", "5", "5", "5", "5"},
respLimitHeader: []string{"", "", "", "", "", ""},
respRemainingHeader: []string{"", "", "", "", "", ""},
},
{
name: "always block",
Expand Down
Loading