Skip to content

🐛 fix(limiter): floor sub-second expiration in sliding window - #4564

Open
nikolauspschuetz wants to merge 1 commit into
gofiber:mainfrom
nikolauspschuetz:fix-limiter-subsecond-expiration
Open

🐛 fix(limiter): floor sub-second expiration in sliding window#4564
nikolauspschuetz wants to merge 1 commit into
gofiber:mainfrom
nikolauspschuetz:fix-limiter-subsecond-expiration

Conversation

@nikolauspschuetz

Copy link
Copy Markdown

Description

SlidingWindow computes the window length as:

expiration := uint64(expirationDuration.Seconds())

Any positive sub-second ExpirationFunc value truncates to 0. The window weight is then

weight := float64(resetInSec) / float64(expiration) // / 0  -> +Inf/NaN
rate   := int(math.Ceil(float64(e.prevHits)*weight)) + e.currHits

so rate becomes garbage and every request — including the first — is rejected with 429, regardless of Max. A limiter configured with, say, ExpirationFunc: func(fiber.Ctx) time.Duration { return 500*time.Millisecond } blocks all traffic.

Fix

Floor the truncated window to 1 second (a sub-second window is treated as a 1-second window rather than breaking):

expiration := uint64(expirationDuration.Seconds())
if expiration == 0 {
    expiration = 1
}

Testing

Added TestLimiterSlidingSubSecondExpiration, which is rejected with 429 before this change and returns 200 after. go test ./middleware/limiter/ (full package), go vet, and gofmt are clean.

Note: FixedWindow shares the same uint64(...Seconds()) truncation; I scoped this PR to SlidingWindow since that's the one with a clean, demonstrable failure (the NaN-rate 429). Happy to follow up on the fixed-window window-sizing behavior separately if maintainers agree it's worth addressing.

SlidingWindow computed the expiration as uint64(expirationDuration.Seconds()),
so any positive sub-second ExpirationFunc value (e.g. 500ms) truncated to 0.
The window weight is then float64(resetInSec) / float64(expiration), a
division by zero that makes the rate NaN, so every request — including the
first — was rejected with 429 regardless of Max.

Floor the truncated window to 1 second. Adds a regression test that is
rejected with 429 before the fix and returns 200 after.
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: eccd8dcc-f584-42d8-b4e8-d6446f3f0227

📥 Commits

Reviewing files that changed from the base of the PR and between ed59a77 and f1087ab.

📒 Files selected for processing (2)
  • middleware/limiter/limiter_sliding.go
  • middleware/limiter/limiter_test.go

Walkthrough

The sliding-window limiter now floors sub-second expiration durations to one second, with a regression test covering a 500ms expiration and successful initial request.

Changes

Sliding window expiration

Layer / File(s) Summary
Expiration guard and regression coverage
middleware/limiter/limiter_sliding.go, middleware/limiter/limiter_test.go
SlidingWindow.New prevents a zero-second window after duration conversion, and a parallel regression test verifies that a 500ms expiration accepts the first request.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

  • gofiber/fiber#3893: Updates related sliding-window expiration and duration-to-seconds conversion logic.

Suggested reviewers: efectn

Poem

I’m a bunny guarding the window tonight,
One second keeps every count just right.
Five little hops pass through the gate,
No zero-time trick can seal their fate.
500ms now earns an “OK”!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: flooring sub-second sliding-window expirations.
Description check ✅ Passed The description explains the bug, fix, and testing, though it omits the issue link and several template checklist sections.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ReneWerner87 ReneWerner87 added this to v3 Jul 28, 2026
@ReneWerner87 ReneWerner87 added this to the v3 milestone Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants