diff --git a/.travis.yml b/.travis.yml index 46bb12fd..6b99df2c 100644 --- a/.travis.yml +++ b/.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 diff --git a/Appraisals b/Appraisals index 2cabba73..1328254c 100644 --- a/Appraisals +++ b/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" diff --git a/gemfiles/redis_4.gemfile b/gemfiles/redis_4.gemfile new file mode 100644 index 00000000..4372f718 --- /dev/null +++ b/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: "../" diff --git a/lib/rack/attack.rb b/lib/rack/attack.rb index 97937842..f59e1552 100644 --- a/lib/rack/attack.rb +++ b/lib/rack/attack.rb @@ -19,6 +19,7 @@ class MissingStoreError < StandardError; end 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' diff --git a/lib/rack/attack/store_proxy.rb b/lib/rack/attack/store_proxy.rb index c9bde979..29bef492 100644 --- a/lib/rack/attack/store_proxy.rb +++ b/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 diff --git a/lib/rack/attack/store_proxy/redis_proxy.rb b/lib/rack/attack/store_proxy/redis_proxy.rb new file mode 100644 index 00000000..69fa19d7 --- /dev/null +++ b/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 diff --git a/lib/rack/attack/store_proxy/redis_store_proxy.rb b/lib/rack/attack/store_proxy/redis_store_proxy.rb index a7ac81ae..359b542f 100644 --- a/lib/rack/attack/store_proxy/redis_store_proxy.rb +++ b/lib/rack/attack/store_proxy/redis_store_proxy.rb @@ -5,15 +5,7 @@ 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 @@ def write(key, value, options = {}) 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 diff --git a/spec/acceptance/stores/redis_spec.rb b/spec/acceptance/stores/redis_spec.rb new file mode 100644 index 00000000..26745141 --- /dev/null +++ b/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