From f66aa80a537288d263dab530bbd6f4d873618b60 Mon Sep 17 00:00:00 2001
From: Jean Boussier <jean.boussier@gmail.com>
Date: Fri, 20 Mar 2026 14:34:35 +0100
Subject: [PATCH] Eagerly check for store validity

Ref: https://github.com/rack/rack-attack/pull/315

`respond_to?` is a relatively expensive method that doesn't
benefit from inline caching, so it has to keep looking up
the method every time. Hence it's best to avoid it in hotspots.

In this case, `@store` can only change through the `store=` method,
so we might as well eagerly validate it there.

First because this way it's a one time fixed cost, but also because
this way the error is raised early during boot/configuration rather
than at runtime where it has availability impact.
---
 lib/rack/attack/cache.rb                      | 30 ++++-----
 .../cache_store_config_for_allow2ban_spec.rb  | 67 +++----------------
 .../cache_store_config_for_fail2ban_spec.rb   | 67 +++----------------
 .../cache_store_config_for_throttle_spec.rb   | 19 ++++--
 spec/rack_attack_reset_spec.rb                | 18 ++++-
 5 files changed, 60 insertions(+), 141 deletions(-)

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