From 3543f61b640555f161b6d7f76fdbd622ead32dd3 Mon Sep 17 00:00:00 2001
From: Santiago Bartesaghi <sbartesaghi@hotmail.com>
Date: Tue, 17 Oct 2023 21:19:29 -0300
Subject: [PATCH] Fix #588 don't fail if request.ip is missing (#630)

* Fix #588 don't fail if request.ip is missing

* Fix Rails 4 suite

* Improve tests

---------

Co-authored-by: Gonzalo <456459+grzuy@users.noreply.github.com>
---
 lib/rack/attack/configuration.rb       | 8 ++++++--
 spec/acceptance/blocking_ip_spec.rb    | 6 ++++++
 spec/acceptance/safelisting_ip_spec.rb | 6 ++++++
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/lib/rack/attack/configuration.rb b/lib/rack/attack/configuration.rb
index c90fdee..a4bdc98 100644
--- a/lib/rack/attack/configuration.rb
+++ b/lib/rack/attack/configuration.rb
@@ -61,11 +61,15 @@ module Rack
       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)
diff --git a/spec/acceptance/blocking_ip_spec.rb b/spec/acceptance/blocking_ip_spec.rb
index 102a8fc..4d08042 100644
--- a/spec/acceptance/blocking_ip_spec.rb
+++ b/spec/acceptance/blocking_ip_spec.rb
@@ -19,6 +19,12 @@ describe "Blocking an IP" do
     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
diff --git a/spec/acceptance/safelisting_ip_spec.rb b/spec/acceptance/safelisting_ip_spec.rb
index c0b8faf..fd80dc1 100644
--- a/spec/acceptance/safelisting_ip_spec.rb
+++ b/spec/acceptance/safelisting_ip_spec.rb
@@ -17,6 +17,12 @@ describe "Safelist an IP" do
     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"