Fix Ratelimit every day, every minute, every hour logic bugs

src/Ocelot/Configuration/Validator/FileConfigurationValidator.cs

@@ -28,6 +28,12 @@ namespace Ocelot.Configuration.Validator
 
             result = CheckForReRoutesContainingDownstreamSchemeInDownstreamPathTemplate(configuration);
 
+            if (result.IsError)
+            {
+                return new OkResponse<ConfigurationValidationResult>(result);
+            }
+            result = CheckForReRoutesRateLimitOptions(configuration);
+
             if (result.IsError)
             {
                 return new OkResponse<ConfigurationValidationResult>(result);
@@ -111,5 +117,35 @@ namespace Ocelot.Configuration.Validator
 
             return new ConfigurationValidationResult(true, errors);
         }
+
+        private ConfigurationValidationResult CheckForReRoutesRateLimitOptions(FileConfiguration configuration)
+        {
+            var errors = new List<Error>();
+
+            foreach (var reRoute in configuration.ReRoutes)
+            {
+                if (reRoute.RateLimitOptions.EnableRateLimiting)
+                {
+                    if (!IsValidPeriod(reRoute))
+                    {
+                        errors.Add(new RateLimitOptionsValidationError($"{reRoute.RateLimitOptions.Period} not contains scheme"));
+                    }
+                }
+            }
+
+            if (errors.Any())
+            {
+                return new ConfigurationValidationResult(true, errors);
+            }
+
+            return new ConfigurationValidationResult(false, errors);
+        }
+
+        private static bool IsValidPeriod(FileReRoute reRoute)
+        {
+            string period = reRoute.RateLimitOptions.Period;
+
+            return period.Contains("s") || period.Contains("m") || period.Contains("h") || period.Contains("d");
+        }
     }
 }

src/Ocelot/Configuration/Validator/RateLimitOptionsValidationError.cs

@@ -0,0 +1,15 @@
+using Ocelot.Errors;
+using System;
+using System.Collections.Generic;
+using System.Text;
+
+namespace Ocelot.Configuration.Validator
+{
+    public class RateLimitOptionsValidationError : Error
+    {
+        public RateLimitOptionsValidationError(string message) 
+            : base(message, OcelotErrorCode.RateLimitOptionsError)
+        {
+        }
+    }
+}

src/Ocelot/Errors/OcelotErrorCode.cs

@@ -29,6 +29,7 @@
         RequestTimedOutError,
         UnableToFindQoSProviderError,
         UnableToSetConfigInConsulError,
-        UnmappableRequestError
+        UnmappableRequestError,
+        RateLimitOptionsError
     }
 }

src/Ocelot/RateLimit/ClientRateLimitProcessor.cs

@@ -7,6 +7,7 @@ using System.Threading.Tasks;
 
 namespace Ocelot.RateLimit
 {
+
     public class ClientRateLimitProcessor
     {
         private readonly IRateLimitCounterHandler _counterHandler;
@@ -23,7 +24,8 @@ namespace Ocelot.RateLimit
             return _core.ProcessRequest(requestIdentity, option);
         }
 
-        public string RetryAfterFrom(DateTime timestamp, RateLimitRule rule)
+
+        public int RetryAfterFrom(DateTime timestamp, RateLimitRule rule)
         {
             return _core.RetryAfterFrom(timestamp, rule);
         }
@@ -33,5 +35,11 @@ namespace Ocelot.RateLimit
             return _core.GetRateLimitHeaders(context, requestIdentity, option);
         }
 
+        public TimeSpan ConvertToTimeSpan(string timeSpan)
+        {
+            return _core.ConvertToTimeSpan(timeSpan);
+        }
+
     }
+
 }

src/Ocelot/RateLimit/Middleware/ClientRateLimitMiddleware.cs

@@ -40,7 +40,7 @@ namespace Ocelot.RateLimit.Middleware
                 _logger.LogDebug($"EndpointRateLimiting is not enabled for {DownstreamRoute.ReRoute.DownstreamPathTemplate}");
 
                 _logger.TraceInvokeNext();
-                    await _next.Invoke(context);
+                await _next.Invoke(context);
                 _logger.TraceInvokeNextCompleted();
                 _logger.TraceMiddlewareCompleted();
                 return;
@@ -54,7 +54,7 @@ namespace Ocelot.RateLimit.Middleware
                 _logger.LogDebug($"{DownstreamRoute.ReRoute.DownstreamPathTemplate} is white listed from rate limiting");
 
                 _logger.TraceInvokeNext();
-                    await _next.Invoke(context);
+                await _next.Invoke(context);
                 _logger.TraceInvokeNextCompleted();
                 _logger.TraceMiddlewareCompleted();
                 return;
@@ -75,21 +75,24 @@ namespace Ocelot.RateLimit.Middleware
                     // log blocked request
                     LogBlockedRequest(context, identity, counter, rule);
 
+                    var retrystring = retryAfter.ToString(System.Globalization.CultureInfo.InvariantCulture);
                     // break execution
-                    await ReturnQuotaExceededResponse(context, options, retryAfter);
+                    await ReturnQuotaExceededResponse(context, options, retrystring);
                     _logger.TraceMiddlewareCompleted();
+
                     return;
                 }
+
             }
             //set X-Rate-Limit headers for the longest period
             if (!options.DisableRateLimitHeaders)
             {
-                var headers = _processor.GetRateLimitHeaders( context,identity, options);
+                var headers = _processor.GetRateLimitHeaders(context, identity, options);
                 context.Response.OnStarting(SetRateLimitHeaders, state: headers);
             }
 
             _logger.TraceInvokeNext();
-                await _next.Invoke(context);
+            await _next.Invoke(context);
             _logger.TraceInvokeNextCompleted();
             _logger.TraceMiddlewareCompleted();
         }
@@ -107,11 +110,15 @@ namespace Ocelot.RateLimit.Middleware
                 httpContext.Request.Path.ToString().ToLowerInvariant(),
                 httpContext.Request.Method.ToLowerInvariant()
                 );
-         }
+        }
 
         public bool IsWhitelisted(ClientRequestIdentity requestIdentity, RateLimitOptions option)
         {
-            return option.ClientWhitelist.Contains(requestIdentity.ClientId);
+            if (option.ClientWhitelist.Contains(requestIdentity.ClientId))
+            {
+                return true;
+            }
+            return false;
         }
 
         public virtual void LogBlockedRequest(HttpContext httpContext, ClientRequestIdentity identity, RateLimitCounter counter, RateLimitRule rule)
@@ -144,6 +151,6 @@ namespace Ocelot.RateLimit.Middleware
         }
 
     }
+
 }
 
-

src/Ocelot/RateLimit/RateLimitCore.cs

@@ -34,23 +34,45 @@ namespace Ocelot.RateLimit
                 if (entry.HasValue)
                 {
                     // entry has not expired
-                    if (entry.Value.Timestamp + TimeSpan.FromSeconds(rule.PeriodTimespan) >= DateTime.UtcNow)
+                    if (entry.Value.Timestamp + ConvertToTimeSpan(rule.Period) >= DateTime.UtcNow)
                     {
                         // increment request count
                         var totalRequests = entry.Value.TotalRequests + 1;
-
                         // deep copy
                         counter = new RateLimitCounter(entry.Value.Timestamp, totalRequests);
-                        
                     }
                 }
-                // stores: id (string) - timestamp (datetime) - total_requests (long)
-                _counterHandler.Set(counterId, counter, TimeSpan.FromSeconds(rule.PeriodTimespan));
             }
 
+            if (counter.TotalRequests > rule.Limit)
+            {
+                var retryAfter = RetryAfterFrom(counter.Timestamp, rule);
+                if (retryAfter > 0)
+                {
+                    var expirationTime = TimeSpan.FromSeconds(rule.PeriodTimespan);
+                    _counterHandler.Set(counterId, counter, expirationTime);
+                }
+                else
+                {
+                    _counterHandler.Remove(counterId);
+                }
+            }
+            else
+            {
+                var expirationTime = ConvertToTimeSpan(rule.Period);
+                _counterHandler.Set(counterId, counter, expirationTime);
+            }
             return counter;
         }
 
+        public void SaveRateLimitCounter(ClientRequestIdentity requestIdentity, RateLimitOptions option, RateLimitCounter counter, TimeSpan expirationTime)
+        {
+            var counterId = ComputeCounterKey(requestIdentity, option);
+            var rule = option.RateLimitRule;
+            // stores: id (string) - timestamp (datetime) - total_requests (long)
+            _counterHandler.Set(counterId, counter, expirationTime);
+        }
+
         public RateLimitHeaders GetRateLimitHeaders(HttpContext context, ClientRequestIdentity requestIdentity, RateLimitOptions option)
         {
             var rule = option.RateLimitRule;
@@ -63,7 +85,7 @@ namespace Ocelot.RateLimit
                     (rule.Limit - entry.Value.TotalRequests).ToString(),
                     (entry.Value.Timestamp + ConvertToTimeSpan(rule.Period)).ToUniversalTime().ToString("o", DateTimeFormatInfo.InvariantInfo)
                     );
-             }
+            }
             else
             {
                 headers = new RateLimitHeaders(context,
@@ -74,17 +96,17 @@ namespace Ocelot.RateLimit
             }
 
             return headers;
-         }
+        }
 
         public string ComputeCounterKey(ClientRequestIdentity requestIdentity, RateLimitOptions option)
         {
-            var key =  $"{option.RateLimitCounterPrefix}_{requestIdentity.ClientId}_{option.RateLimitRule.Period}_{requestIdentity.HttpVerb}_{requestIdentity.Path}";
+            var key = $"{option.RateLimitCounterPrefix}_{requestIdentity.ClientId}_{option.RateLimitRule.Period}_{requestIdentity.HttpVerb}_{requestIdentity.Path}";
 
-            var idBytes =  Encoding.UTF8.GetBytes(key);
+            var idBytes = Encoding.UTF8.GetBytes(key);
 
             byte[] hashBytes;
 
-            using (var algorithm =  SHA1.Create())
+            using (var algorithm = SHA1.Create())
             {
                 hashBytes = algorithm.ComputeHash(idBytes);
             }
@@ -92,12 +114,12 @@ namespace Ocelot.RateLimit
             return BitConverter.ToString(hashBytes).Replace("-", string.Empty);
         }
 
-        public string RetryAfterFrom(DateTime timestamp, RateLimitRule rule)
+        public int RetryAfterFrom(DateTime timestamp, RateLimitRule rule)
         {
             var secondsPast = Convert.ToInt32((DateTime.UtcNow - timestamp).TotalSeconds);
             var retryAfter = Convert.ToInt32(TimeSpan.FromSeconds(rule.PeriodTimespan).TotalSeconds);
             retryAfter = retryAfter > 1 ? retryAfter - secondsPast : 1;
-            return retryAfter.ToString(System.Globalization.CultureInfo.InvariantCulture);
+            return retryAfter;
         }
 
         public TimeSpan ConvertToTimeSpan(string timeSpan)