Merge branch 'main' into appraisal

.github/workflows/build.yml

@@ -27,6 +27,7 @@ jobs:
           - rack_3
           - rack_2
           - rack_1
+          - rails_7_1
           - rails_7_0
           - rails_6_1
           - rails_6_0
@@ -73,6 +74,10 @@ jobs:
             ruby: 2.6.10
           - gemfile: rails_7_0
             ruby: 2.5.8
+          - gemfile: rails_7_1
+            ruby: 2.6.10
+          - gemfile: rails_7_1
+            ruby: 2.5.8
     env:
       BUNDLE_GEMFILE: gemfiles/${{ matrix.gemfile }}.gemfile
     steps:

.rubocop.yml

@@ -1,5 +1,7 @@
 require:
+  - rubocop-minitest
   - rubocop-performance
+  - rubocop-rake
 
 inherit_mode:
   merge:
@@ -56,7 +58,6 @@ Security:
 
 Style/BlockDelimiters:
   Enabled: true
-  IgnoredMethods: [] # Workaround rubocop bug: https://github.com/rubocop-hq/rubocop/issues/6179
 
 Style/ClassAndModuleChildren:
   Enabled: true

Appraisals

@@ -21,6 +21,10 @@ appraise "rack_1" do
   gem "rack-test", ">= 0.6"
 end
 
+appraise 'rails_7-1' do
+  gem 'railties', '~> 7.1.0'
+end
+
 appraise 'rails_7-0' do
   gem 'railties', '~> 7.0.0'
 end

gemfiles/rails_7_1.gemfile

@@ -0,0 +1,7 @@
+# This file was generated by Appraisal
+
+source "https://rubygems.org"
+
+gem "railties", "~> 7.1.0"
+
+gemspec path: "../"

lib/rack/attack.rb

@@ -17,8 +17,11 @@
 module Rack
   class Attack
     class Error < StandardError; end
+
     class MisconfiguredStoreError < Error; end
+
     class MissingStoreError < Error; end
+
     class IncompatibleStoreError < Error; end
 
     autoload :Check,                'rack/attack/check'

lib/rack/attack/base_proxy.rb

@@ -11,6 +11,7 @@ def proxies
         end
 
         def inherited(klass)
+          super
           proxies << klass
         end
 

lib/rack/attack/configuration.rb

@@ -61,11 +61,15 @@ def blocklist(name = nil, &block)
       end
 
       def blocklist_ip(ip_address)
-        @anonymous_blocklists << Blocklist.new { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) }
+        @anonymous_blocklists << Blocklist.new do |request|
+          request.ip && !request.ip.empty? && IPAddr.new(ip_address).include?(IPAddr.new(request.ip))
+        end
       end
 
       def safelist_ip(ip_address)
-        @anonymous_safelists << Safelist.new { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) }
+        @anonymous_safelists << Safelist.new do |request|
+          request.ip && !request.ip.empty? && IPAddr.new(ip_address).include?(IPAddr.new(request.ip))
+        end
       end
 
       def throttle(name, options, &block)

rack-attack.gemspec

@@ -34,8 +34,10 @@ Gem::Specification.new do |s|
   s.add_development_dependency "minitest-stub-const", "~> 0.6"
   s.add_development_dependency 'rack-test', "~> 2.0"
   s.add_development_dependency 'rake', "~> 13.0"
-  s.add_development_dependency "rubocop", "0.89.1"
-  s.add_development_dependency "rubocop-performance", "~> 1.5.0"
+  s.add_development_dependency "rubocop", "1.12.1"
+  s.add_development_dependency "rubocop-minitest", "~> 0.11.1"
+  s.add_development_dependency "rubocop-performance", "~> 1.10.2"
+  s.add_development_dependency "rubocop-rake", "~> 0.5.1"
   s.add_development_dependency "timecop", "~> 0.9.1"
 
   # byebug only works with MRI

spec/acceptance/blocking_ip_spec.rb

@@ -19,6 +19,12 @@
     assert_equal 200, last_response.status
   end
 
+  it "succeeds if IP is missing" do
+    get "/", {}, "REMOTE_ADDR" => ""
+
+    assert_equal 200, last_response.status
+  end
+
   it "notifies when the request is blocked" do
     notified = false
     notification_type = nil

spec/acceptance/cache_store_config_for_fail2ban_spec.rb

@@ -79,7 +79,7 @@ def write(key, value); end
   end
 
   it "works with any object that responds to #read, #write and #increment" do
-    FakeStore = Class.new do
+    fake_store_class = Class.new do
       attr_accessor :backend
 
       def initialize
@@ -100,7 +100,7 @@ def increment(key, _count, _options = {})
       end
     end
 
-    Rack::Attack.cache.store = FakeStore.new
+    Rack::Attack.cache.store = fake_store_class.new
 
     get "/"
     assert_equal 200, last_response.status

spec/acceptance/extending_request_object_spec.rb

@@ -4,10 +4,8 @@
 
 describe "Extending the request object" do
   before do
-    class Rack::Attack::Request
-      def authorized?
-        env["APIKey"] == "private-secret"
-      end
+    Rack::Attack::Request.define_method :authorized? do
+      env["APIKey"] == "private-secret"
     end
 
     Rack::Attack.blocklist("unauthorized requests") do |request|
@@ -17,9 +15,7 @@ def authorized?
 
   # We don't want the extension to leak to other test cases
   after do
-    class Rack::Attack::Request
-      remove_method :authorized?
-    end
+    Rack::Attack::Request.undef_method :authorized?
   end
 
   it "forbids request if blocklist condition is true" do

spec/acceptance/safelisting_ip_spec.rb

@@ -17,6 +17,12 @@
     assert_equal 403, last_response.status
   end
 
+  it "forbids request if blocklist condition is true and safelist is false (missing IP)" do
+    get "/admin", {}, "REMOTE_ADDR" => ""
+
+    assert_equal 403, last_response.status
+  end
+
   it "succeeds if blocklist condition is false and safelist is false" do
     get "/", {}, "REMOTE_ADDR" => "1.2.3.4"
 

spec/rack_attack_request_spec.rb

@@ -5,10 +5,8 @@
 describe 'Rack::Attack' do
   describe 'helpers' do
     before do
-      class Rack::Attack::Request
-        def remote_ip
-          ip
-        end
+      Rack::Attack::Request.define_method :remote_ip do
+        ip
       end
 
       Rack::Attack.safelist('valid IP') do |req|

spec/rack_attack_track_spec.rb

@@ -3,17 +3,19 @@
 require_relative 'spec_helper'
 
 describe 'Rack::Attack.track' do
-  class Counter
-    def self.incr
-      @counter += 1
-    end
+  let(:counter_class) do
+    Class.new do
+      def self.incr
+        @counter += 1
+      end
 
-    def self.reset
-      @counter = 0
-    end
+      def self.reset
+        @counter = 0
+      end
 
-    def self.check
-      @counter
+      def self.check
+        @counter
+      end
     end
   end
 
@@ -32,19 +34,19 @@ def self.check
 
   describe "with a notification subscriber and two tracks" do
     before do
-      Counter.reset
+      counter_class.reset
       # A second track
       Rack::Attack.track("homepage") { |req| req.path == "/" }
 
       ActiveSupport::Notifications.subscribe("track.rack_attack") do |*_args|
-        Counter.incr
+        counter_class.incr
       end
 
       get "/"
     end
 
     it "should notify twice" do
-      _(Counter.check).must_equal 2
+      _(counter_class.check).must_equal 2
     end
   end