fix(rate-limit): add Retry-After header to 429 responses and fix TOCTOU race in Redis rate limiter#6156
Closed
Aamod-Dev wants to merge 1 commit into
Closed
Conversation
…ng() key prefix - Add Retry-After header to getRateLimitHeaders() for RFC-compliant 429 responses - Fix middleware.ts to use getRateLimitHeaders() instead of manual headers - Fix TOCTOU race in RateLimiter.checkWithResult() Redis path (replaced GET+TTL+INCR with single atomic INCR+EXPIRE pipeline) - Fix remaining() to use correct key prefix (ratelimit:) and query Redis when available Related to JhaSourav07#5857
Contributor
|
@Aamod-Dev is attempting to deploy a commit to the jhasourav07's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
👋 Hey @Aamod-Dev! Thanks for your interest in contributing to CommitPulse! 🙏 Unfortunately, this PR has been automatically closed because you are not assigned to the linked issue #6149 — fix(rate-limit): add Retry-After header to 429 responses and fix TOCTOU race in Redis rate limiter. To avoid this in the future, please follow these steps:
We look forward to your contribution once you're assigned! 🚀 |
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.
Summary
Fixes the rate-limiting issues documented in #6149. Addresses the missing \Retry-After\ header (as originally requested in #5857) and fixes two bugs in the Redis rate-limit path.
Changes
1. Added \Retry-After\ header to all 429 responses
Files: \lib/rate-limit.ts, \middleware.ts\
The \getRateLimitHeaders()\ helper now includes the standard \Retry-After\ HTTP header in addition to the existing \X-RateLimit-*\ headers. The middleware now uses this helper instead of manually constructing headers.
Before: 429 responses had \X-RateLimit-Limit, \X-RateLimit-Remaining, \X-RateLimit-Reset\ but no \Retry-After.
After: 429 responses include:
\
Retry-After: 42
X-RateLimit-Limit: 60
X-RateLimit-Remaining: 0
X-RateLimit-Reset: 1712345678000
\\
2. Fixed TOCTOU race condition in Redis rate-limit path
File: \lib/rate-limit.ts:72-121\
The \RateLimiter.checkWithResult()\ method previously used a non-atomic read-then-write pattern:
Under concurrent load, multiple requests could all read the same sub-limit count and all pass through. Replaced with a single atomic \INCR\ + \EXPIRE\ pipeline that matches the approach used in the
ateLimit()\ helper function.
3. Fixed \
emaining()\ method for Redis-backed deployments
File: \lib/rate-limit.ts:195-198\
The
emaining()\ method was reading from local cache key
atelimit:\ while the Redis path stored under
atelimit_class:\. When Redis was active, this always returned the full limit. Now queries Redis directly when available using the correct key prefix.
Related Issues
Closes: #6149
Related: #5857