Merge pull request #354 from grzuy/redis_cache_store_pooled

Pooled RedisCacheStore as a backend store

lib/rack/attack/store_proxy/redis_cache_store_proxy.rb

@@ -8,13 +8,18 @@ module Rack
           defined?(::ActiveSupport::Cache::RedisCacheStore) && store.is_a?(::ActiveSupport::Cache::RedisCacheStore)
         end
 
-        def increment(name, amount, options = {})
-          # Redis doesn't check expiration on the INCRBY command. See https://redis.io/commands/expire
-          count = redis.pipelined do
-            redis.incrby(name, amount)
-            redis.expire(name, options[:expires_in]) if options[:expires_in]
+        def increment(name, amount = 1, options = {})
+          # 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, amount, options)
+
+            1
+          else
+            super
           end
-          count.first
         end
 
         def read(name, options = {})

spec/acceptance/stores/redis_cache_store_pooled_spec.rb

@@ -0,0 +1,40 @@
+require_relative "../../spec_helper"
+require_relative "../../support/cache_store_helper"
+
+require "timecop"
+
+if ActiveSupport.version >= Gem::Version.new("5.2.0")
+  describe "RedisCacheStore (pooled) as a cache backend" do
+    before do
+      Rack::Attack.cache.store = ActiveSupport::Cache::RedisCacheStore.new(pool_size: 2)
+    end
+
+    after do
+      Rack::Attack.cache.store.clear
+    end
+
+    it_works_for_cache_backed_features
+
+    it "doesn't leak keys" do
+      Rack::Attack.throttle("by ip", limit: 1, period: 1) do |request|
+        request.ip
+      end
+
+      key = nil
+
+      # Freeze time during these statement to be sure that the key used by rack attack is the same
+      # we pre-calculate in local variable `key`
+      Timecop.freeze do
+        key = "rack::attack:#{Time.now.to_i}:by ip:1.2.3.4"
+
+        get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
+      end
+
+      assert Rack::Attack.cache.store.fetch(key)
+
+      sleep 2.1
+
+      assert_nil Rack::Attack.cache.store.fetch(key)
+    end
+  end
+end