diff --git a/lib/rack/attack.rb b/lib/rack/attack.rb index c9094b21..cd5baeeb 100644 --- a/lib/rack/attack.rb +++ b/lib/rack/attack.rb @@ -128,6 +128,17 @@ def call(env) configuration.tracked?(request) @app.call(env) end + rescue *allowed_errors + @app.call(request.env) + end + + private + + def allowed_errors + errors = [] + errors << Dalli::DalliError if defined?(Dalli) + errors << Redis::BaseError if defined?(Redis) + errors end end end diff --git a/lib/rack/attack/store_proxy/dalli_proxy.rb b/lib/rack/attack/store_proxy/dalli_proxy.rb index 48198bb2..b2914ade 100644 --- a/lib/rack/attack/store_proxy/dalli_proxy.rb +++ b/lib/rack/attack/store_proxy/dalli_proxy.rb @@ -24,34 +24,26 @@ def initialize(client) end def read(key) - rescuing do - with do |client| - client.get(key) - end + with do |client| + client.get(key) end end def write(key, value, options = {}) - rescuing do - with do |client| - client.set(key, value, options.fetch(:expires_in, 0), raw: true) - end + with do |client| + client.set(key, value, options.fetch(:expires_in, 0), raw: true) end end def increment(key, amount, options = {}) - rescuing do - with do |client| - client.incr(key, amount, options.fetch(:expires_in, 0), amount) - end + with do |client| + client.incr(key, amount, options.fetch(:expires_in, 0), amount) end end def delete(key) - rescuing do - with do |client| - client.delete(key) - end + with do |client| + client.delete(key) end end @@ -66,12 +58,6 @@ def with end end end - - def rescuing - yield - rescue Dalli::DalliError - nil - end end end end diff --git a/lib/rack/attack/store_proxy/redis_proxy.rb b/lib/rack/attack/store_proxy/redis_proxy.rb index 830d39de..e8d833b8 100644 --- a/lib/rack/attack/store_proxy/redis_proxy.rb +++ b/lib/rack/attack/store_proxy/redis_proxy.rb @@ -19,50 +19,38 @@ def self.handle?(store) end def read(key) - rescuing { get(key) } + get(key) end def write(key, value, options = {}) if (expires_in = options[:expires_in]) - rescuing { setex(key, expires_in, value) } + setex(key, expires_in, value) else - rescuing { set(key, value) } + set(key, value) end end def increment(key, amount, options = {}) - rescuing do - pipelined do |redis| - redis.incrby(key, amount) - redis.expire(key, options[:expires_in]) if options[:expires_in] - end.first - end + pipelined do |redis| + redis.incrby(key, amount) + redis.expire(key, options[:expires_in]) if options[:expires_in] + end.first end def delete(key, _options = {}) - rescuing { del(key) } + del(key) end def delete_matched(matcher, _options = nil) cursor = "0" - rescuing do - # Fetch keys in batches using SCAN to avoid blocking the Redis server. - loop do - cursor, keys = scan(cursor, match: matcher, count: 1000) - del(*keys) unless keys.empty? - break if cursor == "0" - end + # Fetch keys in batches using SCAN to avoid blocking the Redis server. + loop do + cursor, keys = scan(cursor, match: matcher, count: 1000) + del(*keys) unless keys.empty? + break if cursor == "0" end end - - private - - def rescuing - yield - rescue Redis::BaseConnectionError - nil - 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 28557bcb..7249e1d5 100644 --- a/lib/rack/attack/store_proxy/redis_store_proxy.rb +++ b/lib/rack/attack/store_proxy/redis_store_proxy.rb @@ -11,14 +11,14 @@ def self.handle?(store) end def read(key) - rescuing { get(key, raw: true) } + get(key, raw: true) end def write(key, value, options = {}) if (expires_in = options[:expires_in]) - rescuing { setex(key, expires_in, value, raw: true) } + setex(key, expires_in, value, raw: true) else - rescuing { set(key, value, raw: true) } + set(key, value, raw: true) end end end diff --git a/rack-attack.gemspec b/rack-attack.gemspec index 41cc7a8f..2d5c72f6 100644 --- a/rack-attack.gemspec +++ b/rack-attack.gemspec @@ -32,6 +32,7 @@ Gem::Specification.new do |s| s.add_development_dependency "bundler", ">= 1.17", "< 3.0" s.add_development_dependency 'minitest', "~> 5.11" s.add_development_dependency "minitest-stub-const", "~> 0.6" + s.add_development_dependency 'rspec-mocks', '~> 3.11.0' s.add_development_dependency 'rack-test', "~> 2.0" s.add_development_dependency 'rake', "~> 13.0" s.add_development_dependency "rubocop", "1.12.1" diff --git a/spec/acceptance/error_handling_spec.rb b/spec/acceptance/error_handling_spec.rb new file mode 100644 index 00000000..ae27993b --- /dev/null +++ b/spec/acceptance/error_handling_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require_relative "../spec_helper" + +describe "error handling" do + + let(:store) do + ActiveSupport::Cache::MemoryStore.new + end + + before do + Rack::Attack.cache.store = store + + Rack::Attack.blocklist("fail2ban pentesters") do |request| + Rack::Attack::Fail2Ban.filter(request.ip, maxretry: 0, bantime: 600, findtime: 30) { true } + end + end + + describe '.call' do + before do + allow(store).to receive(:read).and_raise(raised_error) + end + + describe 'when raising Dalli::DalliError' do + let(:raised_error) { stub_const('Dalli::DalliError', Class.new(StandardError)) } + + it 'allows the response' do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + + assert_equal 200, last_response.status + end + end + + describe 'when raising Redis::BaseError' do + let(:raised_error) { stub_const('Redis::BaseError', Class.new(StandardError)) } + + it 'allows the response' do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + + assert_equal 200, last_response.status + end + end + + describe 'when raising other error' do + let(:raised_error) { RuntimeError } + + it 'raises error if not ignored' do + assert_raises(RuntimeError) do + get "/", {}, "REMOTE_ADDR" => "1.2.3.4" + end + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index f529e6a1..bda61a84 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -3,7 +3,7 @@ require "bundler/setup" require "minitest/autorun" -require "minitest/pride" +require "rspec/mocks/minitest_integration" require "rack/test" require "active_support" require "rack/attack"