Skip to content

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