diff --git a/lib/rack/attack/cache.rb b/lib/rack/attack/cache.rb index 9111ab8a..31c5de6c 100644 --- a/lib/rack/attack/cache.rb +++ b/lib/rack/attack/cache.rb @@ -26,6 +26,9 @@ def store=(store) else store end + if @store + check_store_methods_presence(:read, :write, :delete, :increment) + end end def count(unprefixed_key, period) @@ -34,13 +37,14 @@ def count(unprefixed_key, period) end def read(unprefixed_key) - enforce_store_presence! - enforce_store_method_presence!(:read) + raise Rack::Attack::MissingStoreError if store.nil? store.read("#{prefix}:#{unprefixed_key}") end def write(unprefixed_key, value, expires_in) + raise Rack::Attack::MissingStoreError if store.nil? + store.write("#{prefix}:#{unprefixed_key}", value, expires_in: expires_in) end @@ -74,32 +78,22 @@ def key_and_expiry(unprefixed_key, period) end def do_count(key, expires_in) - enforce_store_presence! - enforce_store_method_presence!(:increment) + raise Rack::Attack::MissingStoreError if store.nil? result = store.increment(key, 1, expires_in: expires_in) # NB: Some stores return nil when incrementing uninitialized values if result.nil? - enforce_store_method_presence!(:write) - store.write(key, 1, expires_in: expires_in) end result || 1 end - def enforce_store_presence! - if store.nil? - raise Rack::Attack::MissingStoreError - end - end - - def enforce_store_method_presence!(method_name) - if !store.respond_to?(method_name) - raise( - Rack::Attack::MisconfiguredStoreError, - "Configured store #{store.class.name} doesn't respond to ##{method_name} method" - ) + def check_store_methods_presence(*method_names) + missing = method_names.reject { |m| store.respond_to?(m) } + unless missing.empty? + missing = missing.map { |m| "##{m}" }.join(", ") + warn "[rack-attack] Configured store #{store.class.name} doesn't respond to #{missing}" end end end diff --git a/spec/acceptance/cache_store_config_for_allow2ban_spec.rb b/spec/acceptance/cache_store_config_for_allow2ban_spec.rb index 45f8f5c3..e6d1405b 100644 --- a/spec/acceptance/cache_store_config_for_allow2ban_spec.rb +++ b/spec/acceptance/cache_store_config_for_allow2ban_spec.rb @@ -20,67 +20,14 @@ end end - it "gives semantic error if store is missing #read method" do - raised_exception = nil - - fake_store_class = Class.new do - def write(key, value); end - - def increment(key, count, options = {}); end + it "display warning if store is missing methods" do + warning = "[rack-attack] Configured store Object doesn't respond to #read, #write, #delete, #increment\n" + assert_output("", warning) do + Rack::Attack.cache.store = Object.new end - - Object.stub_const(:FakeStore, fake_store_class) do - Rack::Attack.cache.store = FakeStore.new - - raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do - get "/scarce-resource" - end - end - - assert_equal "Configured store FakeStore doesn't respond to #read method", raised_exception.message end - it "gives semantic error if store is missing #write method" do - raised_exception = nil - - fake_store_class = Class.new do - def read(key); end - - def increment(key, count, options = {}); end - end - - Object.stub_const(:FakeStore, fake_store_class) do - Rack::Attack.cache.store = FakeStore.new - - raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do - get "/scarce-resource" - end - end - - assert_equal "Configured store FakeStore doesn't respond to #write method", raised_exception.message - end - - it "gives semantic error if store is missing #increment method" do - raised_exception = nil - - fake_store_class = Class.new do - def read(key); end - - def write(key, value); end - end - - Object.stub_const(:FakeStore, fake_store_class) do - Rack::Attack.cache.store = FakeStore.new - - raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do - get "/scarce-resource" - end - end - - assert_equal "Configured store FakeStore doesn't respond to #increment method", raised_exception.message - end - - it "works with any object that responds to #read, #write and #increment" do + it "works with any object that responds to #read, #write, #delete and #increment" do fake_store_class = Class.new do attr_accessor :backend @@ -100,6 +47,10 @@ def increment(key, _count, _options = {}) @backend[key] ||= 0 @backend[key] += 1 end + + def delete(key) + @backend.delete(key) + end end Object.stub_const(:FakeStore, fake_store_class) do diff --git a/spec/acceptance/cache_store_config_for_fail2ban_spec.rb b/spec/acceptance/cache_store_config_for_fail2ban_spec.rb index b46f9fe5..52e87592 100644 --- a/spec/acceptance/cache_store_config_for_fail2ban_spec.rb +++ b/spec/acceptance/cache_store_config_for_fail2ban_spec.rb @@ -20,67 +20,14 @@ end end - it "gives semantic error if store is missing #read method" do - raised_exception = nil - - fake_store_class = Class.new do - def write(key, value); end - - def increment(key, count, options = {}); end + it "display warning if store is missing methods" do + warning = "[rack-attack] Configured store Object doesn't respond to #read, #write, #delete, #increment\n" + assert_output("", warning) do + Rack::Attack.cache.store = Object.new end - - Object.stub_const(:FakeStore, fake_store_class) do - Rack::Attack.cache.store = FakeStore.new - - raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do - get "/private-place" - end - end - - assert_equal "Configured store FakeStore doesn't respond to #read method", raised_exception.message end - it "gives semantic error if store is missing #write method" do - raised_exception = nil - - fake_store_class = Class.new do - def read(key); end - - def increment(key, count, options = {}); end - end - - Object.stub_const(:FakeStore, fake_store_class) do - Rack::Attack.cache.store = FakeStore.new - - raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do - get "/private-place" - end - end - - assert_equal "Configured store FakeStore doesn't respond to #write method", raised_exception.message - end - - it "gives semantic error if store is missing #increment method" do - raised_exception = nil - - fake_store_class = Class.new do - def read(key); end - - def write(key, value); end - end - - Object.stub_const(:FakeStore, fake_store_class) do - Rack::Attack.cache.store = FakeStore.new - - raised_exception = assert_raises(Rack::Attack::MisconfiguredStoreError) do - get "/private-place" - end - end - - assert_equal "Configured store FakeStore doesn't respond to #increment method", raised_exception.message - end - - it "works with any object that responds to #read, #write and #increment" do + it "works with any object that responds to #read, #write, #delete and #increment" do fake_store_class = Class.new do attr_accessor :backend @@ -100,6 +47,10 @@ def increment(key, _count, _options = {}) @backend[key] ||= 0 @backend[key] += 1 end + + def delete(key) + @backend.delete(key) + end end Rack::Attack.cache.store = fake_store_class.new diff --git a/spec/acceptance/cache_store_config_for_throttle_spec.rb b/spec/acceptance/cache_store_config_for_throttle_spec.rb index a89b8272..7ba4ad67 100644 --- a/spec/acceptance/cache_store_config_for_throttle_spec.rb +++ b/spec/acceptance/cache_store_config_for_throttle_spec.rb @@ -18,10 +18,9 @@ end it "gives semantic error if incompatible store was configured" do - Rack::Attack.cache.store = Object.new - - assert_raises(Rack::Attack::MisconfiguredStoreError) do - get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + warning = "[rack-attack] Configured store Object doesn't respond to #read, #write, #delete, #increment\n" + assert_output("", warning) do + Rack::Attack.cache.store = Object.new end end @@ -33,10 +32,22 @@ def initialize @counts = {} end + def read(key) + @counts[key] + end + + def write(key, count) + @counts[key] = count + end + def increment(key, _count, _options) @counts[key] ||= 0 @counts[key] += 1 end + + def delete(key) + @counts.delete(key) + end end Rack::Attack.cache.store = basic_store_class.new diff --git a/spec/rack_attack_reset_spec.rb b/spec/rack_attack_reset_spec.rb index b9a94e39..b4e167ac 100644 --- a/spec/rack_attack_reset_spec.rb +++ b/spec/rack_attack_reset_spec.rb @@ -4,9 +4,21 @@ describe "Rack::Attack.reset!" do it "raises an error when is not supported by cache store" do - Rack::Attack.cache.store = Class.new - assert_raises(Rack::Attack::IncompatibleStoreError) do - Rack::Attack.reset! + fake_store_class = Class.new do + def read(key); end + + def write(key, value); end + + def increment(key, count, options = {}); end + + def delete(key); end + end + + Object.stub_const(:FakeStore, fake_store_class) do + Rack::Attack.cache.store = FakeStore.new + assert_raises(Rack::Attack::IncompatibleStoreError) do + Rack::Attack.reset! + end end end