fix(06-07): make limiter admission atomic

- Replace split store checks with one mutex-guarded Attempt operation
- Use domainless trusted-client keys for anonymous inline throttles
- Preserve fixed-window headers, expiry, stacking, and principal isolation
This commit is contained in:
Jakub Zych
2026-09-20 17:02:53 +02:00
parent 5bcd7ba011
commit 6852a8f8c3
4 changed files with 56 additions and 83 deletions

View File

@@ -93,8 +93,8 @@ func (l *FixedWindowLimiter) Middleware(param string) pact.Middleware {
return return
} }
key := b.Key(r) key := b.Key(r)
if l.store.TooManyAttempts(key, b.Max) { allowed, attempts, retryAfter := l.store.Attempt(key, b.Max, b.Decay)
retryAfter := l.store.AvailableIn(key) if !allowed {
secs := int(retryAfter / time.Second) secs := int(retryAfter / time.Second)
w.Header().Set("Content-Type", "application/json") w.Header().Set("Content-Type", "application/json")
w.Header().Set("Retry-After", strconv.Itoa(secs)) w.Header().Set("Retry-After", strconv.Itoa(secs))
@@ -105,7 +105,6 @@ func (l *FixedWindowLimiter) Middleware(param string) pact.Middleware {
_, _ = w.Write([]byte(tooManyAttemptsBody)) _, _ = w.Write([]byte(tooManyAttemptsBody))
return return
} }
attempts := l.store.Hit(key, b.Decay)
remaining := b.Max - attempts remaining := b.Max - attempts
if remaining < 0 { if remaining < 0 {
remaining = 0 remaining = 0
@@ -157,11 +156,7 @@ func (l *FixedWindowLimiter) resolve(param string) (Bucket, error) {
return "u:" + strconv.FormatUint(uint64(u.ID), 10) return "u:" + strconv.FormatUint(uint64(u.ID), 10)
} }
} }
host := "" return "inline:domainless|" + ClientIP(r, trusted)
if r != nil {
host = r.Host
}
return host + "|" + ClientIP(r, trusted)
}, },
} }
l.inline[param] = b l.inline[param] = b

View File

@@ -12,20 +12,20 @@ import (
"git.golem15.com/golem15/summercms/compass" "git.golem15.com/golem15/summercms/compass"
) )
// Gap (b): MemoryStore's sweep goroutine (purge), not TooManyAttempts' lazy // Gap (b): MemoryStore's sweep goroutine (purge), not Attempt's lazy expiry.
// expiry. Construct with a short sweep and assert the internal map drops // Construct with a short sweep and assert the internal map drops
// an expired entry without calling TooManyAttempts. // an expired entry without a later Attempt.
func TestMemoryStoreSweepRemovesExpiredEntry(t *testing.T) { func TestMemoryStoreSweepRemovesExpiredEntry(t *testing.T) {
s := NewMemoryStore(15 * time.Millisecond) s := NewMemoryStore(15 * time.Millisecond)
t.Cleanup(func() { close(s.stop) }) t.Cleanup(func() { close(s.stop) })
s.Hit("k", 25*time.Millisecond) s.Attempt("k", 1, 25*time.Millisecond)
s.mu.Lock() s.mu.Lock()
n := len(s.entries) n := len(s.entries)
s.mu.Unlock() s.mu.Unlock()
if n != 1 { if n != 1 {
t.Fatalf("after Hit, entries = %d", n) t.Fatalf("after Attempt, entries = %d", n)
} }
deadline := time.Now().Add(200 * time.Millisecond) deadline := time.Now().Add(200 * time.Millisecond)
@@ -41,18 +41,20 @@ func TestMemoryStoreSweepRemovesExpiredEntry(t *testing.T) {
t.Fatalf("sweep did not drop expired entry, count=%d", n) t.Fatalf("sweep did not drop expired entry, count=%d", n)
} }
func TestMemoryStoreAvailableInExpiredAndMissing(t *testing.T) { func TestMemoryStoreAttemptRetryAfterAndExpiry(t *testing.T) {
s := NewMemoryStore(0) s := NewMemoryStore(0)
if d := s.AvailableIn("missing"); d != 0 { allowed, attempts, retryAfter := s.Attempt("k", 1, 20*time.Millisecond)
t.Fatalf("missing AvailableIn = %s", d) if !allowed || attempts != 1 || retryAfter <= 0 {
t.Fatalf("first attempt = allowed %v, attempts %d, retryAfter %s", allowed, attempts, retryAfter)
} }
s.Hit("k", 20*time.Millisecond) allowed, attempts, retryAfter = s.Attempt("k", 1, 20*time.Millisecond)
if d := s.AvailableIn("k"); d <= 0 { if allowed || attempts != 1 || retryAfter <= 0 {
t.Fatalf("live AvailableIn = %s", d) t.Fatalf("denied attempt = allowed %v, attempts %d, retryAfter %s", allowed, attempts, retryAfter)
} }
time.Sleep(30 * time.Millisecond) time.Sleep(30 * time.Millisecond)
if d := s.AvailableIn("k"); d != 0 { allowed, attempts, retryAfter = s.Attempt("k", 1, 20*time.Millisecond)
t.Fatalf("expired AvailableIn = %s", d) if !allowed || attempts != 1 || retryAfter <= 0 {
t.Fatalf("expired attempt = allowed %v, attempts %d, retryAfter %s", allowed, attempts, retryAfter)
} }
} }

View File

@@ -5,14 +5,11 @@ import (
"time" "time"
) )
// Store mirrors Illuminate\Cache\RateLimiter's hit/tooManyAttempts/ // Store owns fixed-window admission as one atomic operation. Attempt performs
// availableIn control flow: a fixed window, first-hit-wins (an existing // lazy expiry, threshold comparison, and an admitted increment together so
// unexpired window is never extended), with resetAttempts as a side effect // concurrent callers cannot pass a split check-then-increment boundary.
// of TooManyAttempts observing an expired window.
type Store interface { type Store interface {
Hit(key string, decay time.Duration) (attempts int) Attempt(key string, max int, decay time.Duration) (allowed bool, attempts int, retryAfter time.Duration)
TooManyAttempts(key string, max int) bool
AvailableIn(key string) time.Duration
} }
type counterEntry struct { type counterEntry struct {
@@ -68,48 +65,29 @@ func (s *MemoryStore) purge() {
} }
} }
// Hit increments key's counter, opening a decay window on first hit // Attempt admits and counts one request when key is below max. The first
// (first-hit-wins: an existing unexpired window is never extended). // attempt opens the window; later attempts never extend it. A denied attempt
func (s *MemoryStore) Hit(key string, decay time.Duration) int { // leaves the exhausted count unchanged.
func (s *MemoryStore) Attempt(key string, max int, decay time.Duration) (bool, int, time.Duration) {
s.mu.Lock() s.mu.Lock()
defer s.mu.Unlock() defer s.mu.Unlock()
now := time.Now() now := time.Now()
e, ok := s.entries[key] e, ok := s.entries[key]
if !ok || now.After(e.resetAt) { if ok && now.After(e.resetAt) {
delete(s.entries, key)
ok = false
}
if !ok {
e = &counterEntry{count: 0, resetAt: now.Add(decay)} e = &counterEntry{count: 0, resetAt: now.Add(decay)}
s.entries[key] = e s.entries[key] = e
} }
retryAfter := e.resetAt.Sub(now)
if retryAfter < 0 {
retryAfter = 0
}
if e.count >= max {
return false, e.count, retryAfter
}
e.count++ e.count++
return e.count return true, e.count, retryAfter
}
// TooManyAttempts is true only while count >= max and the window has not
// expired. An expired window is deleted (PHP resetAttempts) and returns false.
func (s *MemoryStore) TooManyAttempts(key string, max int) bool {
s.mu.Lock()
defer s.mu.Unlock()
e, ok := s.entries[key]
if !ok {
return false
}
if time.Now().After(e.resetAt) {
delete(s.entries, key)
return false
}
return e.count >= max
}
// AvailableIn is the time until the window resets, or 0 if the key is absent.
func (s *MemoryStore) AvailableIn(key string) time.Duration {
s.mu.Lock()
defer s.mu.Unlock()
e, ok := s.entries[key]
if !ok {
return 0
}
d := e.resetAt.Sub(time.Now())
if d < 0 {
return 0
}
return d
} }

View File

@@ -129,39 +129,37 @@ func TestFixedWindowLimiterMemoryStoreWindow(t *testing.T) {
s := NewMemoryStore(0) s := NewMemoryStore(0)
decay := 80 * time.Millisecond decay := 80 * time.Millisecond
if c := s.Hit("k", decay); c != 1 { if allowed, attempts, _ := s.Attempt("k", 3, decay); !allowed || attempts != 1 {
t.Fatalf("hit 1 = %d", c) t.Fatalf("attempt 1 = allowed %v, attempts %d", allowed, attempts)
} }
if c := s.Hit("k", decay); c != 2 { if allowed, attempts, _ := s.Attempt("k", 3, decay); !allowed || attempts != 2 {
t.Fatalf("hit 2 = %d", c) t.Fatalf("attempt 2 = allowed %v, attempts %d", allowed, attempts)
} }
if s.TooManyAttempts("k", 3) { if allowed, attempts, _ := s.Attempt("k", 3, decay); !allowed || attempts != 3 {
t.Fatal("TooManyAttempts at count==max-1") t.Fatalf("attempt 3 = allowed %v, attempts %d", allowed, attempts)
} }
if c := s.Hit("k", decay); c != 3 { if allowed, attempts, _ := s.Attempt("k", 3, decay); allowed || attempts != 3 {
t.Fatalf("hit 3 = %d", c) t.Fatalf("denied attempt = allowed %v, attempts %d", allowed, attempts)
}
if !s.TooManyAttempts("k", 3) {
t.Fatal("TooManyAttempts at count==max")
} }
time.Sleep(decay + 20*time.Millisecond) time.Sleep(decay + 20*time.Millisecond)
if s.TooManyAttempts("k", 3) { if allowed, attempts, _ := s.Attempt("k", 3, decay); !allowed || attempts != 1 {
t.Fatal("TooManyAttempts after window elapsed") t.Fatalf("fresh-window attempt = allowed %v, attempts %d", allowed, attempts)
}
if c := s.Hit("k", decay); c != 1 {
t.Fatalf("fresh window hit = %d", c)
} }
} }
func TestFixedWindowLimiterMemoryStoreFirstHitWins(t *testing.T) { func TestFixedWindowLimiterMemoryStoreFirstHitWins(t *testing.T) {
s := NewMemoryStore(0) s := NewMemoryStore(0)
decay := 200 * time.Millisecond decay := 200 * time.Millisecond
s.Hit("k", decay) if allowed, _, _ := s.Attempt("k", 2, decay); !allowed {
t.Fatal("first attempt denied")
}
time.Sleep(120 * time.Millisecond) time.Sleep(120 * time.Millisecond)
s.Hit("k", decay) // must not extend the original window if allowed, _, _ := s.Attempt("k", 2, decay); !allowed {
t.Fatal("second attempt denied")
}
time.Sleep(100 * time.Millisecond) time.Sleep(100 * time.Millisecond)
if s.TooManyAttempts("k", 1) { if allowed, attempts, _ := s.Attempt("k", 1, decay); !allowed || attempts != 1 {
t.Fatal("window was extended by a later hit") t.Fatal("window was extended by a later hit")
} }
} }