Skip to content

Reject negative rate limit costs#362

Open
carlok wants to merge 1 commit into
alisaifee:masterfrom
carlok:fix/negative-cost-validation
Open

Reject negative rate limit costs#362
carlok wants to merge 1 commit into
alisaifee:masterfrom
carlok:fix/negative-cost-validation

Conversation

@carlok

@carlok carlok commented Jun 23, 2026

Copy link
Copy Markdown

Summary

Hi Ali-Akber, first of all thank you for maintaining limits.

I noticed that negative cost values are currently not rejected before strategy evaluation. With the in-memory fixed-window and sliding-window-counter strategies, passing cost=-1 can increase the apparent remaining quota instead of consuming it.

This PR adds a small shared validation step in the sync and async RateLimiter base classes and applies it consistently before hit() / test() strategy logic.

The change deliberately preserves the existing cost=0 no-op behavior and only rejects cost < 0.

What changed

  • Reject negative cost values with ValueError
  • Apply validation in sync and async strategies
  • Add regression tests for:
    • FixedWindowRateLimiter
    • MovingWindowRateLimiter
    • SlidingWindowCounterRateLimiter
    • sync and async variants
    • no state mutation after invalid calls

Local verification

.venv/bin/ruff check limits/strategies.py limits/aio/strategies.py tests/test_strategy.py tests/aio/test_strategy.py
.venv/bin/pytest tests/test_strategy.py -k invalid_cost_is_rejected -q
.venv/bin/pytest tests/aio/test_strategy.py -k invalid_cost_is_rejected -q
.venv/bin/pytest tests/test_strategy.py -m memory -q
.venv/bin/pytest tests/aio/test_strategy.py -m memory -q

@alisaifee

alisaifee commented Jun 24, 2026

Copy link
Copy Markdown
Owner

Thanks for the contribution! You can fix most linting errors by running make lint-fix in the working directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants