Skip to content

fix(rate-limit): bound shard memory with eviction instead of unbounded growth + O(n) sweep - #230

Merged
ZhuchkaTriplesix merged 1 commit into
devfrom
issue-208-rate-limit-bounded-shards
Sep 29, 2026
Merged

ZhuchkaTriplesix merged 1 commit into
devfrom
issue-208-rate-limit-bounded-shards

Conversation

@ZhuchkaTriplesix

Copy link
Copy Markdown
Member

Problem

Each rate-limit shard's HashMap could grow without limit under a flood of unique keys — the periodic retain() only evicted entries idle longer than two windows. Once past MAX_SHARD_ENTRIES (8192), every check() call scanned the entire shard under its mutex: O(n) per request, on both memory and CPU. Combined with a client-controlled key (e.g. the trusted-forwarded-ip strategy from #207), this is a straightforward DoS.

Fix

  • Replaced the periodic full-shard retain sweep with strict per-insert bounded eviction: when a shard is at MAX_SHARD_ENTRIES and a new key arrives, evict one existing entry before inserting. check() is now O(1) per request regardless of key cardinality — no full-shard scan on the hot path.
  • Capped the key string itself at MAX_KEY_LEN (256 bytes), since keys can come from client-controlled input (e.g. a header value) and an oversized key would otherwise inflate a shard's memory footprint per request.

Testing

  • cargo test --lib rate_limit: 4 passed (2 new: test_shard_bounded_under_unique_key_flood" floods ~262k unique keys and asserts every shard stays ≤ MAX_SHARD_ENTRIES; test_key_length_is_cappedasserts a 1MB key is truncated to ≤MAX_KEY_LEN`).
  • cargo clippy --all-targets: clean.
  • Full suite (pytest tests/): 161 passed, 1 skipped.

Closes #208

…d growth + O(n) sweep

Each shard's HashMap could grow without limit under a flood of unique
keys, since the periodic retain() only evicted entries idle longer
than two windows. Once past MAX_SHARD_ENTRIES, every request scanned
the whole shard under its mutex — O(n) per request on both the memory
and CPU axes, a DoS vector especially combined with a client-supplied
key (e.g. the forwarded-IP strategy).

Replace the periodic full-shard sweep with strict per-insert bounded
eviction: when a shard is at MAX_SHARD_ENTRIES and a new key arrives,
evict one existing entry before inserting. This keeps check() O(1)
per request regardless of key cardinality, with no full-shard scan.

Also cap the key string itself at MAX_KEY_LEN bytes, since keys can
come from client-controlled input (e.g. a forwarded-IP header value)
and an oversized key would otherwise inflate a shard's memory
footprint per request.

Closes #208
@ZhuchkaTriplesix
ZhuchkaTriplesix merged commit af956a3 into dev Sep 29, 2026
17 checks passed
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.

1 participant