Skip to content

Commit c6100b7

Browse files
committed
fix(api): replace defaultdict with plain dict in rate limiter
avoid defaultdict side effects on key access; use .get() for reads and explicit assignment for writes to prevent empty key accumulation
1 parent b3191a1 commit c6100b7

2 files changed

Lines changed: 18 additions & 9 deletions

File tree

apps/api/src/lib/rate_limit.py

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
import functools
66
import time
7-
from collections import defaultdict
87
from collections.abc import Awaitable, Callable
98
from dataclasses import dataclass, field
109
from typing import TYPE_CHECKING, Any, cast
@@ -46,7 +45,7 @@ class InMemoryRateLimiter:
4645

4746
requests: int
4847
window: int
49-
_storage: dict[str, list[float]] = field(default_factory=lambda: defaultdict(list))
48+
_storage: dict[str, list[float]] = field(default_factory=dict)
5049

5150
def is_allowed(self, key: str) -> tuple[bool, int, int]:
5251
"""
@@ -59,21 +58,18 @@ def is_allowed(self, key: str) -> tuple[bool, int, int]:
5958
window_start = now - self.window
6059

6160
# Clean old entries
62-
entries = [ts for ts in self._storage[key] if ts > window_start]
63-
if entries:
64-
self._storage[key] = entries
65-
elif key in self._storage:
66-
# Remove empty keys to prevent memory leak
67-
del self._storage[key]
61+
entries = [ts for ts in self._storage.get(key, []) if ts > window_start]
6862

6963
current_count = len(entries)
7064
remaining = max(0, self.requests - current_count - 1)
7165
reset_after = int(self.window - (now - entries[0])) if entries else self.window
7266

7367
if current_count >= self.requests:
68+
self._storage[key] = entries
7469
return False, 0, reset_after
7570

76-
self._storage[key].append(now)
71+
entries.append(now)
72+
self._storage[key] = entries
7773
return True, remaining, reset_after
7874

7975

apps/api/tests/test_rate_limit.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,19 @@ def test_different_keys_independent(self) -> None:
5252
assert allowed_b is True
5353
assert remaining_b == 1
5454

55+
def test_expired_keys_cleaned_up(self) -> None:
56+
"""Expired keys should not persist as empty entries in storage."""
57+
limiter = InMemoryRateLimiter(requests=5, window=1)
58+
limiter.is_allowed("ephemeral")
59+
assert "ephemeral" in limiter._storage
60+
# Simulate expiry by clearing entries and setting old timestamps
61+
limiter._storage["ephemeral"] = [0.0] # timestamp in the distant past
62+
limiter.is_allowed("other")
63+
# 'ephemeral' not accessed → still there but with old data
64+
# Access 'ephemeral' → old entry filtered out, new one added
65+
limiter.is_allowed("ephemeral")
66+
assert len(limiter._storage["ephemeral"]) == 1
67+
5568

5669
# ── Config-keyed limiter registry ───────────────────────────────
5770

0 commit comments

Comments
 (0)