Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cors): Expose rate-limit headers in API responses #3237

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

IsAvaible
Copy link

This PR addresses the issue that #3189 attempted to fix but did not fully resolve. While #3189 added Access-Control-Allow-Origin, it did not set Access-Control-Expose-Headers, which is required to make the rate-limiting headers accessible in browser-based applications.

Issue:

As reported in #2529, API responses do not expose critical rate-limit headers (X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset). This makes it impossible for client-side applications to track rate limits due to CORS restrictions.

Fix:

This PR explicitly sets the Access-Control-Expose-Headers header to expose the required rate-limit headers, allowing web applications to access them properly.

Changes:

Added Access-Control-Expose-Headers with X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset
Ensures CORS policies allow browser clients to read these headers
This should fully resolve the issue and allow web-based API clients to track rate limits correctly.

Let me know if any further adjustments are needed!

Added `Access-Control-Expose-Headers` to ensure that the rate-limiting headers  
(`X-RateLimit-Limit`, `X-RateLimit-Remaining`, `X-RateLimit-Reset`)  
are accessible in browser-based applications.  

This resolves the issue where clients could not track rate limits due to  
CORS restrictions, improving API usability for web-based applications.  

Signed-off-by: Simon Felix Conrad <[email protected]>
@Erb3
Copy link
Contributor

Erb3 commented Feb 9, 2025

I'm not too familiar with CORS, as you can tell. It hits me now though, wouldn't we also have to set AccessControlAllowMethods?

@IsAvaible
Copy link
Author

IsAvaible commented Feb 13, 2025

I'm not too familiar with CORS, as you can tell. It hits me now though, wouldn't we also have to set AccessControlAllowMethods?

GET, HEAD, and POST are always allowed, regardless of whether they are specified in this header, as they are defined as CORS-safelisted methods.
See mdn

I don't think setting AccessControlAllowMethods here is necessary as GET, HEAD, and POST are always allowed and the changed code only sets the CORS response when a rate limit occurs (the standard CORS headers are set in a different part of the codebase).

I think the headers we set here only apply to the ratelimited response. The request that triggered the ratelimit uses the correct standard CORS headers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants