Rework Request#ip to handle empty forwarded_for.

lib/rack/request.rb

@@ -352,16 +352,26 @@ def ssl?
       end
 
       def ip
-        remote_addrs = split_header(get_header('REMOTE_ADDR'))
-        remote_addrs = reject_trusted_ip_addresses(remote_addrs)
+        remote_addresses = split_header(get_header('REMOTE_ADDR'))
+        external_addresses = reject_trusted_ip_addresses(remote_addresses)
 
-        if remote_addrs.any?
-          remote_addrs.first
-        else
-          forwarded_ips = self.forwarded_for
+        unless external_addresses.empty?
+          return external_addresses.first
+        end
 
-          reject_trusted_ip_addresses(forwarded_ips).last || forwarded_ips.first || get_header("REMOTE_ADDR")
+        if forwarded_for = self.forwarded_for
+          unless forwarded_for.empty?
+            # The forwarded for addresses are ordered: client, proxy1, proxy2.
+            # So we reject all the trusted addresses (proxy*) and return the
+            # last client. Or if we trust everyone, we just return the first
+            # address.
+            return reject_trusted_ip_addresses(forwarded_for).last || forwarded_for.first
+          end
         end
+
+        # If all the addresses are trusted, and we aren't forwarded, just return
+        # the first remote address, which represents the source of the request.
+        remote_addresses.first
       end
 
       # The media type (type/subtype) portion of the CONTENT_TYPE header

test/spec_request.rb

@@ -1270,6 +1270,12 @@ def ip_app
 
     res = mock.get '/', 'REMOTE_ADDR' => '1.2.3.4,3.4.5.6'
     res.body.must_equal '1.2.3.4'
+
+    res = mock.get '/', 'REMOTE_ADDR' => '127.0.0.1'
+    res.body.must_equal '127.0.0.1'
+
+    res = mock.get '/', 'REMOTE_ADDR' => '127.0.0.1,127.0.0.1'
+    res.body.must_equal '127.0.0.1'
   end
 
   it 'deals with proxies' do