Chris Ross
Chris Ross
It's interesting that we avoided this issue in ResponseCompression by not implementing Deflate. https://github.com/dotnet/aspnetcore/blob/83d6c56ab616709ec66381def032ffca0065c0ee/src/Middleware/ResponseCompression/src/ResponseCompressionProvider.cs#L41-L45 HttpClient seems to support both formats by checking the first byte. We could do the same...
> The app is containerized and orchestrated using Docker compose. I think this explains the difference. If you try environment variables vs code outside of the container they should work...
> There are scenarios where we know KestrelServerOptions.ConfigurationLoader is not set though. Such as?
What's the full exception stack trace? We need to understand which code path is making these requests. @jmprieur
@jennyf19 these are all Identity.Web callstacks, can you take a look?
Risk - would anyone use this to dispose of the lease early? Would that have any strange side-effects, especially if it got double-disposed by the middleware later?
IRateLimiterContextFeature - Consider making all of the properties read only. There's no scenario for replacing them, correct?
We'll review the API in a week or two when people are back from the holidays.
Having HttpContext on a Feature interface is weirdly circular, normally you're retrieving features from the HttpContext. Why would you need to store the context?
> ```diff > + namespace Microsoft.AspNetCore.RateLimiting.Features; > ``` @halter73 why did we decide to put this in the Features namespace? That's not something we've done for other components like https://github.com/dotnet/aspnetcore/blob/334da01db159058defbd39f128d659ddf3ae3f7e/src/Middleware/OutputCaching/src/IOutputCacheFeature.cs#L4-L9...