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 <[email protected]>
rack · Oct 18, 2023 · 3543f61 · 3543f61
1 parent e9f4727
commit 3543f61
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 2 deletions.
diff --git a/lib/rack/attack/configuration.rb b/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)

diff --git a/spec/acceptance/blocking_ip_spec.rb b/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

diff --git a/spec/acceptance/safelisting_ip_spec.rb b/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"