fix(rate-limit): bound shard memory with eviction instead of unbounded growth + O(n) sweep - #230
Merged
Merged
Conversation
…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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Each rate-limit shard's
HashMapcould grow without limit under a flood of unique keys — the periodicretain()only evicted entries idle longer than two windows. Once pastMAX_SHARD_ENTRIES(8192), everycheck()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. thetrusted-forwarded-ipstrategy from #207), this is a straightforward DoS.Fix
retainsweep with strict per-insert bounded eviction: when a shard is atMAX_SHARD_ENTRIESand 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.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.pytest tests/): 161 passed, 1 skipped.Closes #208