Skip to content

#1305 Populate RateLimiting headers in the original HttpContext response accessed via IHttpContextAccessor #1307

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

Merged
merged 13 commits into from
Nov 9, 2024

Conversation

jlukawska
Copy link
Contributor

@jlukawska jlukawska commented Jul 31, 2020

Fixes #1305

Proposed Changes

  • It doesn't work because the headers are set on the newly created HttpContext and not the original one that is returned with a response. The fix is that the rate limiting headers are set for the original HttpContext returned by HttpContextAccessor.

@jackletter
Copy link

finnaly HttpResponseFeature.OnStarting was invoked, but HttpResponseFeature.OnStarting method body has nothing

@garybond
Copy link

garybond commented Apr 8, 2022

+1 to get this merged. This is stopping me being able to update a project to net6.0 + Ocelot v18

@jackletter

This comment was marked as off-topic.

@Versa78
Copy link

Versa78 commented Apr 21, 2022

+1 We need this fix as well

@raman-m raman-m changed the title Fix/1305 ratelimiting headers #1305 RateLimiting headers Jul 18, 2023
@raman-m raman-m changed the base branch from master to develop July 18, 2023 11:13
@raman-m raman-m self-requested a review July 18, 2023 11:21
@raman-m raman-m force-pushed the fix/1305-ratelimiting-headers branch from 72da5b1 to 1060ef9 Compare August 2, 2023 13:33
@raman-m
Copy link
Member

raman-m commented Aug 2, 2023

The feature branch has been rebased onto ThreeMammals:develop!

We can start the code review...

@jackletter

This comment was marked as off-topic.

@raman-m
Copy link
Member

raman-m commented Aug 2, 2023

@jackletter commented on Oct 24, 2020:

finnaly HttpResponseFeature.OnStarting was invoked, but HttpResponseFeature.OnStarting method body has nothing

Is this a result of code review (testing)?
If Not, could you review the code please?


P.S. Don't write non-English messages please!

@raman-m
Copy link
Member

raman-m commented Aug 2, 2023

@garybond @Versa78 @neetra @nls44
Welcome to the code review!
Your opinion is highly required.

@raman-m raman-m added bug Identified as a potential bug proposal Proposal for a new functionality in Ocelot needs feedback Issue is waiting on feedback before acceptance labels Aug 2, 2023
@raman-m raman-m force-pushed the fix/1305-ratelimiting-headers branch from 53fbcd2 to eaff1d9 Compare November 8, 2024 13:27
@raman-m raman-m requested review from raman-m and ggnaegi November 8, 2024 16:09
Copy link
Member

@raman-m raman-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ready for Delivery ✅

@raman-m raman-m changed the title #1305 RateLimiting headers #1305 Populate RateLimiting headers in the original HttpContext response using IHttpContextAccessor Nov 8, 2024
@raman-m raman-m changed the title #1305 Populate RateLimiting headers in the original HttpContext response using IHttpContextAccessor #1305 Populate RateLimiting headers in the original HttpContext response accessed via IHttpContextAccessor. Nov 8, 2024
@raman-m raman-m changed the title #1305 Populate RateLimiting headers in the original HttpContext response accessed via IHttpContextAccessor. #1305 Populate RateLimiting headers in the original HttpContext response accessed via IHttpContextAccessor Nov 8, 2024
@raman-m
Copy link
Member

raman-m commented Nov 8, 2024

@garybond commented on Apr 8, 2022

Hang in there, Gary!
Your project's leap to the elite Ocelot stack is just around the corner: net8.0 vs Ocelot v24.0 is going to be epic! 😄
Apologies for the two-and-a-half-year suspense – good things come to those who wait, right? 😉

@raman-m raman-m dismissed ggnaegi’s stale review November 9, 2024 17:04

Dev Complete and reviewer is offline

@raman-m raman-m merged commit da9d6fa into ThreeMammals:develop Nov 9, 2024
1 check passed
@raman-m raman-m deleted the fix/1305-ratelimiting-headers branch November 9, 2024 17:10
yjorayev pushed a commit to yjorayev/Ocelot that referenced this pull request Nov 13, 2024
…Context` response accessed via `IHttpContextAccessor` (ThreeMammals#1307)

* set rate limiting headers on the proper httpcontext

* fix Retry-After header

* merge fix

* refactor of ClientRateLimitTests

* merge fix

* Fix build after rebasing

* EOL: test/Ocelot.AcceptanceTests/Steps.cs

* Add `RateLimitingSteps`

* code review by @raman-m

* Inject IHttpContextAccessor, not IServiceProvider

* Ocelot's rate-limiting headers have become legacy

* Headers definition life hack

* A good StackOverflow link

---------

Co-authored-by: Jolanta Łukawska <jolanta.lukawska@outlook.com>
Co-authored-by: Raman Maksimchuk <dotnet044@gmail.com>
@raman-m raman-m added Nov'24 November 2024 release and removed Summer'25 Summer 2025 release labels Nov 20, 2024
@raman-m raman-m modified the milestones: Autumn'24, November'24 Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Identified as a potential bug Nov'24 November 2024 release proposal Proposal for a new functionality in Ocelot Rate Limiting Ocelot feature: Rate Limiting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"DisableRateLimitHeaders": false is not showing X-Rate-Limit and Retry-After headers in response
9 participants