From 074e8e5aa47ee4a5bbaede7fcf9e117967519bcd Mon Sep 17 00:00:00 2001 From: Aaron Suggs Date: Sat, 30 Aug 2014 21:30:05 -0400 Subject: [PATCH] Add 1 second buffer to expiry to correct throttles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #69. There was a race condition when `Time.now.to_i` changes between when `epoch_time` is computed in line 18, and the cache request is made (and the `key` is expired). I.e., a throttle check starts at t0, but doesn’t reach the cache until t1, the cache will have expired the throttle count. The request will likely be allowed, even if the request exceeded the limit. This has the effect of keeping keys in cache about 1 second longer than strictly necessary. But the extra cache space seems like a good trade-off for correct throttling. --- lib/rack/attack/cache.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/rack/attack/cache.rb b/lib/rack/attack/cache.rb index e65fa84..c7ef610 100644 --- a/lib/rack/attack/cache.rb +++ b/lib/rack/attack/cache.rb @@ -16,7 +16,8 @@ module Rack def count(unprefixed_key, period) epoch_time = Time.now.to_i - expires_in = period - (epoch_time % period) + # Add 1 to expires_in to avoid timing error: http://git.io/i1PHXA + expires_in = period - (epoch_time % period) + 1 key = "#{prefix}:#{(epoch_time/period).to_i}:#{unprefixed_key}" do_count(key, expires_in) end