Skip to content

Commit 1827e47

Browse files
Update rate limiter to have different configurable limits for IP-only and IP+UA (#219)
1 parent f41d2b7 commit 1827e47

File tree

7 files changed

+279
-129
lines changed

7 files changed

+279
-129
lines changed

handler/handler.go

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ type Signer interface {
4444
}
4545

4646
type Limiter interface {
47-
IsLimited(ip, ua string) (bool, error)
47+
IsLimited(ip, ua string) (limits.LimitStatus, error)
4848
}
4949

5050
// Client contains state needed for xyz.
@@ -170,26 +170,25 @@ func (c *Client) Nearest(rw http.ResponseWriter, req *http.Request) {
170170
ip = strings.TrimSpace(ips[0])
171171
}
172172
if ip != "" {
173-
// An empty UA is technically possible. In this case, the key will be
174-
// "ip:" and the rate limiting will be based on the IP address only.
173+
// An empty UA is technically possible.
175174
ua := req.Header.Get("User-Agent")
176-
limited, err := c.ipLimiter.IsLimited(ip, ua)
175+
status, err := c.ipLimiter.IsLimited(ip, ua)
177176
if err != nil {
178177
// Log error but don't block request (fail open).
179178
// TODO: Add tests for this path.
180179
log.Printf("Rate limiter error: %v", err)
181-
} else if limited {
180+
} else if status.IsLimited {
182181
// Log IP and UA and block the request.
183182
result.Error = v2.NewError("client", tooManyRequests, http.StatusTooManyRequests)
184183
metrics.RequestsTotal.WithLabelValues("nearest", "rate limit",
185184
http.StatusText(result.Error.Status)).Inc()
186185
// If the client provided a client_name, we want to know how many times
187186
// that client_name was rate limited. This may be empty, which is fine.
188187
clientName := req.Form.Get("client_name")
189-
metrics.RateLimitedTotal.WithLabelValues(clientName).Inc()
188+
metrics.RateLimitedTotal.WithLabelValues(clientName, status.LimitType).Inc()
190189

191-
log.Printf("Rate limit exceeded for IP: %s, client: %s, UA: %s", ip,
192-
clientName, ua)
190+
log.Printf("Rate limit (%s) exceeded for IP: %s, client: %s, UA: %s", ip,
191+
status.LimitType, clientName, ua)
193192
writeResult(rw, result.Error.Status, &result)
194193
return
195194
}

handler/handler_test.go

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,15 +72,15 @@ func (l *fakeAppEngineLocator) Locate(req *http.Request) (*clientgeo.Location, e
7272
}
7373

7474
type fakeRateLimiter struct {
75-
limited bool
76-
err error
75+
status limits.LimitStatus
76+
err error
7777
}
7878

79-
func (r *fakeRateLimiter) IsLimited(ip, ua string) (bool, error) {
79+
func (r *fakeRateLimiter) IsLimited(ip, ua string) (limits.LimitStatus, error) {
8080
if r.err != nil {
81-
return false, r.err
81+
return limits.LimitStatus{}, r.err
8282
}
83-
return r.limited, nil
83+
return r.status, nil
8484
}
8585

8686
func TestClient_Nearest(t *testing.T) {
@@ -220,7 +220,7 @@ func TestClient_Nearest(t *testing.T) {
220220
wantStatus: http.StatusOK,
221221
},
222222
{
223-
name: "error-rate-limit-exceeded",
223+
name: "error-rate-limit-exceeded-ip",
224224
path: "ndt/ndt5",
225225
signer: &fakeSigner{},
226226
locator: &fakeLocatorV2{
@@ -231,7 +231,29 @@ func TestClient_Nearest(t *testing.T) {
231231
"User-Agent": []string{"test-client"},
232232
},
233233
ipLimiter: &fakeRateLimiter{
234-
limited: true,
234+
status: limits.LimitStatus{
235+
IsLimited: true,
236+
LimitType: "ip",
237+
},
238+
},
239+
wantStatus: http.StatusTooManyRequests,
240+
},
241+
{
242+
name: "error-rate-limit-exceeded-ipua",
243+
path: "ndt/ndt5",
244+
signer: &fakeSigner{},
245+
locator: &fakeLocatorV2{
246+
targets: []v2.Target{{Machine: "mlab1-lga0t.measurement-lab.org"}},
247+
},
248+
header: http.Header{
249+
"X-Forwarded-For": []string{"192.0.2.1"},
250+
"User-Agent": []string{"test-client"},
251+
},
252+
ipLimiter: &fakeRateLimiter{
253+
status: limits.LimitStatus{
254+
IsLimited: true,
255+
LimitType: "ipua",
256+
},
235257
},
236258
wantStatus: http.StatusTooManyRequests,
237259
},
@@ -252,7 +274,10 @@ func TestClient_Nearest(t *testing.T) {
252274
"User-Agent": []string{"test-client"},
253275
},
254276
ipLimiter: &fakeRateLimiter{
255-
limited: false,
277+
status: limits.LimitStatus{
278+
IsLimited: false,
279+
LimitType: "",
280+
},
256281
},
257282
wantLatLon: "40.3,-70.4",
258283
wantKey: "ws://:3001/ndt_protocol",
@@ -297,7 +322,12 @@ func TestClient_Nearest(t *testing.T) {
297322
// No X-Forwarded-For
298323
"User-Agent": []string{"test-client"},
299324
},
300-
ipLimiter: &fakeRateLimiter{limited: false},
325+
ipLimiter: &fakeRateLimiter{
326+
status: limits.LimitStatus{
327+
IsLimited: false,
328+
LimitType: "",
329+
},
330+
},
301331
wantLatLon: "40.3,-70.4",
302332
wantKey: "ws://:3001/ndt_protocol",
303333
wantStatus: http.StatusOK,

limits/ratelimiter.go

Lines changed: 102 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -8,83 +8,147 @@ import (
88
"github.com/gomodule/redigo/redis"
99
)
1010

11-
// RateLimitConfig holds the configuration for IP+UA rate limiting.
12-
type RateLimitConfig struct {
13-
// Interval defines the duration of the sliding window.
11+
// LimitStatus indicates the result of a rate limit check
12+
type LimitStatus struct {
13+
// IsLimited indicates if the request should be rate limited
14+
IsLimited bool
15+
// LimitType indicates which limit was exceeded ("ip" or "ipua" or "")
16+
LimitType string
17+
}
18+
19+
// LimitConfig holds the configuration for a single rate limit type
20+
type LimitConfig struct {
21+
// Interval defines the duration of the sliding window
1422
Interval time.Duration
15-
// MaxEvents defines the maximum number of events allowed in the interval.
23+
24+
// MaxEvents defines the maximum number of events allowed in the interval
1625
MaxEvents int
17-
// KeyPrefix is the prefix for Redis keys.
26+
}
27+
28+
// RateLimitConfig holds the configuration for both IP-only and IP+UA rate limiting.
29+
type RateLimitConfig struct {
30+
// IPConfig defines the rate limiting configuration for IP-only checks
31+
IPConfig LimitConfig
32+
33+
// IPUAConfig defines the rate limiting configuration for IP+UA checks
34+
IPUAConfig LimitConfig
35+
36+
// KeyPrefix is the prefix for Redis keys
1837
KeyPrefix string
1938
}
2039

2140
// RateLimiter implements a distributed rate limiter using Redis sorted sets (ZSET).
22-
// It maintains a sliding window of events for each IP+UA combination, where:
41+
// It maintains sliding windows for both IP-only and IP+UA combinations, where:
2342
// - Each event is stored in a ZSET with the timestamp as score
2443
// - Old events (outside the window) are automatically removed
2544
// - Keys automatically expire after the configured interval
2645
//
2746
// The limiter considers a request to be rate-limited if the number of events
28-
// in the current window exceeds MaxEvents.
47+
// in either window exceeds their respective MaxEvents.
2948
type RateLimiter struct {
30-
pool *redis.Pool
31-
interval time.Duration
32-
maxEvents int
33-
keyPrefix string
49+
pool *redis.Pool
50+
ipConfig LimitConfig
51+
ipuaConfig LimitConfig
52+
keyPrefix string
3453
}
3554

3655
// NewRateLimiter creates a new rate limiter.
3756
func NewRateLimiter(pool *redis.Pool, config RateLimitConfig) *RateLimiter {
3857
return &RateLimiter{
39-
pool: pool,
40-
interval: config.Interval,
41-
maxEvents: config.MaxEvents,
42-
keyPrefix: config.KeyPrefix,
58+
pool: pool,
59+
ipConfig: config.IPConfig,
60+
ipuaConfig: config.IPUAConfig,
61+
keyPrefix: config.KeyPrefix,
4362
}
4463
}
4564

46-
// generateKey creates a Redis key from IP and User-Agent.
47-
func (rl *RateLimiter) generateKey(ip, ua string) string {
65+
// generateIPKey creates a Redis key from IP only.
66+
func (rl *RateLimiter) generateIPKey(ip string) string {
67+
return fmt.Sprintf("%s:%s", rl.keyPrefix, ip)
68+
}
69+
70+
// generateIPUAKey creates a Redis key from IP and User-Agent.
71+
func (rl *RateLimiter) generateIPUAKey(ip, ua string) string {
72+
// If User-Agent is empty, use "none" as the value. This allows to distinguish
73+
// between IP-only keys and IPUA keys with an empty User-Agent.
74+
if ua == "" {
75+
ua = "none"
76+
}
4877
return fmt.Sprintf("%s:%s:%s", rl.keyPrefix, ip, ua)
4978
}
5079

5180
// IsLimited checks if the given IP and User-Agent combination should be rate limited.
52-
func (rl *RateLimiter) IsLimited(ip, ua string) (bool, error) {
81+
// It first checks the IP-only limit, then the IP+UA limit if the IP-only check passes.
82+
func (rl *RateLimiter) IsLimited(ip, ua string) (LimitStatus, error) {
5383
conn := rl.pool.Get()
5484
defer conn.Close()
5585

5686
now := time.Now().UnixMicro()
57-
windowStart := now - rl.interval.Microseconds()
58-
redisKey := rl.generateKey(ip, ua)
59-
60-
// Send all commands in pipeline.
61-
// 1. Remove events outside the window
62-
conn.Send("ZREMRANGEBYSCORE", redisKey, "-inf", windowStart)
63-
// 2. Add current event
64-
conn.Send("ZADD", redisKey, now, strconv.FormatInt(now, 10))
65-
// 3. Set key expiration
66-
conn.Send("EXPIRE", redisKey, int64(rl.interval.Seconds()))
67-
// 4. Get total event count
68-
conn.Send("ZCARD", redisKey)
87+
ipKey := rl.generateIPKey(ip)
88+
ipuaKey := rl.generateIPUAKey(ip, ua)
89+
90+
// Start pipeline for both checks
91+
// 1. IP-only check
92+
conn.Send("ZREMRANGEBYSCORE", ipKey, "-inf", now-rl.ipConfig.Interval.Microseconds())
93+
conn.Send("ZADD", ipKey, now, strconv.FormatInt(now, 10))
94+
conn.Send("EXPIRE", ipKey, int64(rl.ipConfig.Interval.Seconds()))
95+
conn.Send("ZCARD", ipKey)
96+
97+
// 2. IP+UA limit check
98+
conn.Send("ZREMRANGEBYSCORE", ipuaKey, "-inf", now-rl.ipuaConfig.Interval.Microseconds())
99+
conn.Send("ZADD", ipuaKey, now, strconv.FormatInt(now, 10))
100+
conn.Send("EXPIRE", ipuaKey, int64(rl.ipuaConfig.Interval.Seconds()))
101+
conn.Send("ZCARD", ipuaKey)
69102

70103
// Flush pipeline
71104
if err := conn.Flush(); err != nil {
72-
return false, fmt.Errorf("failed to flush pipeline: %w", err)
105+
return LimitStatus{}, fmt.Errorf("failed to flush pipeline: %w", err)
73106
}
74107

75-
// Receive all replies
108+
// Receive first 3 replies for IP limit (ZREMRANGEBYSCORE, ZADD, EXPIRE)
76109
for i := 0; i < 3; i++ {
77-
// Receive replies for ZREMRANGEBYSCORE, ZADD, and EXPIRE
78110
if _, err := conn.Receive(); err != nil {
79-
return false, fmt.Errorf("failed to receive reply %d: %w", i, err)
111+
return LimitStatus{}, fmt.Errorf("failed to receive IP limit reply %d: %w", i, err)
80112
}
81113
}
82114

83-
// Receive and process ZCARD reply
84-
count, err := redis.Int64(conn.Receive())
115+
// Receive IP limit count
116+
ipCount, err := redis.Int64(conn.Receive())
85117
if err != nil {
86-
return false, fmt.Errorf("failed to receive count: %w", err)
118+
return LimitStatus{}, fmt.Errorf("failed to receive IP limit count: %w", err)
119+
}
120+
121+
// Check IP-only limit first
122+
if ipCount > int64(rl.ipConfig.MaxEvents) {
123+
return LimitStatus{
124+
IsLimited: true,
125+
LimitType: "ip",
126+
}, nil
127+
}
128+
129+
// Receive next 3 replies for IP+UA limit (ZREMRANGEBYSCORE, ZADD, EXPIRE)
130+
for i := 0; i < 3; i++ {
131+
if _, err := conn.Receive(); err != nil {
132+
return LimitStatus{}, fmt.Errorf("failed to receive IP+UA limit reply %d: %w", i, err)
133+
}
134+
}
135+
136+
// Receive IP+UA limit count
137+
ipuaCount, err := redis.Int64(conn.Receive())
138+
if err != nil {
139+
return LimitStatus{}, fmt.Errorf("failed to receive IP+UA limit count: %w", err)
140+
}
141+
142+
// Check IP+UA limit
143+
if ipuaCount > int64(rl.ipuaConfig.MaxEvents) {
144+
return LimitStatus{
145+
IsLimited: true,
146+
LimitType: "ipua",
147+
}, nil
87148
}
88149

89-
return count > int64(rl.maxEvents), nil
150+
return LimitStatus{
151+
IsLimited: false,
152+
LimitType: "",
153+
}, nil
90154
}

limits/ratelimiter_bench_test.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,16 @@ func BenchmarkRateLimiter_RealWorld(b *testing.B) {
3838
},
3939
}
4040

41-
// Create rate limiter with 60 requests/hour limit
41+
// Create rate limiter with different limits for IP and IP+UA
4242
limiter := NewRateLimiter(pool, RateLimitConfig{
43-
Interval: time.Hour,
44-
MaxEvents: 60,
43+
IPConfig: LimitConfig{
44+
Interval: time.Hour,
45+
MaxEvents: 120, // More permissive IP-only limit
46+
},
47+
IPUAConfig: LimitConfig{
48+
Interval: time.Hour,
49+
MaxEvents: 60, // Stricter IP+UA limit
50+
},
4551
KeyPrefix: "benchmark:",
4652
})
4753

@@ -59,8 +65,10 @@ func BenchmarkRateLimiter_RealWorld(b *testing.B) {
5965
tests := []struct {
6066
name string
6167
duration time.Duration
62-
ipRange int // Number of unique IPs
63-
uaRange int // Number of unique UAs
68+
ipRange int // Number of unique IPs
69+
uaRange int // Number of unique UAs
70+
ipOnly bool // Whether to test IP-only limiting
71+
6472
}{
6573
{
6674
name: "SingleIPUA_Long",

0 commit comments

Comments
 (0)