Merge pull request #3631 from indirect/remote_ip

Force calculation of remote IP before using || operator

2 files changed, 15 insertions, 8 deletions

diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb
index 69ca050d0c..b10f6b48c7 100644
--- a/actionpack/lib/action_dispatch/http/request.rb
+++ b/actionpack/lib/action_dispatch/http/request.rb
@@ -157,7 +157,8 @@ module ActionDispatch
 
     # Originating IP address, usually set by the RemoteIp middleware.
     def remote_ip
-      @remote_ip ||= (@env["action_dispatch.remote_ip"] || ip).to_s
+      # Coerce the remote_ip object into a string, because to_s could return nil
+      @remote_ip ||= @env["action_dispatch.remote_ip"].to_s || ip
     end
 
     # Returns the unique request id, which is based off either the X-Request-Id header that can
diff --git a/actionpack/lib/action_dispatch/middleware/remote_ip.rb b/actionpack/lib/action_dispatch/middleware/remote_ip.rb
index 58e25aed5a..ee0d19a50d 100644
--- a/actionpack/lib/action_dispatch/middleware/remote_ip.rb
+++ b/actionpack/lib/action_dispatch/middleware/remote_ip.rb
@@ -13,16 +13,16 @@ module ActionDispatch
        )\.
     }x
 
-    attr_reader :check_ip_spoofing, :trusted_proxies
+    attr_reader :check_ip, :proxies
 
     def initialize(app, check_ip_spoofing = true, custom_proxies = nil)
       @app = app
-      @check_ip_spoofing = check_ip_spoofing
+      @check_ip = check_ip_spoofing
       if custom_proxies
         custom_regexp = Regexp.new(custom_proxies)
-        @trusted_proxies = Regexp.union(TRUSTED_PROXIES, custom_regexp)
+        @proxies = Regexp.union(TRUSTED_PROXIES, custom_regexp)
       else
-        @trusted_proxies = TRUSTED_PROXIES
+        @proxies = TRUSTED_PROXIES
       end
     end
 
@@ -42,12 +42,12 @@ module ActionDispatch
       # HTTP_X_FORWARDED_FOR may be a comma-delimited list in the case of
       # multiple chained proxies. The last address which is not a known proxy
       # will be the originating IP.
-      def to_s
+      def calculate_ip
         client_ip     = @env['HTTP_CLIENT_IP']
         forwarded_ips = ips_from('HTTP_X_FORWARDED_FOR')
         remote_addrs  = ips_from('REMOTE_ADDR')
 
-        check_ip = client_ip && @middleware.check_ip_spoofing
+        check_ip = client_ip && @middleware.check_ip
         if check_ip && !forwarded_ips.include?(client_ip)
           # We don't know which came from the proxy, and which from the user
           raise IpSpoofAttackError, "IP spoofing attack?!" \
@@ -58,11 +58,17 @@ module ActionDispatch
         client_ip || forwarded_ips.last || remote_addrs.first
       end
 
+      def to_s
+        return @ip if @calculated_ip
+        @calculated_ip = true
+        @ip = calculate_ip
+      end
+
     protected
 
       def ips_from(header)
         ips = @env[header] ? @env[header].strip.split(/[,\s]+/) : []
-        ips.reject{|ip| ip =~ @middleware.trusted_proxies }
+        ips.reject{|ip| ip =~ @middleware.proxies }
       end
     end