Merge pull request #369 from grzuy/test_plain_redis

Acceptance test plain redis as a cache store backend

.travis.yml

@@ -26,6 +26,7 @@ gemfile:
   - gemfiles/rails_5_1.gemfile
   - gemfiles/rails_4_2.gemfile
   - gemfiles/dalli2.gemfile
+  - gemfiles/redis_4.gemfile
   - gemfiles/connection_pool_dalli.gemfile
   - gemfiles/active_support_redis_cache_store.gemfile
   - gemfiles/active_support_redis_cache_store_pooled.gemfile

Appraisals

@@ -40,6 +40,10 @@ appraise 'dalli2' do
   gem 'dalli', '~> 2.0'
 end
 
+appraise 'redis_4' do
+  gem 'redis', '~> 4.0'
+end
+
 appraise "connection_pool_dalli" do
   gem "connection_pool", "~> 2.2"
   gem "dalli", "~> 2.7"

gemfiles/redis_4.gemfile

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+# This file was generated by Appraisal
+
+source "https://rubygems.org"
+
+gem "redis", "~> 4.0"
+
+gemspec path: "../"

lib/rack/attack.rb

@@ -19,6 +19,7 @@ class Rack::Attack
   autoload :StoreProxy,           'rack/attack/store_proxy'
   autoload :DalliProxy,           'rack/attack/store_proxy/dalli_proxy'
   autoload :MemCacheProxy,        'rack/attack/store_proxy/mem_cache_proxy'
+  autoload :RedisProxy,           'rack/attack/store_proxy/redis_proxy'
   autoload :RedisStoreProxy,      'rack/attack/store_proxy/redis_store_proxy'
   autoload :RedisCacheStoreProxy, 'rack/attack/store_proxy/redis_cache_store_proxy'
   autoload :Fail2Ban,             'rack/attack/fail2ban'

lib/rack/attack/store_proxy.rb

@@ -3,7 +3,7 @@
 module Rack
   class Attack
     module StoreProxy
-      PROXIES = [DalliProxy, MemCacheProxy, RedisStoreProxy, RedisCacheStoreProxy].freeze
+      PROXIES = [DalliProxy, MemCacheProxy, RedisStoreProxy, RedisProxy, RedisCacheStoreProxy].freeze
 
       ACTIVE_SUPPORT_WRAPPER_CLASSES = Set.new(['ActiveSupport::Cache::MemCacheStore', 'ActiveSupport::Cache::RedisStore', 'ActiveSupport::Cache::RedisCacheStore']).freeze
       ACTIVE_SUPPORT_CLIENTS = Set.new(['Redis::Store', 'Dalli::Client', 'MemCache']).freeze

lib/rack/attack/store_proxy/redis_proxy.rb

@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+require 'delegate'
+
+module Rack
+  class Attack
+    module StoreProxy
+      class RedisProxy < SimpleDelegator
+        def initialize(*args)
+          if Gem::Version.new(Redis::VERSION) < Gem::Version.new("3")
+            warn 'RackAttack requires Redis gem >= 3.0.0.'
+          end
+
+          super(*args)
+        end
+
+        def self.handle?(store)
+          defined?(::Redis) && store.is_a?(::Redis)
+        end
+
+        def read(key)
+          get(key)
+        rescue Redis::BaseError
+        end
+
+        def write(key, value, options = {})
+          if (expires_in = options[:expires_in])
+            setex(key, expires_in, value)
+          else
+            set(key, value)
+          end
+        rescue Redis::BaseError
+        end
+
+        def increment(key, amount, options = {})
+          count = nil
+
+          pipelined do
+            count = incrby(key, amount)
+            expire(key, options[:expires_in]) if options[:expires_in]
+          end
+
+          count.value if count
+        rescue Redis::BaseError
+        end
+
+        def delete(key, _options = {})
+          del(key)
+        rescue Redis::BaseError
+        end
+      end
+    end
+  end
+end

lib/rack/attack/store_proxy/redis_store_proxy.rb

@@ -5,15 +5,7 @@ require 'delegate'
 module Rack
   class Attack
     module StoreProxy
-      class RedisStoreProxy < SimpleDelegator
-        def initialize(*args)
-          if Gem::Version.new(Redis::VERSION) < Gem::Version.new("3")
-            warn 'RackAttack requires Redis gem >= 3.0.0.'
-          end
-
-          super(*args)
-        end
-
+      class RedisStoreProxy < RedisProxy
         def self.handle?(store)
           defined?(::Redis::Store) && store.is_a?(::Redis::Store)
         end
@@ -31,23 +23,6 @@ module Rack
           end
         rescue Redis::BaseError
         end
-
-        def increment(key, amount, options = {})
-          count = nil
-
-          pipelined do
-            count = incrby(key, amount)
-            expire(key, options[:expires_in]) if options[:expires_in]
-          end
-
-          count.value if count
-        rescue Redis::BaseError
-        end
-
-        def delete(key, _options = {})
-          del(key)
-        rescue Redis::BaseError
-        end
       end
     end
   end

spec/acceptance/stores/redis_spec.rb

@@ -0,0 +1,42 @@
+# frozen_string_literal: true
+
+require_relative "../../spec_helper"
+
+if defined?(::Redis)
+  require_relative "../../support/cache_store_helper"
+  require "timecop"
+
+  describe "Plain redis as a cache backend" do
+    before do
+      Rack::Attack.cache.store = Redis.new
+    end
+
+    after do
+      Rack::Attack.cache.store.flushdb
+    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.get(key)
+
+      sleep 2.1
+
+      assert_nil Rack::Attack.cache.store.get(key)
+    end
+  end
+end