Passing a proc as period value results in count being reset

[djpowers]

I'm experiencing some odd behavior while attempting to pass a proc as an argument to a throttle period.

I tried following the suggestion to use a proc by @ktheory in #81 (comment), as my use case is very similar.

The following code appears to work fine for me:

period_proc =  proc { (Time.now.end_of_month - Time.now).to_i }
Rack::Attack.throttle('daily per-user throttle', limit: 100_000, period: period_proc) do |request|
  # business logic
end

However, when I changed the period_proc above to use #end_of_day instead, the value is calculated properly, but each request seems to result in a new throttle being created, and the count value never surpasses 1:

period_proc = proc { (Time.now.end_of_day - Time.now).to_i }
Rack::Attack.throttle('daily per-user throttle', limit: 100_000, period: period_proc) do |request|
  # business logic
end

My understanding is that the ID showing up in the logs corresponds to a current throttle period, and a new one is created after a current throttle period expires. This ID was being incremented on each new request, even though the period had not ended

Cache increment: rack::attack:58997
Cache increment: rack::attack:59008

The ID using the #end_of_month code remained consistent, as far as I saw.

Out of curiosity, I tried using #end_of_week, and the count would reset seemingly sporadically, creating showing an incremented ID when the count went back to 1 (without hitting the limit).

It seems that the problem is actually occurring in each scenario, but when the proc yields lower values, it happens more reliably. My guess is that something is busting the cache, but I'm not sure why, or how to explain the discrepancy in behavior.

I'm using v5.4.2 of the gem on Ruby 2.2 with ActiveSupport::Cache::DalliStore.

Any guidance is very much appreciated!

[jamiemccarthy]

Hi @djpowers , I think this is a misunderstanding of what period means.

Rack::Attack's period is a time duration to break up the timeline of the unix epoch into equal-size pieces. As such, its value should not depend on the current time but on other variables.

The logic's all in key_and_expiry: https://github.com/rack/rack-attack/blob/e31488aebaef/lib/rack/attack/cache.rb#L63-L68 -- specifically where the key includes the value (@last_epoch_time / period).to_i which is the number of the "piece" the current time belongs to.

What's happening in your case is that the current unix epoch time is about 1.6 billion. Let's say it's 1.6 billion exactly. When you invoke your "daily" throttle, (Time.now.end_of_day - Time.now).to_i, if it's say 12:00 noon you're defining a period of 43100 seconds, and when you divide 1.6 billion by 43100, Rack::Attack thinks we're in "piece" number 37122 of the timeline. In other words, 37122 half-days have passed since the epoch began.

If your lambda is invoked even 1 second later, you define a period of 43099 seconds, so every piece from the beginning of the unix epoch to now is slightly smaller, the whole thing ratchets up, and 1.6 billion / 43099 puts us in piece 37123.

By the time we get near end_of_day the "piece number" accelerates very rapidly, changing in those last few seconds to 0.4, 0.8, then 1.6 billion. That's not how period is meant to work :)

You didn't notice this when using end_of_month because the piece number was changing more slowly, but the bug was the same.

Unfortunately there's no way to do this currently in Rack::Attack, because months are of varying length and whatever period value you pick you're not going to hit the beginning and end of any month precisely. If you want that behavior, you're going to have to fork the gem and add that functionality to key_and_expiry yourself.

[djpowers]

Hi @jamiemccarthy, thanks for the breakdown. It seems like the idea in #81 to add an until parameter would be the best approach if this feature were to be implemented, but since this explains the behavior I was seeing I'll go ahead and close the issue (exactly two years to the date).