From 2e3febbc065a9a86150889385ee34d78c1d8bbd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Pe=C3=B1aloza?= Date: Tue, 21 Apr 2026 11:16:18 -0400 Subject: [PATCH 1/5] feat: Introduce bypassable_store_errors option in base proxy Replaces the bypass_all_store_errors + bypassable_store_errors pair with a single option that accepts :all, :none, or an Array of Classes / String class names (Strings resolve by ancestor-name match so missing gems don't crash loading). Subclasses can declare their own default via default_bypassable_store_errors. Invalid values raise MisconfiguredStoreError. --- lib/rack/attack/base_proxy.rb | 65 +++++++++ spec/rack_attack_base_proxy_spec.rb | 210 ++++++++++++++++++++++++++++ 2 files changed, 275 insertions(+) create mode 100644 spec/rack_attack_base_proxy_spec.rb diff --git a/lib/rack/attack/base_proxy.rb b/lib/rack/attack/base_proxy.rb index f10af3d4..73d3ccd9 100644 --- a/lib/rack/attack/base_proxy.rb +++ b/lib/rack/attack/base_proxy.rb @@ -5,6 +5,71 @@ module Rack class Attack class BaseProxy < SimpleDelegator + attr_reader :bypassable_store_errors + + def initialize(store, bypassable_store_errors: nil) + super(store) + @bypassable_store_errors = normalize_bypassable_store_errors(bypassable_store_errors) + end + + def self.default_bypassable_store_errors + [] + end + + protected + + def handle_store_error + yield + rescue => e + raise e unless should_bypass_error?(e) + end + + private + + def should_bypass_error?(error) + case @bypassable_store_errors + when :all + true + when :none + false + else + @bypassable_store_errors.any? { |candidate| error_matches?(error, candidate) } + end + end + + def error_matches?(error, candidate) + case candidate + when Class + error.is_a?(candidate) + when String + klass = error.class + while klass + return true if klass.name == candidate + + klass = klass.superclass + end + false + end + end + + def normalize_bypassable_store_errors(value) + case value + when nil + self.class.default_bypassable_store_errors + when :all, :none + value + when Array + unless value.all? { |e| e.is_a?(Class) || e.is_a?(String) } + raise Rack::Attack::MisconfiguredStoreError, + "bypassable_store_errors Array must contain only Class or String entries" + end + value + else + raise Rack::Attack::MisconfiguredStoreError, + "bypassable_store_errors must be :all, :none, or an Array of error Classes or class name Strings" + end + end + class << self def proxies @@proxies ||= [] diff --git a/spec/rack_attack_base_proxy_spec.rb b/spec/rack_attack_base_proxy_spec.rb new file mode 100644 index 00000000..e7552c61 --- /dev/null +++ b/spec/rack_attack_base_proxy_spec.rb @@ -0,0 +1,210 @@ +# frozen_string_literal: true + +require_relative 'spec_helper' + +module BaseProxySpec + CustomError = Class.new(StandardError) + SubCustomError = Class.new(CustomError) + OtherError = Class.new(StandardError) + DefaultError = Class.new(StandardError) + ClassDefaultError = Class.new(StandardError) + + class TestProxy < Rack::Attack::BaseProxy + def self.handle?(store) + store.is_a?(Hash) && store[:test_proxy] + end + + def call_handle_store_error(&block) + handle_store_error(&block) + end + end + + class DefaultsProxy < Rack::Attack::BaseProxy + def self.handle?(_store) + false + end + + def self.default_bypassable_store_errors + ['BaseProxySpec::DefaultError', BaseProxySpec::ClassDefaultError] + end + + def call_handle_store_error(&block) + handle_store_error(&block) + end + end +end + +describe Rack::Attack::BaseProxy do + describe "#initialize" do + it "defaults bypassable_store_errors to [] when proxy has no default" do + proxy = BaseProxySpec::TestProxy.new(Object.new) + _(proxy.bypassable_store_errors).must_equal [] + end + + it "uses the subclass default when bypassable_store_errors is not provided" do + proxy = BaseProxySpec::DefaultsProxy.new(Object.new) + _(proxy.bypassable_store_errors).must_include 'BaseProxySpec::DefaultError' + _(proxy.bypassable_store_errors).must_include BaseProxySpec::ClassDefaultError + end + + it "accepts :all" do + proxy = BaseProxySpec::TestProxy.new(Object.new, bypassable_store_errors: :all) + _(proxy.bypassable_store_errors).must_equal :all + end + + it "accepts :none" do + proxy = BaseProxySpec::TestProxy.new(Object.new, bypassable_store_errors: :none) + _(proxy.bypassable_store_errors).must_equal :none + end + + it "accepts an Array of Classes" do + proxy = BaseProxySpec::TestProxy.new(Object.new, bypassable_store_errors: [BaseProxySpec::CustomError]) + _(proxy.bypassable_store_errors).must_equal [BaseProxySpec::CustomError] + end + + it "accepts an Array of String class names" do + proxy = BaseProxySpec::TestProxy.new(Object.new, bypassable_store_errors: ['BaseProxySpec::CustomError']) + _(proxy.bypassable_store_errors).must_equal ['BaseProxySpec::CustomError'] + end + + it "accepts a mixed Array of Classes and Strings" do + proxy = BaseProxySpec::TestProxy.new( + Object.new, + bypassable_store_errors: [BaseProxySpec::CustomError, 'Other'] + ) + _(proxy.bypassable_store_errors).must_equal [BaseProxySpec::CustomError, 'Other'] + end + + it "user-provided value overrides subclass default" do + proxy = BaseProxySpec::DefaultsProxy.new( + Object.new, + bypassable_store_errors: [BaseProxySpec::CustomError] + ) + _(proxy.bypassable_store_errors).must_equal [BaseProxySpec::CustomError] + end + + it ":none overrides subclass default to bypass nothing" do + proxy = BaseProxySpec::DefaultsProxy.new(Object.new, bypassable_store_errors: :none) + _(proxy.bypassable_store_errors).must_equal :none + end + + it "raises MisconfiguredStoreError for unsupported Symbol" do + error = _(-> { + BaseProxySpec::TestProxy.new(Object.new, bypassable_store_errors: :sometimes) + }).must_raise Rack::Attack::MisconfiguredStoreError + _(error.message).must_match(/:all, :none/) + end + + it "raises MisconfiguredStoreError for non-Array, non-Symbol value" do + _(-> { + BaseProxySpec::TestProxy.new(Object.new, bypassable_store_errors: "Redis::Error") + }).must_raise Rack::Attack::MisconfiguredStoreError + end + + it "raises MisconfiguredStoreError for an Array containing invalid entries" do + _(-> { + BaseProxySpec::TestProxy.new(Object.new, bypassable_store_errors: [BaseProxySpec::CustomError, 42]) + }).must_raise Rack::Attack::MisconfiguredStoreError + end + + it "accepts an empty Array as 'bypass nothing'" do + proxy = BaseProxySpec::TestProxy.new(Object.new, bypassable_store_errors: []) + _(proxy.bypassable_store_errors).must_equal [] + end + end + + describe "#handle_store_error" do + it "returns the block's result when it does not raise" do + proxy = BaseProxySpec::TestProxy.new(Object.new, bypassable_store_errors: [BaseProxySpec::CustomError]) + result = proxy.call_handle_store_error { 42 } + _(result).must_equal 42 + end + + it "bypasses matching error class and returns nil" do + proxy = BaseProxySpec::TestProxy.new(Object.new, bypassable_store_errors: [BaseProxySpec::CustomError]) + result = proxy.call_handle_store_error { raise BaseProxySpec::CustomError, "boom" } + _(result).must_be_nil + end + + it "bypasses subclass of a matching error class" do + proxy = BaseProxySpec::TestProxy.new(Object.new, bypassable_store_errors: [BaseProxySpec::CustomError]) + result = proxy.call_handle_store_error { raise BaseProxySpec::SubCustomError, "boom" } + _(result).must_be_nil + end + + it "bypasses matching error by String class name" do + proxy = BaseProxySpec::TestProxy.new( + Object.new, + bypassable_store_errors: ['BaseProxySpec::CustomError'] + ) + result = proxy.call_handle_store_error { raise BaseProxySpec::CustomError, "boom" } + _(result).must_be_nil + end + + it "bypasses subclass error when ancestor name is configured as String" do + proxy = BaseProxySpec::TestProxy.new( + Object.new, + bypassable_store_errors: ['BaseProxySpec::CustomError'] + ) + result = proxy.call_handle_store_error { raise BaseProxySpec::SubCustomError, "boom" } + _(result).must_be_nil + end + + it "re-raises unmatched errors" do + proxy = BaseProxySpec::TestProxy.new(Object.new, bypassable_store_errors: [BaseProxySpec::CustomError]) + _(-> { + proxy.call_handle_store_error { raise BaseProxySpec::OtherError, "nope" } + }).must_raise BaseProxySpec::OtherError + end + + it "re-raises all errors when bypassable_store_errors is :none" do + proxy = BaseProxySpec::TestProxy.new(Object.new, bypassable_store_errors: :none) + _(-> { + proxy.call_handle_store_error { raise BaseProxySpec::CustomError, "boom" } + }).must_raise BaseProxySpec::CustomError + end + + it "bypasses all errors when bypassable_store_errors is :all" do + proxy = BaseProxySpec::TestProxy.new(Object.new, bypassable_store_errors: :all) + result = proxy.call_handle_store_error { raise RuntimeError, "anything" } + _(result).must_be_nil + end + + it "bypasses errors using default list when no override is provided" do + proxy = BaseProxySpec::DefaultsProxy.new(Object.new) + result_a = proxy.call_handle_store_error { raise BaseProxySpec::DefaultError, "boom" } + _(result_a).must_be_nil + result_b = proxy.call_handle_store_error { raise BaseProxySpec::ClassDefaultError, "boom" } + _(result_b).must_be_nil + end + + it "re-raises errors not in the default list" do + proxy = BaseProxySpec::DefaultsProxy.new(Object.new) + _(-> { + proxy.call_handle_store_error { raise BaseProxySpec::OtherError, "nope" } + }).must_raise BaseProxySpec::OtherError + end + end + + describe "String ancestor matching" do + it "ignores String entries whose name matches no ancestor" do + proxy = BaseProxySpec::TestProxy.new( + Object.new, + bypassable_store_errors: ['DoesNotExist::Error'] + ) + _(-> { + proxy.call_handle_store_error { raise BaseProxySpec::CustomError, "boom" } + }).must_raise BaseProxySpec::CustomError + end + + it "works even if the referenced constant is not loaded" do + proxy = BaseProxySpec::TestProxy.new( + Object.new, + bypassable_store_errors: ['Totally::Unknown'] + ) + _(-> { + proxy.call_handle_store_error { raise BaseProxySpec::CustomError, "boom" } + }).must_raise BaseProxySpec::CustomError + end + end +end From 97c224e7413684681c2a40f03f0df89523eede96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Pe=C3=B1aloza?= Date: Tue, 21 Apr 2026 11:27:09 -0400 Subject: [PATCH 2/5] feat: Wire bypassable_store_errors through Cache Adds Cache#bypassable_store_errors= as a separate accessor so callers can configure the option without the awkward multi-arg setter pattern. Both store= and bypassable_store_errors= re-wrap the underlying store, so configuration order is irrelevant. --- lib/rack/attack/cache.rb | 26 ++-- spec/rack_attack_cache_bypassable_spec.rb | 146 ++++++++++++++++++++++ 2 files changed, 165 insertions(+), 7 deletions(-) create mode 100644 spec/rack_attack_cache_bypassable_spec.rb diff --git a/lib/rack/attack/cache.rb b/lib/rack/attack/cache.rb index 9111ab8a..1caa0d3f 100644 --- a/lib/rack/attack/cache.rb +++ b/lib/rack/attack/cache.rb @@ -4,7 +4,7 @@ module Rack class Attack class Cache attr_accessor :prefix - attr_reader :last_epoch_time + attr_reader :last_epoch_time, :bypassable_store_errors def self.default_store if Object.const_defined?(:Rails) && Rails.respond_to?(:cache) @@ -13,6 +13,7 @@ def self.default_store end def initialize(store: self.class.default_store) + @bypassable_store_errors = nil self.store = store @prefix = 'rack::attack' end @@ -20,12 +21,13 @@ def initialize(store: self.class.default_store) attr_reader :store def store=(store) - @store = - if (proxy = BaseProxy.lookup(store)) - proxy.new(store) - else - store - end + @raw_store = store + @store = wrap_store(store) + end + + def bypassable_store_errors=(value) + @bypassable_store_errors = value + @store = wrap_store(@raw_store) if @raw_store end def count(unprefixed_key, period) @@ -66,6 +68,16 @@ def reset! private + def wrap_store(store) + return store if store.nil? + + if (proxy = BaseProxy.lookup(store)) + proxy.new(store, bypassable_store_errors: @bypassable_store_errors) + else + store + end + end + def key_and_expiry(unprefixed_key, period) @last_epoch_time = Time.now.to_i # Add 1 to expires_in to avoid timing error: https://github.com/rack/rack-attack/pull/85 diff --git a/spec/rack_attack_cache_bypassable_spec.rb b/spec/rack_attack_cache_bypassable_spec.rb new file mode 100644 index 00000000..3419de2e --- /dev/null +++ b/spec/rack_attack_cache_bypassable_spec.rb @@ -0,0 +1,146 @@ +# frozen_string_literal: true + +require_relative 'spec_helper' + +module CacheBypassableSpec + class FlakyStore + class Error < StandardError; end + + attr_reader :writes, :reads, :increments, :deletes + + def initialize(raises: nil) + @raises = raises + @writes = [] + @reads = [] + @increments = [] + @deletes = [] + end + + def read(key) + @reads << key + raise @raises if @raises + + nil + end + + def write(key, value, opts = {}) + @writes << [key, value, opts] + raise @raises if @raises + + :ok + end + + def increment(key, amount, opts = {}) + @increments << [key, amount, opts] + raise @raises if @raises + + 1 + end + + def delete(key) + @deletes << key + raise @raises if @raises + + :ok + end + + def delete_matched(_); end + end + + class FlakyStoreProxy < Rack::Attack::BaseProxy + def self.handle?(store) + store.is_a?(CacheBypassableSpec::FlakyStore) + end + + def read(key, *args) + handle_store_error { __getobj__.read(key, *args) } + end + + def write(key, value, opts = {}) + handle_store_error { __getobj__.write(key, value, opts) } + end + + def increment(key, amount, opts = {}) + handle_store_error { __getobj__.increment(key, amount, opts) } + end + + def delete(key) + handle_store_error { __getobj__.delete(key) } + end + end +end + +describe "Rack::Attack::Cache bypassable_store_errors integration" do + before do + @cache = Rack::Attack::Cache.new(store: nil) + end + + it "wraps the store with a matching proxy when set via #store=" do + flaky = CacheBypassableSpec::FlakyStore.new + @cache.store = flaky + _(@cache.store).must_be_instance_of CacheBypassableSpec::FlakyStoreProxy + end + + it "passes bypassable_store_errors to the proxy when set before the store" do + @cache.bypassable_store_errors = [CacheBypassableSpec::FlakyStore::Error] + @cache.store = CacheBypassableSpec::FlakyStore.new + _(@cache.store.bypassable_store_errors).must_equal [CacheBypassableSpec::FlakyStore::Error] + end + + it "re-wraps the store when bypassable_store_errors is changed after store=" do + @cache.store = CacheBypassableSpec::FlakyStore.new + @cache.bypassable_store_errors = :all + _(@cache.store.bypassable_store_errors).must_equal :all + end + + it "re-wraps preserving the same underlying raw store" do + raw = CacheBypassableSpec::FlakyStore.new + @cache.store = raw + wrapped_before = @cache.store + @cache.bypassable_store_errors = [CacheBypassableSpec::FlakyStore::Error] + _(@cache.store).wont_equal wrapped_before + _(@cache.store.__getobj__).must_be_same_as raw + end + + it "propagates cache writes and bypasses configured errors" do + flaky = CacheBypassableSpec::FlakyStore.new(raises: CacheBypassableSpec::FlakyStore::Error.new("oom")) + @cache.bypassable_store_errors = [CacheBypassableSpec::FlakyStore::Error] + @cache.store = flaky + + result = @cache.count("key", 60) + _(result).must_equal 1 + end + + it "leaves non-proxyable stores untouched" do + plain = Object.new + @cache.store = plain + _(@cache.store).must_be_same_as plain + end + + it "allows nil store to remain nil" do + @cache.store = nil + _(@cache.store).must_be_nil + end + + it "raises MisconfiguredStoreError when bypassable_store_errors is invalid" do + flaky = CacheBypassableSpec::FlakyStore.new + _(-> { + @cache.bypassable_store_errors = :sometimes + @cache.store = flaky + }).must_raise Rack::Attack::MisconfiguredStoreError + end + + it "re-raises errors not in the bypass list" do + flaky = CacheBypassableSpec::FlakyStore.new(raises: RuntimeError.new("boom")) + @cache.bypassable_store_errors = [CacheBypassableSpec::FlakyStore::Error] + @cache.store = flaky + _(-> { @cache.count("key", 60) }).must_raise RuntimeError + end + + it ":none disables any bypass" do + flaky = CacheBypassableSpec::FlakyStore.new(raises: CacheBypassableSpec::FlakyStore::Error.new("oom")) + @cache.bypassable_store_errors = :none + @cache.store = flaky + _(-> { @cache.count("key", 60) }).must_raise CacheBypassableSpec::FlakyStore::Error + end +end From 75e40dab9da97676c5afc84cbe2c94651654e41a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Pe=C3=B1aloza?= Date: Tue, 21 Apr 2026 11:27:16 -0400 Subject: [PATCH 3/5] feat: Route DalliProxy errors through handle_store_error Replaces the bespoke rescuing helper with handle_store_error and declares ['Dalli::DalliError'] as DalliProxy.default_bypassable_store_errors. Because the entry is a String, loading DalliProxy does not require the dalli gem to be present. --- lib/rack/attack/store_proxy/dalli_proxy.rb | 22 +++--- spec/rack_attack_dalli_proxy_spec.rb | 85 ++++++++++++++++++++++ 2 files changed, 95 insertions(+), 12 deletions(-) diff --git a/lib/rack/attack/store_proxy/dalli_proxy.rb b/lib/rack/attack/store_proxy/dalli_proxy.rb index 48198bb2..6db3d1e3 100644 --- a/lib/rack/attack/store_proxy/dalli_proxy.rb +++ b/lib/rack/attack/store_proxy/dalli_proxy.rb @@ -18,13 +18,17 @@ def self.handle?(store) end end - def initialize(client) - super(client) + def self.default_bypassable_store_errors + ['Dalli::DalliError'] + end + + def initialize(client, **options) + super(client, **options) stub_with_if_missing end def read(key) - rescuing do + handle_store_error do with do |client| client.get(key) end @@ -32,7 +36,7 @@ def read(key) end def write(key, value, options = {}) - rescuing do + handle_store_error do with do |client| client.set(key, value, options.fetch(:expires_in, 0), raw: true) end @@ -40,7 +44,7 @@ def write(key, value, options = {}) end def increment(key, amount, options = {}) - rescuing do + handle_store_error do with do |client| client.incr(key, amount, options.fetch(:expires_in, 0), amount) end @@ -48,7 +52,7 @@ def increment(key, amount, options = {}) end def delete(key) - rescuing do + handle_store_error do with do |client| client.delete(key) end @@ -66,12 +70,6 @@ def with end end end - - def rescuing - yield - rescue Dalli::DalliError - nil - end end end end diff --git a/spec/rack_attack_dalli_proxy_spec.rb b/spec/rack_attack_dalli_proxy_spec.rb index 7eb23f00..50905b4b 100644 --- a/spec/rack_attack_dalli_proxy_spec.rb +++ b/spec/rack_attack_dalli_proxy_spec.rb @@ -2,9 +2,94 @@ require_relative 'spec_helper' +module DalliProxySpec + class FakeDalliClient + def initialize(raises: nil) + @raises = raises + end + + def get(_key) + bomb_or(:value) + end + + def set(*_args) + bomb_or(:ok) + end + + def incr(*_args) + bomb_or(1) + end + + def delete(_key) + bomb_or(:ok) + end + + def with + yield(self) + end + + private + + def bomb_or(value) + raise @raises if @raises + + value + end + end +end + describe Rack::Attack::StoreProxy::DalliProxy do it 'should stub Dalli::Client#with on older clients' do proxy = Rack::Attack::StoreProxy::DalliProxy.new(Class.new) proxy.with {} # will not raise an error end + + describe "bypassable_store_errors" do + it "uses ['Dalli::DalliError'] as the default bypass list (as a String)" do + _(Rack::Attack::StoreProxy::DalliProxy.default_bypassable_store_errors) + .must_equal ['Dalli::DalliError'] + end + + it "bypasses Dalli::DalliError by default on #read" do + skip 'dalli gem not available' unless defined?(::Dalli::DalliError) + client = DalliProxySpec::FakeDalliClient.new(raises: Dalli::DalliError.new("server down")) + proxy = Rack::Attack::StoreProxy::DalliProxy.new(client) + _(proxy.read("k")).must_be_nil + end + + it "re-raises non-Dalli errors by default" do + client = DalliProxySpec::FakeDalliClient.new(raises: ArgumentError.new("boom")) + proxy = Rack::Attack::StoreProxy::DalliProxy.new(client) + _(-> { proxy.read("k") }).must_raise ArgumentError + end + + it "expresses its default as a String class name so no const is resolved eagerly" do + default = Rack::Attack::StoreProxy::DalliProxy.default_bypassable_store_errors + _(default).must_be_kind_of Array + _(default.all? { |entry| entry.is_a?(String) }).must_equal true + end + + it "user :none disables the default Dalli bypass" do + skip 'dalli gem not available' unless defined?(::Dalli::DalliError) + client = DalliProxySpec::FakeDalliClient.new(raises: Dalli::DalliError.new("server down")) + proxy = Rack::Attack::StoreProxy::DalliProxy.new(client, bypassable_store_errors: :none) + _(-> { proxy.read("k") }).must_raise Dalli::DalliError + end + + it "user Array overrides the default Dalli bypass" do + skip 'dalli gem not available' unless defined?(::Dalli::DalliError) + client = DalliProxySpec::FakeDalliClient.new(raises: Dalli::DalliError.new("server down")) + proxy = Rack::Attack::StoreProxy::DalliProxy.new( + client, + bypassable_store_errors: [ArgumentError] + ) + _(-> { proxy.read("k") }).must_raise Dalli::DalliError + end + + it "user :all bypasses any error" do + client = DalliProxySpec::FakeDalliClient.new(raises: RuntimeError.new("boom")) + proxy = Rack::Attack::StoreProxy::DalliProxy.new(client, bypassable_store_errors: :all) + _(proxy.read("k")).must_be_nil + end + end end From a861023bddd2f0b821aecbc460ca34b55fb8b2f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Pe=C3=B1aloza?= Date: Tue, 21 Apr 2026 11:27:23 -0400 Subject: [PATCH 4/5] feat: Route RedisProxy errors through handle_store_error Replaces the bespoke rescuing helper with handle_store_error and declares ['Redis::BaseConnectionError'] as the default bypass list. --- lib/rack/attack/store_proxy/redis_proxy.rb | 28 ++--- spec/rack_attack_redis_proxy_spec.rb | 135 +++++++++++++++++++++ 2 files changed, 147 insertions(+), 16 deletions(-) create mode 100644 spec/rack_attack_redis_proxy_spec.rb diff --git a/lib/rack/attack/store_proxy/redis_proxy.rb b/lib/rack/attack/store_proxy/redis_proxy.rb index 599213ae..6b00b8a5 100644 --- a/lib/rack/attack/store_proxy/redis_proxy.rb +++ b/lib/rack/attack/store_proxy/redis_proxy.rb @@ -6,32 +6,36 @@ module Rack class Attack module StoreProxy class RedisProxy < BaseProxy - def initialize(*args) + def initialize(store, **options) if Gem::Version.new(Redis::VERSION) < Gem::Version.new("3") warn 'RackAttack requires Redis gem >= 3.0.0.' end - super(*args) + super(store, **options) end def self.handle?(store) defined?(::Redis) && store.class == ::Redis end + def self.default_bypassable_store_errors + ['Redis::BaseConnectionError'] + end + def read(key) - rescuing { get(key) } + handle_store_error { get(key) } end def write(key, value, options = {}) if (expires_in = options[:expires_in]) - rescuing { setex(key, expires_in, value) } + handle_store_error { setex(key, expires_in, value) } else - rescuing { set(key, value) } + handle_store_error { set(key, value) } end end def increment(key, amount, options = {}) - rescuing do + handle_store_error do pipelined do |redis| redis.incrby(key, amount) redis.expire(key, options[:expires_in]) if options[:expires_in] @@ -40,14 +44,14 @@ def increment(key, amount, options = {}) end def delete(key, _options = {}) - rescuing { del(key) } + handle_store_error { del(key) } end def delete_matched(matcher, _options = nil) cursor = "0" source = matcher.source - rescuing do + handle_store_error do # Fetch keys in batches using SCAN to avoid blocking the Redis server. loop do cursor, keys = scan(cursor, match: source, count: 1000) @@ -56,14 +60,6 @@ def delete_matched(matcher, _options = nil) end end end - - private - - def rescuing - yield - rescue Redis::BaseConnectionError - nil - end end end end diff --git a/spec/rack_attack_redis_proxy_spec.rb b/spec/rack_attack_redis_proxy_spec.rb new file mode 100644 index 00000000..da86d1cb --- /dev/null +++ b/spec/rack_attack_redis_proxy_spec.rb @@ -0,0 +1,135 @@ +# frozen_string_literal: true + +require_relative 'spec_helper' + +module RedisProxySpec + class FakeRedis + class Error < StandardError; end + + attr_reader :calls + + def initialize(raises: nil) + @raises = raises + @calls = [] + end + + def get(key) + record(:get, key) + end + + def set(key, value) + record(:set, [key, value]) + end + + def setex(key, expires_in, value) + record(:setex, [key, expires_in, value]) + end + + def del(*keys) + record(:del, keys) + end + + def pipelined + record(:pipelined) + yield(self) if block_given? + [1] + end + + def incrby(key, amount) + record(:incrby, [key, amount]) + end + + def expire(key, ttl) + record(:expire, [key, ttl]) + end + + def scan(_cursor, **_opts) + ["0", []] + end + + def record(method, args = nil) + @calls << [method, args] + raise @raises if @raises + + :ok + end + end +end + +describe Rack::Attack::StoreProxy::RedisProxy do + before do + skip 'redis gem not available' unless defined?(::Redis) + end + + it "declares its default as ['Redis::BaseConnectionError']" do + _(Rack::Attack::StoreProxy::RedisProxy.default_bypassable_store_errors) + .must_equal ['Redis::BaseConnectionError'] + end + + it "bypasses Redis::BaseConnectionError by default on #read" do + redis = RedisProxySpec::FakeRedis.new(raises: Redis::BaseConnectionError.new("down")) + proxy = Rack::Attack::StoreProxy::RedisProxy.new(redis) + _(proxy.read("k")).must_be_nil + end + + it "bypasses Redis::BaseConnectionError subclasses on #write" do + redis = RedisProxySpec::FakeRedis.new(raises: Redis::CannotConnectError.new("down")) + proxy = Rack::Attack::StoreProxy::RedisProxy.new(redis) + _(proxy.write("k", "v")).must_be_nil + end + + it "re-raises other errors by default" do + redis = RedisProxySpec::FakeRedis.new(raises: RuntimeError.new("boom")) + proxy = Rack::Attack::StoreProxy::RedisProxy.new(redis) + _(-> { proxy.read("k") }).must_raise RuntimeError + end + + it "bypasses custom errors when configured" do + redis = RedisProxySpec::FakeRedis.new(raises: RedisProxySpec::FakeRedis::Error.new("oom")) + proxy = Rack::Attack::StoreProxy::RedisProxy.new( + redis, + bypassable_store_errors: [RedisProxySpec::FakeRedis::Error] + ) + _(proxy.read("k")).must_be_nil + end + + it "bypasses all errors when configured with :all" do + redis = RedisProxySpec::FakeRedis.new(raises: RuntimeError.new("anything")) + proxy = Rack::Attack::StoreProxy::RedisProxy.new(redis, bypassable_store_errors: :all) + _(proxy.read("k")).must_be_nil + end + + it "re-raises default errors when configured with :none" do + redis = RedisProxySpec::FakeRedis.new(raises: Redis::BaseConnectionError.new("down")) + proxy = Rack::Attack::StoreProxy::RedisProxy.new(redis, bypassable_store_errors: :none) + _(-> { proxy.read("k") }).must_raise Redis::BaseConnectionError + end + + it "user-provided Array overrides the default (does not merge)" do + redis = RedisProxySpec::FakeRedis.new(raises: Redis::BaseConnectionError.new("down")) + proxy = Rack::Attack::StoreProxy::RedisProxy.new( + redis, + bypassable_store_errors: [RedisProxySpec::FakeRedis::Error] + ) + _(-> { proxy.read("k") }).must_raise Redis::BaseConnectionError + end + + it "returns nil from #increment on a matching error" do + redis = RedisProxySpec::FakeRedis.new(raises: Redis::BaseConnectionError.new("down")) + proxy = Rack::Attack::StoreProxy::RedisProxy.new(redis) + _(proxy.increment("k", 1, expires_in: 60)).must_be_nil + end + + it "returns nil from #delete on a matching error" do + redis = RedisProxySpec::FakeRedis.new(raises: Redis::BaseConnectionError.new("down")) + proxy = Rack::Attack::StoreProxy::RedisProxy.new(redis) + _(proxy.delete("k")).must_be_nil + end + + it "raises MisconfiguredStoreError for invalid config" do + redis = RedisProxySpec::FakeRedis.new + _(-> { + Rack::Attack::StoreProxy::RedisProxy.new(redis, bypassable_store_errors: :sometimes) + }).must_raise Rack::Attack::MisconfiguredStoreError + end +end From 5b08553b0a81a1ba8e1f1416bc9295cd0a56d3df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Pe=C3=B1aloza?= Date: Tue, 21 Apr 2026 11:27:31 -0400 Subject: [PATCH 5/5] feat: Route remaining proxies through handle_store_error and document Applies handle_store_error in RedisStoreProxy, RedisCacheStoreProxy and MemCacheStoreProxy so their calls participate in the shared bypass mechanism. Adds cross-proxy specs pinning each subclass default and documents bypassable_store_errors in the README. --- README.md | 29 +++++++++++++++ .../store_proxy/mem_cache_store_proxy.rb | 4 +-- .../store_proxy/redis_cache_store_proxy.rb | 10 +++--- .../attack/store_proxy/redis_store_proxy.rb | 6 ++-- spec/rack_attack_store_proxy_defaults_spec.rb | 36 +++++++++++++++++++ 5 files changed, 75 insertions(+), 10 deletions(-) create mode 100644 spec/rack_attack_store_proxy_defaults_spec.rb diff --git a/README.md b/README.md index 35d1cdaa..5bae091e 100644 --- a/README.md +++ b/README.md @@ -315,6 +315,35 @@ Most applications should use a new, separate database used only for `rack-attack Note that `Rack::Attack.cache` is only used for throttling, allow2ban and fail2ban filtering; not blocklisting and safelisting. Your cache store must implement `increment` and `write` like [ActiveSupport::Cache::Store](http://api.rubyonrails.org/classes/ActiveSupport/Cache/Store.html). This means that other cache stores which inherit from ActiveSupport::Cache::Store are also compatible. In-memory stores which are not backed by an external database, such as `ActiveSupport::Cache::MemoryStore.new`, will be mostly ineffective because each Ruby process in your deployment will have it's own state, effectively multiplying the number of requests each client can make by the number of Ruby processes you have deployed. +#### Bypassing store errors + +By default, some store proxies will swallow the errors they historically rescued (`Redis::BaseConnectionError` for `Redis`, `Dalli::DalliError` for `Dalli`). When one of those errors is raised inside the proxy, the request goes through as if no throttling were applied, which keeps your app available if the dedicated rack-attack store goes down. + +You can customize this behavior through `Rack::Attack.cache.bypassable_store_errors`: + +```ruby +# Use the proxy's built-in defaults (this is the default) +Rack::Attack.cache.store = ActiveSupport::Cache::RedisCacheStore.new(url: "...") + +# Bypass ALL errors raised by the store — requests continue serving even if the +# store misbehaves in unexpected ways (e.g. Redis OOM, timeouts, protocol errors). +Rack::Attack.cache.bypassable_store_errors = :all + +# Bypass NO errors — any error from the store will propagate. This disables the +# proxy's historical default rescue behavior as well. +Rack::Attack.cache.bypassable_store_errors = :none + +# Bypass a specific list of error classes (or class-name Strings). This REPLACES +# the proxy's built-in defaults - include any you still want to rescue. +Rack::Attack.cache.bypassable_store_errors = [ + Redis::BaseConnectionError, + Redis::TimeoutError, + "Redis::CommandError" +] +``` + +`bypassable_store_errors` can be set before or after assigning `cache.store`; the store is re-wrapped automatically. + ## Customizing responses Customize the response of blocklisted and throttled requests using an object that adheres to the [Rack app interface](http://www.rubydoc.info/github/rack/rack/file/SPEC.rdoc). diff --git a/lib/rack/attack/store_proxy/mem_cache_store_proxy.rb b/lib/rack/attack/store_proxy/mem_cache_store_proxy.rb index f7b66c92..81ae840f 100644 --- a/lib/rack/attack/store_proxy/mem_cache_store_proxy.rb +++ b/lib/rack/attack/store_proxy/mem_cache_store_proxy.rb @@ -13,11 +13,11 @@ def self.handle?(store) end def read(name, options = {}) - super(name, options.merge!(raw: true)) + handle_store_error { super(name, options.merge!(raw: true)) } end def write(name, value, options = {}) - super(name, value, options.merge!(raw: true)) + handle_store_error { super(name, value, options.merge!(raw: true)) } end end end diff --git a/lib/rack/attack/store_proxy/redis_cache_store_proxy.rb b/lib/rack/attack/store_proxy/redis_cache_store_proxy.rb index 74f665b5..fdd41bf5 100644 --- a/lib/rack/attack/store_proxy/redis_cache_store_proxy.rb +++ b/lib/rack/attack/store_proxy/redis_cache_store_proxy.rb @@ -17,25 +17,25 @@ def increment(name, amount = 1, **options) # 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) + handle_store_error { write(name, amount, options) } amount else - super + handle_store_error { super } end end end def read(name, options = {}) - super(name, options.merge!(raw: true)) + handle_store_error { super(name, options.merge!(raw: true)) } end def write(name, value, options = {}) - super(name, value, options.merge!(raw: true)) + handle_store_error { super(name, value, options.merge!(raw: true)) } end def delete_matched(matcher, options = nil) - super(matcher.source, options) + handle_store_error { super(matcher.source, options) } 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..2f199da9 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) } + handle_store_error { get(key, raw: true) } end def write(key, value, options = {}) if (expires_in = options[:expires_in]) - rescuing { setex(key, expires_in, value, raw: true) } + handle_store_error { setex(key, expires_in, value, raw: true) } else - rescuing { set(key, value, raw: true) } + handle_store_error { set(key, value, raw: true) } end end end diff --git a/spec/rack_attack_store_proxy_defaults_spec.rb b/spec/rack_attack_store_proxy_defaults_spec.rb new file mode 100644 index 00000000..2e3ed81e --- /dev/null +++ b/spec/rack_attack_store_proxy_defaults_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require_relative 'spec_helper' + +describe "Store proxy defaults (String-based)" do + it "RedisStoreProxy inherits Redis default bypass list from RedisProxy" do + skip 'redis gem not available' unless defined?(::Redis) + _(Rack::Attack::StoreProxy::RedisStoreProxy.default_bypassable_store_errors) + .must_equal ['Redis::BaseConnectionError'] + end + + it "RedisCacheStoreProxy has no default bypass list (preserves upstream behavior)" do + _(Rack::Attack::StoreProxy::RedisCacheStoreProxy.default_bypassable_store_errors).must_equal [] + end + + it "MemCacheStoreProxy has no default bypass list (preserves upstream behavior)" do + _(Rack::Attack::StoreProxy::MemCacheStoreProxy.default_bypassable_store_errors).must_equal [] + end + + it "BaseProxy base default is an empty Array" do + _(Rack::Attack::BaseProxy.default_bypassable_store_errors).must_equal [] + end + + it "default list entries are Strings so missing gems don't break loading" do + [ + Rack::Attack::StoreProxy::DalliProxy, + Rack::Attack::StoreProxy::RedisProxy + ].each do |proxy_class| + defaults = proxy_class.default_bypassable_store_errors + _(defaults.all? { |e| e.is_a?(String) }).must_equal( + true, + "#{proxy_class}: expected all defaults to be Strings, got #{defaults.inspect}" + ) + end + end +end