Throttling period is not "rolling"

[rouaneta]

Hi all,

The doc (https://github.com/rack/rack-attack#throttling) seems to indicate that the throttling period is a rolling period over which one cannot do more than x requests/period, but I noticed it was more a fixed reset period for the cache.

Did I miss something? Do you know if this behaviour was intentional?

In any case, a good first step could be to clarify the doc (happy to make a suggestion).

Steps to reproduce

• setup a throttling protection: for instance, a limit of 5 requests per hour for a given ip

   Rack::Attack.throttle("requests by ip", limit: 5, period: 1.hour) do |request|
     request.ip
   end

• the following spec passes: 10 requests over 2 minutes, then the 11th fails

   RSpec.describe 'Throttling requests', type: :request do
     around(:each) do |example|
       cache_store = ActionController::Base.cache_store
       ActionController::Base.cache_store = ActiveSupport::Cache::MemoryStore.new
       ActionController::Base.perform_caching = true
       Rack::Attack.enabled = true
       Rack::Attack.reset!
   
       example.run
   
       Rack::Attack.enabled = false
       ActionController::Base.perform_caching = false
       ActionController::Base.cache_store = cache_store
     end
   
     describe 'requests by ip' do
       it 'accepts 10 requests from the same ip within a 2 minutes period, then rejects the 11th' do
         travel_to Time.zone.local(2022, 12, 12, 16, 59, 0o0)
         5.times do
           get '/'
           expect(response).to have_http_status(:found)
         end
   
         travel_to Time.zone.local(2022, 12, 12, 17, 0o0, 0o0)
         5.times do
           get '/'
           expect(response).to have_http_status(:found)
         end
   
         get '/'
         expect(response).to have_http_status(:too_many_requests)
       end
     end
   end

Would be happy to hear your thoughts :) thanks

[pcothenet]

This also took me out for a spin, as I couldn't understand why our request specs were being flaky.

According the source correctly, the logic to set the cache expiry is:

@last_epoch_time = Time.now.to_i
expires_in = (period - (@last_epoch_time % period) + 1).to_i

So I believe you're right and this is essentially a ticker, not a rolling window.
E.g. if I choose period = 60. The intervals are fixed windows of 60 seconds, regardless of when I make my first request.

This isn't necessarily an issue for our use cases (we use small windows to prevent abuse) but agree that the documentation could be more explicit.