The Rate Limiting feature doesn't work when use DistributedCache

rocgao commented 6 years ago

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

Adding DistributedCache. Such as RedisCache, which is supplied by Microsoft.Extensions.Caching.Redis nuget package.

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();
    }

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
        }
    }

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 commented 6 years ago

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;
        }

TomPallister commented 6 years ago

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

TomPallister commented 6 years ago

fixed in 11.0.3, any problems let me know

geffzhang commented 6 years ago

@rocgao add me with wechat geffzhang

raman-m commented 8 months ago

@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.

ThreeMammals / Ocelot