Improve documentation of X-Forwarded-For header handling

symfony · Oct 16, 2013 · 0d232ba · 0d232ba
1 parent 9ed5608
commit 0d232ba
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 14 deletions.
diff --git a/src/Symfony/Component/HttpFoundation/IpUtils.php b/src/Symfony/Component/HttpFoundation/IpUtils.php
@@ -24,10 +24,10 @@ class IpUtils
     private function __construct() {}
 
     /**
-     * Validates an IPv4 or IPv6 address.
+     * Checks if an IPv4 or IPv6 address is contained in the list of given IPs or subnets
      *
-     * @param string       $requestIp
-     * @param string|array $ips
+     * @param string       $requestIp   IP to check
+     * @param string|array $ips         List of IPs or subnets (can be a string if only a single one)
      *
      * @return boolean Whether the IP is valid
      */
@@ -49,10 +49,11 @@ public static function checkIp($requestIp, $ips)
     }
 
     /**
-     * Validates an IPv4 address.
+     * Compares two IPv4 addresses.
+     * In case a subnet is given, it checks if it contains the request IP.
      *
-     * @param string $requestIp
-     * @param string $ip
+     * @param string $requestIp IPv4 address to check
+     * @param string $ip        IPv4 address or subnet in CIDR notation
      *
      * @return boolean Whether the IP is valid
      */
@@ -73,13 +74,14 @@ public static function checkIp4($requestIp, $ip)
     }
 
     /**
-     * Validates an IPv6 address.
+     * Compares two IPv6 addresses.
+     * In case a subnet is given, it checks if it contains the request IP.
      *
      * @author David Soria Parra <dsp at php dot net>
      * @see https://github.com/dsp/v6tools
      *
-     * @param string $requestIp
-     * @param string $ip
+     * @param string $requestIp IPv6 address to check
+     * @param string $ip        IPv6 address or subnet in CIDR notation
      *
      * @return boolean Whether the IP is valid
      *

diff --git a/src/Symfony/Component/HttpFoundation/Request.php b/src/Symfony/Component/HttpFoundation/Request.php
@@ -732,9 +732,9 @@ public function setSession(SessionInterface $session)
     /**
      * Returns the client IP addresses.
      *
-     * The least trusted IP address is first, and the most trusted one last.
-     * The "real" client IP address is the first one, but this is also the
-     * least trusted one.
+     * In the returned array the most trusted IP address is first, and the
+     * least trusted one last. The "real" client IP address is the last one,
+     * but this is also the least trusted one. Trusted proxies are stripped.
      *
      * Use this method carefully; you should use getClientIp() instead.
      *
@@ -755,17 +755,19 @@ public function getClientIps()
         }
 
         $clientIps = array_map('trim', explode(',', $this->headers->get(self::$trustedHeaders[self::HEADER_CLIENT_IP])));
-        $clientIps[] = $ip;
+        $clientIps[] = $ip; // Complete the IP chain with the IP the request actually came from
 
         $trustedProxies = !self::$trustedProxies ? array($ip) : self::$trustedProxies;
-        $ip = $clientIps[0];
+        $ip = $clientIps[0]; // Fallback to this when the client IP falls into the range of trusted proxies
 
+        // Eliminate all IPs from the forwarded IP chain which are trusted proxies
         foreach ($clientIps as $key => $clientIp) {
             if (IpUtils::checkIp($clientIp, $trustedProxies)) {
                 unset($clientIps[$key]);
             }
         }
 
+        // Now the IP chain contains only untrusted proxies and the client IP
         return $clientIps ? array_reverse($clientIps) : array($ip);
     }