Reviewing files that changed from the base of the PR and between 5caa478 and b8eabb4.

• middleware/limiter/limiter_sliding.go (3 hunks)
• middleware/limiter/limiter_test.go (2 hunks)

• GitHub Check: Upload results
• GitHub Check: Compare
• GitHub Check: lint
• GitHub Check: unit (1.25.x, windows-latest)
• GitHub Check: unit (1.25.x, macos-latest)
• GitHub Check: repeated

middleware/limiter/limiter_sliding.go (1)

85-132: Dynamic maxRequests correctly drives remaining and limit enforcement

Using remaining := maxRequests - rate and checking if remaining < 0 before calling cfg.LimitReached correctly ties the effective limit and remaining capacity to the per-request MaxFunc value, and keeps it consistent with X-RateLimit-Limit (which already uses maxRequests). This fixes the previous mismatch where a static cfg.Max could diverge from the dynamic limit while preserving the existing sliding-window math and storage semantics.

Reviewing files that changed from the base of the PR and between b8eabb4 and 035a918.

• middleware/limiter/limiter_sliding.go (5 hunks)
• middleware/limiter/limiter_test.go (3 hunks)

• GitHub Check: CodeQL analysis (go)
• GitHub Check: Agent
• GitHub Check: repeated
• GitHub Check: Compare

middleware/limiter/limiter_test.go (1)

772-801: LGTM: solid coverage of sliding-window recomputation after handler delay.

This test cleanly verifies that, with a delayed handler, the sliding window state and X-RateLimit-* headers are recalculated based on the updated window, and it follows existing test patterns (t.Parallel, plain handler, require in the test only). No changes needed.

middleware/limiter/limiter_sliding.go (1)

162-215: Overflow-safe TTL helpers are a nice improvement.

rotateWindow cleanly encapsulates the per-entry window rotation and returns resetInSec, and the new ttlDuration/secondsToDuration helpers correctly:

• Convert second counts to time.Duration with a bound (maxSeconds) to avoid overflow.
• Saturate to time.Duration(math.MaxInt64) both on conversion and on addition when resetDuration + expirationDuration would overflow.

This keeps the effective TTL as “time until reset + one more window” while making the arithmetic robust for large expirations.

Reviewing files that changed from the base of the PR and between 5d7697a and a36f95e.

• middleware/limiter/limiter_sliding.go (5 hunks)
• middleware/limiter/limiter_test.go (4 hunks)

• GitHub Check: unit (1.25.x, macos-latest)
• GitHub Check: repeated
• GitHub Check: unit (1.25.x, windows-latest)
• GitHub Check: lint
• GitHub Check: Compare

middleware/limiter/limiter_test.go (2)

375-399: Nice focused regression test for disabled headers + skip path

TestLimiterSlidingSkipsPostUpdateWhenHeadersDisabled cleanly verifies that with DisableHeaders: true and no skip flags, the sliding limiter only does one get/set on storage. This directly guards against unnecessary post-handler storage work and matches the new if skipHit || !cfg.DisableHeaders condition.

---

798-827: Window rotation / expiration tests align well with sliding-window math

Both Test_Limiter_Sliding_Window_RecalculatesAfterHandlerDelay and Test_Limiter_Sliding_Window_ExpiresStalePrevHits do a good job of:

• Exercising recomputation of rate/remaining after handler-induced delays, ensuring headers are based on the rotated window.
• Verifying that prevHits are dropped after more than one full window, avoiding overly strict limiting.

The sleep margins look sufficient for second‑granularity timestamps, so these should be stable under normal CI load.

Also applies to: 829-853

middleware/limiter/limiter_sliding.go (4)

64-79: Core sliding-window rate/remaining computation now correctly uses dynamic maxRequests

The refactor to:

• Rotate the window via rotateWindow(e, ts, expiration),
• Compute rate := int(float64(e.prevHits)*weight) + e.currHits with weight derived from resetInSec/expiration, and
• Derive remaining := maxRequests - rate using the per-request maxRequests from MaxFunc,

ensures dynamic limits are consistently applied for both enforcement and header values. Persisting with ttlDuration(resetInSec, expiration) also matches the “end of next window” TTL semantics described in the comment.

This looks logically sound and aligns with the new tests around dynamic MaxFunc and header behavior.

Also applies to: 92-96, 100-111

---

119-157: Skip‑hit path and window rotation fix the previous race and bucket mis-accounting

The combination of:

• Recomputing ts, resetInSec, weight and re-running rotateWindow after c.Next(),
• Using bucketForOriginalHit(e, windowExpiresAt, ts, expiration) to decide whether to decrement currHits or prevHits (or neither) for a skipped request, and
• Recalculating rate/remaining before persisting and updating headers,

addresses the earlier issues where:

• A long-running request could have its hit moved into the previous window, but the decrement still targeted currHits.
• The remaining calculation after skip was previously adjusted by +1 instead of recomputing from the sliding-window formula.

The updated logic is guarded by if skipHit || !cfg.DisableHeaders, so you avoid extra storage churn when neither skipping nor emitting headers. Overall, this is a solid, race-resistant implementation of the skip behavior.

Also applies to: 163-197

---

199-225: TTL helpers correctly guard against duration overflow

ttlDuration and secondsToDuration provide a clear and safe way to derive:

• A reset duration from resetInSec,
• The base expiration window, and
• A combined TTL clamped to math.MaxInt64 when either seconds are too large or resetDuration + expirationDuration would overflow.

This is a nice improvement over ad‑hoc arithmetic and should make behavior robust even with very large expiration values.

---

24-28: Config validation prevents the division-by-zero risk; however, the current approach lacks clarity about sub-second expiration handling

The concern in the review is partially mitigated by existing code. When cfg.Expiration is set to a sub-second value like 200 * time.Millisecond, the config validation in config.go (line 111) detects this because int(0.2) <= 0 evaluates to true, and automatically replaces it with the default 1 * time.Minute. This prevents the expiration == 0 scenario that would cause division by zero at line 72.

However, the review's underlying point remains valid: the behavior is implicit rather than explicit. The current code silently converts sub-second values without documentation, and the truncation via uint64(cfg.Expiration.Seconds()) is still a design issue even though it's protected by validation. Consider adding explicit validation with clear error messaging or documentation to make the sub-second limitation intentional and obvious to users.

Reviewing files that changed from the base of the PR and between a36f95e and 4299c50.

• middleware/limiter/limiter_sliding.go (5 hunks)

• GitHub Check: unit (1.25.x, windows-latest)
• GitHub Check: unit (1.25.x, macos-latest)
• GitHub Check: lint
• GitHub Check: repeated
• GitHub Check: Compare

middleware/limiter/limiter_sliding.go (4)

5-8: Dynamic maxRequests and Ceil-based rate calculation look correct; verify Expiration can’t be zero

Using maxRequests := cfg.MaxFunc(c) consistently for remaining := maxRequests - rate and setting X-RateLimit-Limit fixes the static cfg.Max issue, and switching to math.Ceil(float64(e.prevHits)*weight) correctly avoids truncating fractional previous-window hits.

One assumption here is that expiration := uint64(cfg.Expiration.Seconds()) is always ≥ 1; if a caller configures Expiration < time.Second, expiration becomes 0 and weight := float64(resetInSec) / float64(expiration) will panic with divide-by-zero. If this isn’t already enforced in Config validation, consider guarding or documenting the minimum expiration.

Also applies to: 64-80, 100-110

---

119-156: Skip logic + post-handler recomputation correctly model sliding window and dynamic max

The skipHit predicate based on SkipSuccessfulRequests / SkipFailedRequests plus the guarded block if skipHit || !cfg.DisableHeaders is a good balance between correctness and overhead: you only re-read state when you need to adjust hits or refresh headers.

Recomputing ts, calling rotateWindow again, recomputing weight, and then rebuilding rate and remaining ensures the second pass uses a fresh view of the window (addressing the stale-weight concern) and includes concurrent hits that arrived while c.Next() ran. Using bucketForOriginalHit(e, windowExpiresAt, ts, expiration) to decide which bucket to decrement avoids the earlier race where a skipped request could accidentally remove another request’s hit after a window rotation.

Behaviorally this matches the objectives: dynamic maxRequests is honored, skipped hits are rolled back against the correct bucket, and X-RateLimit headers reflect the updated sliding-window state.

---

163-185: rotateWindow correctly handles single and multi-window expirations

The new rotateWindow logic:

• Initializes e.exp on first use.
• For ts >= e.exp, computes elapsed := ts - e.exp and:
  • Resets both prevHits and currHits when elapsed >= expiration, so hits older than a full window are fully discarded.
  • Otherwise shifts currHits into prevHits, zeros currHits, and sets e.exp = ts + expiration - elapsed, giving resetInSec = e.exp - ts = expiration - elapsed.

This fixes the earlier bug where prevHits could retain hits from windows that should have expired, and it keeps resetInSec consistent with the sliding-window math used for weight.

---

187-197: bucketForOriginalHit cleanly identifies the correct bucket for skipped hits

Using the captured windowExpiresAt (requestExpiration) to decide where the original hit landed works well:

• ts < requestExpiration → still in the window that was “current” when the hit was added ⇒ decrement currHits.
• ts-requestExpiration < expiration → now in the immediately previous window ⇒ decrement prevHits.
• Otherwise, the hit’s window is older than one full expiration and is no longer counted ⇒ return nil.

The boundary choices (< vs <=) are appropriate: at ts == requestExpiration the hit moves into prevHits, and once ts-requestExpiration >= expiration the hit falls completely out of the sliding window. This gives the skip logic the necessary precision without over-complicating the state.