FIX: sliding window end time in rate limiter (#11691)

If the sliding window size is N seconds, then a moment at the Nth second
should be considered as the moment outside of the sliding window.

Otherwise, if the sliding window is already full, at the Nth second,
a new call wouldn't be allowed, but a time to wait before the next call
would be equal to zero, which is confusing.

In other words, the end of the time range shouldn't be included in the
sliding window.

Let's say we start at the second 0, and the sliding window size is 10
seconds. In the current version of rate limiter, this sliding window will
be considered as a time range [0, 10] (including the end of the range),
which actually is 11 seconds in length.

After this fix, the time range will be considered as [0, 10)
(excluding the end of the range), which is exactly 10 seconds in length.

lib/rate_limiter.rb

@@ -66,7 +66,7 @@ class RateLimiter
 
 
       if ((tonumber(redis.call("LLEN", key)) < max) or
-          (now - tonumber(redis.call("LRANGE", key, -1, -1)[1])) > secs) then
+          (now - tonumber(redis.call("LRANGE", key, -1, -1)[1])) >= secs) then
         redis.call("LPUSH", key, now)
         redis.call("LTRIM", key, 0, max - 1)
         redis.call("EXPIRE", key, secs * 2)
@@ -91,7 +91,7 @@ class RateLimiter
       local return_val = 0
 
       if ((tonumber(redis.call("LLEN", key)) < max) or
-          (now - tonumber(redis.call("LRANGE", key, -1, -1)[1])) > secs) then
+          (now - tonumber(redis.call("LRANGE", key, -1, -1)[1])) >= secs) then
         return_val = 1
       else
         return_val = 0
@@ -185,8 +185,8 @@ class RateLimiter
   def is_under_limit?
     # number of events in buffer less than max allowed? OR
     (redis.llen(prefixed_key) < @max) ||
-    # age bigger than silding window size?
-    (age_of_oldest > @secs)
+    # age bigger or equal than sliding window size?
+    (age_of_oldest >= @secs)
   end
 
   def rate_unlimited?

spec/components/rate_limiter_spec.rb

@@ -72,10 +72,10 @@ describe RateLimiter do
           limiter.performed!
         end.to raise_error(RateLimiter::LimitExceeded)
 
-        freeze_time 31.seconds.from_now
+        freeze_time 30.seconds.from_now
 
-        limiter.performed!
-        limiter.performed!
+        expect { limiter.performed! }.not_to raise_error
+        expect { limiter.performed! }.not_to raise_error
 
       end
     end
@@ -150,6 +150,7 @@ describe RateLimiter do
 
     context "multiple calls" do
       before do
+        freeze_time
         rate_limiter.performed!
         rate_limiter.performed!
       end
@@ -160,7 +161,15 @@ describe RateLimiter do
       end
 
       it "raises an error the third time called" do
-        expect { rate_limiter.performed! }.to raise_error(RateLimiter::LimitExceeded)
+        expect { rate_limiter.performed! }.to raise_error do |error|
+          expect(error).to be_a(RateLimiter::LimitExceeded)
+          expect(error).to having_attributes(available_in: 60)
+        end
+      end
+
+      it 'raises no error when the sliding window ended' do
+        freeze_time 60.seconds.from_now
+        expect { rate_limiter.performed! }.not_to raise_error
       end
 
       context "as an admin/moderator" do