From 3af7394b6ac6ae97e2cd63eda18daa92a29d0e6b Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Wed, 20 Jun 2018 18:32:55 -0300 Subject: [PATCH] Refactor RedisCacheStoreProxy to unlearn everything about redis client details to make it less prone to bugs in the future Let RedisCacheStoreProxy only know and assume things about RedisCacheStore API. Don't let it know anything about the specific redis client behind the scenes, that's the job of RedisCacheStore only, not ours. --- .../store_proxy/redis_cache_store_proxy.rb | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/rack/attack/store_proxy/redis_cache_store_proxy.rb b/lib/rack/attack/store_proxy/redis_cache_store_proxy.rb index dcf085f..2b506f3 100644 --- a/lib/rack/attack/store_proxy/redis_cache_store_proxy.rb +++ b/lib/rack/attack/store_proxy/redis_cache_store_proxy.rb @@ -9,13 +9,16 @@ module Rack end def increment(name, amount, options = {}) - # Redis doesn't check expiration on the INCRBY command. See https://redis.io/commands/expire - redis.with do |r| - count = r.pipelined do - r.incrby(name, amount) - r.expire(name, options[:expires_in]) if options[:expires_in] - end - count.first + # RedisCacheStore#increment ignores options[:expires_in]. + # + # So in order to workaround this we use RedisCacheStore#write (which sets expiration) to initialize + # the counter. After that we continue using the original RedisCacheStore#increment. + if options[:expires_in] && !read(name) + write(name, 1, options) + + 1 + else + super end end