Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions TODO-03-rate-limiter-memory-leak.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# TODO: Fix unbounded memory leak in sliding-window rate limiters

## Steps

- [x] Read and understand source files (rate-limiter.js, conn-rate-limiter.js)
- [x] Read existing test files (rate-limiter.test.js, conn-rate-limiter.test.js)
- [x] Issue analysis complete
- [x] **Edit `src/rate-limiter.js`**:
- [x] Replace `shift()` loop with batch `filter()` in `check()`
- [x] Add `cleanup()` method (iterate all keys, filter stale timestamps)
- [x] Add `size` getter
- [x] **Edit `src/conn-rate-limiter.js`**:
- [x] Replace `shift()` loop with batch `filter()` in `check()`
- [x] Add `cleanup()` method (iterate all keys, filter stale timestamps)
- [x] Add `size` getter
- [x] Add `remove(ip)` method for API parity
- [ ] Create PR

53 changes: 50 additions & 3 deletions src/conn-rate-limiter.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,24 @@
/**
* Per-IP connection rate limiter using a sliding 60-second window.
* Memory is bounded: stale entries are proactively evictable via cleanup()
* and check() uses batch filter() instead of O(n) shift().
*
* @param {number} [maxPerMinute]
* @returns {{ check: (ip: string) => boolean }}
* @returns {{ check: (ip: string) => boolean, remove: (ip: string) => void, cleanup: () => void, size: number }}
*/
export function createConnRateLimiter(maxPerMinute) {
const limit = maxPerMinute ?? (Number(process.env.MAX_CONNECTIONS_PER_IP ?? process.env.CONN_RATE_LIMIT) || 30);
/** @type {Map<string, number[]>} */
const windows = new Map();

return {
/**
* Returns true if the connection is allowed, false if rate-limited.
* Side effect: records the current timestamp for the IP.
*
* @param {string} ip
* @returns {boolean}
*/
check(ip) {
const now = Date.now();
const cutoff = now - 60_000;
Expand All @@ -17,12 +27,49 @@ export function createConnRateLimiter(maxPerMinute) {
timestamps = [];
windows.set(ip, timestamps);
}
while (timestamps.length > 0 && timestamps[0] <= cutoff) {
timestamps.shift();
// Batch-evict entries outside the 60-second window (O(k) where k = expired entries)
if (timestamps.length > 0 && timestamps[0] <= cutoff) {
const filtered = timestamps.filter(t => t > cutoff);
windows.set(ip, filtered);
timestamps = filtered;
}
if (timestamps.length >= limit) return false;
timestamps.push(now);
return true;
},

/**
* Removes the rate-limit state for an IP (call on disconnect).
*
* @param {string} ip
*/
remove(ip) {
windows.delete(ip);
},

/**
* Iterates all tracked IPs and evicts stale entries.
* Keys with no remaining timestamps within the window are removed entirely.
* This is safe to call from a periodic timer (e.g. every 30s).
*/
cleanup() {
const cutoff = Date.now() - 60_000;
for (const [ip, timestamps] of windows) {
const filtered = timestamps.filter(t => t > cutoff);
if (filtered.length === 0) {
windows.delete(ip);
} else {
windows.set(ip, filtered);
}
}
},

/**
* Returns the number of distinct IPs currently tracked by this limiter.
* @returns {number}
*/
get size() {
return windows.size;
},
};
}
40 changes: 34 additions & 6 deletions src/rate-limiter.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
* Tracks message timestamps in a 1-second window for each client.
* Clients that exceed the configured limit have their messages dropped
* and receive an error frame.
*
* Memory is bounded: stale entries are proactively evictable via cleanup()
* and check() uses batch filter() instead of O(n) shift().
*/

/**
Expand All @@ -12,7 +15,7 @@
*
* @param {number} [maxPerSecond] - Max messages per second per client.
* Defaults to MAX_MESSAGES_PER_SECOND env var, or 100.
* @returns {{ check: (clientId: string) => boolean, remove: (clientId: string) => void }}
* @returns {{ check: (clientId: string) => boolean, remove: (clientId: string) => void, cleanup: () => void, size: number }}
*/
export function createRateLimiter(maxPerSecond) {
const envLimit = Number(process.env.MAX_MESSAGES_PER_SECOND) || 100;
Expand All @@ -36,9 +39,11 @@ export function createRateLimiter(maxPerSecond) {
timestamps = [];
windows.set(clientId, timestamps);
}
// Remove entries outside the 1-second window
while (timestamps.length > 0 && timestamps[0] <= cutoff) {
timestamps.shift();
// Batch-evict entries outside the 1-second window (O(k) where k = expired entries)
if (timestamps.length > 0 && timestamps[0] <= cutoff) {
const filtered = timestamps.filter(t => t > cutoff);
windows.set(clientId, filtered);
timestamps = filtered;
}
if (timestamps.length >= limit) return false;
timestamps.push(now);
Expand All @@ -53,5 +58,28 @@ export function createRateLimiter(maxPerSecond) {
remove(clientId) {
windows.delete(clientId);
},
};
}

/**
* Iterates all tracked clients and evicts stale entries.
* Keys with no remaining timestamps within the window are removed entirely.
* This is safe to call from a periodic timer (e.g. every 10s).
*/
cleanup() {
const cutoff = Date.now() - 1000;
for (const [clientId, timestamps] of windows) {
const filtered = timestamps.filter(t => t > cutoff);
if (filtered.length === 0) {
windows.delete(clientId);
} else {
windows.set(clientId, filtered);
}
}
},

/**
* Returns the number of distinct clients currently tracked by this limiter.
* @returns {number}
*/
get size() {
return windows.size;
},