Description

I've identified three critical bugs in the sliding window rate limiter implementation (middleware/limiter/limiter_sliding.go). These bugs can cause incorrect rate limiting behavior, especially under high concurrency.

Bug 1: Race Condition in SkipFailedRequests/SkipSuccessfulRequests

Location: Lines 117-133

Problem: When SkipFailedRequests or SkipSuccessfulRequests is enabled, the code releases the lock after incrementing currHits, executes the handler, then re-acquires the lock and reads the entry again from storage. This creates a race condition where concurrent requests can interfere with each other's counts.

Code:

// First: increment and save
e.currHits++
manager.set(c, key, e, ...)
mux.Unlock()  // ⚠️ Lock released

err = c.Next()  // Handler execution (time-consuming)

// Second: re-acquire and decrement
mux.Lock()
entry, getErr := manager.get(c, key)  // ⚠️ Re-fetch from storage
e = entry
e.currHits--  // ⚠️ May decrement wrong value

Scenario:

1. Request A: currHits = 5 → 6, saves, unlocks, executes handler
2. 2. Request B: Concurrently enters, reads currHits = 6 → 7, saves
3. 3. Request A: Handler completes, needs to skip, re-fetches currHits = 7 → 6
4. 4. Result: Request B's increment is incorrectly canceled!

Bug 2: Inconsistent Variable Usage for Remaining Calculation

Location: Line 91

Problem: The code uses cfg.Max instead of maxRequests when calculating remaining, but maxRequests is the dynamically generated limit that should be used.

Code:

// Line 33: Dynamic max value
maxRequests := cfg.MaxFunc(c)  // e.g., returns 100 for VIP users

// Line 91: Uses wrong variable ❌
remaining := cfg.Max - rate  // Uses fixed cfg.Max instead of maxRequests

// Line 99: Rate limit check
if remaining < 0 {
    return cfg.LimitReached(c)
}

// Line 137: Response header uses correct variable
c.Set(xRateLimitLimit, strconv.Itoa(maxRequests))  // ✅
c.Set(xRateLimitRemaining, strconv.Itoa(remaining))  // ❌ Wrong value

Scenario:

• cfg.Max = 10 (default)
• • maxRequests = 100 (VIP user)
• • • rate = 50
• • • • remaining = 10 - 50 = -40 ❌ Should be 100 - 50 = 50
• • • • • Result: VIP users are incorrectly rate-limited!

Bug 3: Incorrect Remaining Recalculation in Skip Logic

Location: Line 130

Problem: When skipping a request, the code simply does remaining++, but this ignores the sliding window weight calculation. The correct remaining depends on both prevHits * weight and currHits.

Code:

e.currHits--
remaining++  // ⚠️ Oversimplified calculation

Correct calculation:

weight := float64(resetInSec) / float64(expiration)
rate := int(float64(e.prevHits)*weight) + e.currHits
remaining = maxRequests - rate

Impact

1. Race condition: Inaccurate request counting under high concurrency
2. 2. Variable inconsistency: Dynamic rate limits (e.g., per-user quotas) don't work correctly
3. 3. Incorrect headers: X-RateLimit-Remaining shows wrong values to clients
4. 4. Security risk: Attackers could exploit these bugs to bypass rate limiting

Proposed Fixes

Fix 1: Eliminate Race Condition

Don't re-fetch from storage when skipping; use the original entry reference:

originalEntry := e
manager.set(c, key, e, ...)
mux.Unlock()

err = c.Next()
statusCode := getEffectiveStatusCode(c, err)

if shouldSkip {
    mux.Lock()
    originalEntry.currHits--  // Use original reference
    // Recalculate rate and remaining properly
    weight := float64(resetInSec) / float64(expiration)
    rate := int(float64(originalEntry.prevHits)*weight) + originalEntry.currHits
    remaining = maxRequests - rate
    
    manager.set(c, key, originalEntry, ...)
    mux.Unlock()
}

Fix 2: Use Consistent Variable

// Line 91 should be:
remaining := maxRequests - rate  // Use maxRequests instead of cfg.Max

Fix 3: Proper Remaining Recalculation

Already covered in Fix 1 - recalculate using the full formula instead of remaining++.

Environment

• Fiber version: v3.0.0-rc.2
• • Go version: 1.25.3
• • • File: middleware/limiter/limiter_sliding.go

Additional Notes

These bugs affect the core correctness of rate limiting. I recommend:

1. Adding comprehensive concurrent test cases
2. 2. Reviewing all paths where mux.Lock()/Unlock() is used
3. 3. Ensuring consistency between cfg.Max and maxRequests throughout the codebase