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

The Rate Limiting feature doesn't work when use DistributedCache #609

Closed
rocgao opened this issue Sep 11, 2018 · 5 comments · Fixed by #610
Closed

The Rate Limiting feature doesn't work when use DistributedCache #609

rocgao opened this issue Sep 11, 2018 · 5 comments · Fixed by #610
Labels
bug Identified as a potential bug merged Issue has been merged to dev and is waiting for the next release

Comments

@rocgao
Copy link
Contributor

rocgao commented Sep 11, 2018

Expected Behavior

External http request should be blocked when quota exceeded.

Actual Behavior

External http request was forwarded to downstream servers always.

Steps to Reproduce the Problem

  1. Adding DistributedCache. Such as RedisCache, which is supplied by Microsoft.Extensions.Caching.Redis nuget package.
  2. Enabling DistributedCache. Refer to following code:
       // Startup.cs
       public void ConfigureServices(IServiceCollection services)
        {
            //使用基于Redis缓存的RateLimiting
            services.Configure<RedisCacheOptions>(Configuration.GetSection("RedisCacheOptions"))
                .AddSingleton<IDistributedCache,RedisCache>()
                .AddSingleton<IRateLimitCounterHandler,DistributedCacheRateLimitCounterHanlder>();

            services.AddOcelot();
        }
  1. Configuring RateLimiting options in Ocelot.json .
      "ReRoutes": [
        {
            "DownstreamPathTemplate": "/health",
            "DownstreamScheme": "http",
            "DownstreamHostAndPorts": [
                {
                    "Host": "localhost",
                    "Port": 6001
                }
            ],
            "UpstreamPathTemplate": "/ocelot/health",
            "RateLimitOptions": {
                "ClientWhitelist": [],
                "EnableRateLimiting": true,
                "Period": "20s",
                "PeriodTimespan": 5,
                "Limit": 1
            }
        }
  1. Sent http request to Ocelot server, it will always response following content. Attention to the X-Rate-Limit-Reset header.
HTTP/1.1 200 OK
Date: Tue, 11 Sep 2018 02:53:36 GMT
Content-Type: application/json
Server: Kestrel
Content-Length: 18
X-Rate-Limit-Limit: 20s
X-Rate-Limit-Remaining: 1
X-Rate-Limit-Reset: 0001-01-01T00:00:00.0000000Z

Specifications

  • Version: 10.*
  • Platform: MacOS
  • Subsystem:
@rocgao
Copy link
Contributor Author

rocgao commented Sep 11, 2018

I have found out the bug. The DistributedCacheRateLimitCounterHanlder.Get method will be called, when the RedisCache is enabled. Within the method, call JsonConvert.DeserializeObject funtion to load RateLimitCounter instance from json string. But both Timestamp and TotalRequests properties are readonly of the RateLimitCounter struct. So to fix this bug, should apply the JsonConstructorAttribute to RateLimitCounter's constructor and rename totalRequest parameter to totalRequests.

       [JsonConstructor]
        public RateLimitCounter(DateTime timestamp, long totalRequests)
        {
            Timestamp = timestamp;
            TotalRequests = totalRequests;
        }

rocgao pushed a commit to rocgao/Ocelot that referenced this issue Sep 11, 2018
@TomPallister TomPallister added bug Identified as a potential bug merged Issue has been merged to dev and is waiting for the next release labels Sep 12, 2018
@TomPallister
Copy link
Member

@rocgao thanks for this issue and the fix. I really appreciate it. I will merge this into NuGet package ASAP.

@TomPallister TomPallister reopened this Sep 12, 2018
@TomPallister
Copy link
Member

fixed in 11.0.3, any problems let me know

@geffzhang
Copy link
Contributor

@rocgao add me with wechat geffzhang

ematlock0418 pushed a commit to ematlock0418/Ocelot that referenced this issue Mar 21, 2021
KatherineGirona added a commit to KatherineGirona/Ocelot that referenced this issue Oct 11, 2022
Olaf1022 pushed a commit to Olaf1022/ocelot that referenced this issue Aug 15, 2023
@raman-m
Copy link
Member

raman-m commented Feb 22, 2024

@rocgao @geffzhang
Are you still with Ocelot?

Regarding the user scenario with RedisCache...
How did/does this setup work in Production env?
Are you satisfied?
Can we develop this setup of services to a real Ocelot feature?
We could develop at least new Ocelot DI-extension method for the first time.

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 merged Issue has been merged to dev and is waiting for the next release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants