Merge branch 'main' into multiple-instances

.github/dependabot.yml

@@ -0,0 +1,7 @@
+version: 2
+
+updates:
+  - package-ecosystem: "github-actions"
+    directory: "/"
+    schedule:
+      interval: "weekly"

.github/workflows/build.yml

@@ -17,6 +17,7 @@ jobs:
     strategy:
       matrix:
         ruby:
+          - '3.3'
           - '3.2'
           - '3.1'
           - '3.0'
@@ -46,6 +47,14 @@ jobs:
           - active_support_5_redis_cache_store_pooled
           - redis_store
         exclude:
+          - gemfile: rails_5_2
+            ruby: '3.3'
+          - gemfile: active_support_5_redis_cache_store
+            ruby: '3.3'
+          - gemfile: active_support_5_redis_cache_store_pooled
+            ruby: '3.3'
+          - gemfile: dalli2
+            ruby: '3.3'
           - gemfile: rails_5_2
             ruby: '3.2'
           - gemfile: active_support_5_redis_cache_store
@@ -97,7 +106,7 @@ jobs:
     env:
       BUNDLE_GEMFILE: gemfiles/${{ matrix.gemfile }}.gemfile
     steps:
-    - uses: actions/checkout@v2
+    - uses: actions/checkout@v4
     - uses: ruby/setup-ruby@v1
       with:
         ruby-version: ${{ matrix.ruby }}

spec/acceptance/cache_store_config_for_allow2ban_spec.rb

@@ -12,9 +12,11 @@
     end
   end
 
-  it "gives semantic error if no store was configured" do
-    assert_raises(Rack::Attack::MissingStoreError) do
-      get "/scarce-resource"
+  unless defined?(Rails)
+    it "gives semantic error if no store was configured" do
+      assert_raises(Rack::Attack::MissingStoreError) do
+        get "/scarce-resource"
+      end
     end
   end
 

spec/acceptance/cache_store_config_for_fail2ban_spec.rb

@@ -12,9 +12,11 @@
     end
   end
 
-  it "gives semantic error if no store was configured" do
-    assert_raises(Rack::Attack::MissingStoreError) do
-      get "/private-place"
+  unless defined?(Rails)
+    it "gives semantic error if no store was configured" do
+      assert_raises(Rack::Attack::MissingStoreError) do
+        get "/private-place"
+      end
     end
   end
 

spec/acceptance/cache_store_config_for_throttle_spec.rb

@@ -9,9 +9,11 @@
     end
   end
 
-  it "gives semantic error if no store was configured" do
-    assert_raises(Rack::Attack::MissingStoreError) do
-      get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
+  unless defined?(Rails)
+    it "gives semantic error if no store was configured" do
+      assert_raises(Rack::Attack::MissingStoreError) do
+        get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
+      end
     end
   end
 

spec/acceptance/cache_store_config_with_rails_spec.rb

@@ -11,10 +11,12 @@
     end
   end
 
-  it "fails when Rails.cache is not set" do
-    Object.stub_const(:Rails, OpenStruct.new(cache: nil)) do
-      assert_raises(Rack::Attack::MissingStoreError) do
-        get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
+  unless defined?(Rails)
+    it "fails when Rails.cache is not set" do
+      Object.stub_const(:Rails, OpenStruct.new(cache: nil)) do
+        assert_raises(Rack::Attack::MissingStoreError) do
+          get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
+        end
       end
     end
   end

spec/acceptance/stores/active_support_dalli_store_spec.rb

@@ -9,7 +9,6 @@
 if should_run
   require_relative "../../support/cache_store_helper"
   require "active_support/cache/dalli_store"
-  require "timecop"
 
   describe "ActiveSupport::Cache::DalliStore as a cache backend" do
     before do

spec/acceptance/stores/active_support_mem_cache_store_pooled_spec.rb

@@ -4,7 +4,6 @@
 
 if defined?(::ConnectionPool) && defined?(::Dalli)
   require_relative "../../support/cache_store_helper"
-  require "timecop"
 
   describe "ActiveSupport::Cache::MemCacheStore (pooled) as a cache backend" do
     before do

spec/acceptance/stores/active_support_mem_cache_store_spec.rb

@@ -4,7 +4,6 @@
 
 if defined?(::Dalli)
   require_relative "../../support/cache_store_helper"
-  require "timecop"
 
   describe "ActiveSupport::Cache::MemCacheStore as a cache backend" do
     before do

spec/acceptance/stores/active_support_memory_store_spec.rb

@@ -3,8 +3,6 @@
 require_relative "../../spec_helper"
 require_relative "../../support/cache_store_helper"
 
-require "timecop"
-
 describe "ActiveSupport::Cache::MemoryStore as a cache backend" do
   before do
     Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new

spec/acceptance/stores/active_support_redis_cache_store_pooled_spec.rb

@@ -10,7 +10,6 @@
 
 if should_run
   require_relative "../../support/cache_store_helper"
-  require "timecop"
 
   describe "ActiveSupport::Cache::RedisCacheStore (pooled) as a cache backend" do
     before do

spec/acceptance/stores/active_support_redis_cache_store_spec.rb

@@ -9,7 +9,6 @@
 
 if should_run
   require_relative "../../support/cache_store_helper"
-  require "timecop"
 
   describe "ActiveSupport::Cache::RedisCacheStore as a cache backend" do
     before do

spec/acceptance/stores/connection_pool_dalli_client_spec.rb

@@ -6,7 +6,6 @@
   require_relative "../../support/cache_store_helper"
   require "connection_pool"
   require "dalli"
-  require "timecop"
 
   describe "ConnectionPool with Dalli::Client as a cache backend" do
     before do

spec/acceptance/stores/dalli_client_spec.rb

@@ -5,7 +5,6 @@
 if defined?(::Dalli)
   require_relative "../../support/cache_store_helper"
   require "dalli"
-  require "timecop"
 
   describe "Dalli::Client as a cache backend" do
     before do

spec/acceptance/stores/redis_spec.rb

@@ -4,7 +4,6 @@
 
 if defined?(::Redis)
   require_relative "../../support/cache_store_helper"
-  require "timecop"
 
   describe "Plain redis as a cache backend" do
     before do

spec/acceptance/stores/redis_store_spec.rb

@@ -4,8 +4,6 @@
 require_relative "../../support/cache_store_helper"
 
 if defined?(::Redis::Store)
-  require "timecop"
-
   describe "Redis::Store as a cache backend" do
     before do
       Rack::Attack.cache.store = ::Redis::Store.new

spec/rack_attack_throttle_spec.rb

@@ -1,10 +1,11 @@
 # frozen_string_literal: true
 
 require_relative 'spec_helper'
+require_relative 'support/freeze_time_helper'
 
 describe 'Rack::Attack.throttle' do
   before do
-    @period = 60 # Use a long period; failures due to cache key rotation less likely
+    @period = 60
     Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
     Rack::Attack.throttle('ip/sec', limit: 1, period: @period) { |req| req.ip }
   end
@@ -14,14 +15,18 @@
   it_allows_ok_requests
 
   describe 'a single request' do
-    before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
-
     it 'should set the counter for one request' do
-      key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
-      _(Rack::Attack.cache.store.read(key)).must_equal 1
+      within_same_period do
+        get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
+
+        key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
+        _(Rack::Attack.cache.store.read(key)).must_equal 1
+      end
     end
 
     it 'should populate throttle data' do
+      get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
+
       data = {
         count: 1,
         limit: 1,
@@ -36,7 +41,9 @@
 
   describe "with 2 requests" do
     before do
-      2.times { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
+      within_same_period do
+        2.times { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
+      end
     end
 
     it 'should block the last request' do
@@ -62,22 +69,25 @@
 
 describe 'Rack::Attack.throttle with limit as proc' do
   before do
-    @period = 60 # Use a long period; failures due to cache key rotation less likely
+    @period = 60
     Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
     Rack::Attack.throttle('ip/sec', limit: lambda { |_req| 1 }, period: @period) { |req| req.ip }
   end
 
   it_allows_ok_requests
 
   describe 'a single request' do
-    before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
-
     it 'should set the counter for one request' do
-      key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
-      _(Rack::Attack.cache.store.read(key)).must_equal 1
+      within_same_period do
+        get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
+
+        key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
+        _(Rack::Attack.cache.store.read(key)).must_equal 1
+      end
     end
 
     it 'should populate throttle data' do
+      get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
       data = {
         count: 1,
         limit: 1,
@@ -93,22 +103,26 @@
 
 describe 'Rack::Attack.throttle with period as proc' do
   before do
-    @period = 60 # Use a long period; failures due to cache key rotation less likely
+    @period = 60
     Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
     Rack::Attack.throttle('ip/sec', limit: lambda { |_req| 1 }, period: lambda { |_req| @period }) { |req| req.ip }
   end
 
   it_allows_ok_requests
 
   describe 'a single request' do
-    before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
-
     it 'should set the counter for one request' do
-      key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
-      _(Rack::Attack.cache.store.read(key)).must_equal 1
+      within_same_period do
+        get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
+
+        key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
+        _(Rack::Attack.cache.store.read(key)).must_equal 1
+      end
     end
 
     it 'should populate throttle data' do
+      get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
+
       data = {
         count: 1,
         limit: 1,
@@ -122,7 +136,7 @@
   end
 end
 
-describe 'Rack::Attack.throttle with block retuning nil' do
+describe 'Rack::Attack.throttle with block returning nil' do
   before do
     @period = 60
     Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new
@@ -132,14 +146,17 @@
   it_allows_ok_requests
 
   describe 'a single request' do
-    before { get '/', {}, 'REMOTE_ADDR' => '1.2.3.4' }
-
     it 'should not set the counter' do
-      key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
-      assert_nil Rack::Attack.cache.store.read(key)
+      within_same_period do
+        get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
+
+        key = "rack::attack:#{Time.now.to_i / @period}:ip/sec:1.2.3.4"
+        assert_nil Rack::Attack.cache.store.read(key)
+      end
     end
 
     it 'should not populate throttle data' do
+      get '/', {}, 'REMOTE_ADDR' => '1.2.3.4'
       assert_nil last_request.env['rack.attack.throttle_data']
     end
   end
@@ -162,20 +179,24 @@
   end
 
   it 'should not differentiate requests when throttle_discriminator_normalizer is enabled' do
-    post_logins
-    key = "rack::attack:#{Time.now.to_i / @period}:logins/email:[email protected]"
-    _(Rack::Attack.cache.store.read(key)).must_equal 3
+    within_same_period do
+      post_logins
+      key = "rack::attack:#{Time.now.to_i / @period}:logins/email:[email protected]"
+      _(Rack::Attack.cache.store.read(key)).must_equal 3
+    end
   end
 
   it 'should differentiate requests when throttle_discriminator_normalizer is disabled' do
     begin
       prev = Rack::Attack.throttle_discriminator_normalizer
       Rack::Attack.throttle_discriminator_normalizer = nil
 
-      post_logins
-      @emails.each do |email|
-        key = "rack::attack:#{Time.now.to_i / @period}:logins/email:#{email}"
-        _(Rack::Attack.cache.store.read(key)).must_equal 1
+      within_same_period do
+        post_logins
+        @emails.each do |email|
+          key = "rack::attack:#{Time.now.to_i / @period}:logins/email:#{email}"
+          _(Rack::Attack.cache.store.read(key)).must_equal 1
+        end
       end
     ensure
       Rack::Attack.throttle_discriminator_normalizer = prev

spec/spec_helper.rb

@@ -20,14 +20,15 @@ def safe_require(name)
 
 safe_require "connection_pool"
 safe_require "dalli"
+safe_require "rails"
 safe_require "redis"
 safe_require "redis-store"
 
 class Minitest::Spec
   include Rack::Test::Methods
 
   before do
-    if Object.const_defined?(:Rails) && Rails.respond_to?(:cache)
+    if Object.const_defined?(:Rails) && Rails.respond_to?(:cache) && Rails.cache.respond_to?(:clear)
       Rails.cache.clear
     end
   end