[HttpFoundation] refactored code to avoid code duplication

symfony · Apr 20, 2013 · e7c1696 · e7c1696
1 parent 1695067
commit e7c1696
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 19 deletions.
diff --git a/src/Symfony/Component/HttpFoundation/IpUtils.php b/src/Symfony/Component/HttpFoundation/IpUtils.php
@@ -26,18 +26,26 @@ private function __construct() {}
     /**
      * Validates an IPv4 or IPv6 address.
      *
-     * @param string $requestIp
-     * @param string $ip
+     * @param string       $requestIp
+     * @param string|array $ips
      *
      * @return boolean Whether the IP is valid
      */
-    public static function checkIp($requestIp, $ip)
+    public static function checkIp($requestIp, $ips)
     {
-        if (false !== strpos($requestIp, ':')) {
-            return self::checkIp6($requestIp, $ip);
+        if (!is_array($ips)) {
+            $ips = array($ips);
+        }
+
+        $method = false !== strpos($requestIp, ':') ? 'checkIp6': 'checkIp4';
+
+        foreach ($ips as $ip) {
+            if (self::$method($requestIp, $ip)) {
+                return true;
+            }
         }
 
-        return self::checkIp4($requestIp, $ip);
+        return false;
     }
 
     /**

diff --git a/src/Symfony/Component/HttpFoundation/Request.php b/src/Symfony/Component/HttpFoundation/Request.php
@@ -690,12 +690,10 @@ public function getClientIps()
         $ip = $clientIps[0];
 
         foreach ($clientIps as $key => $clientIp) {
-            foreach ($trustedProxies as $trustedProxy) {
-                if (IpUtils::checkIp($clientIp, $trustedProxy)) {
-                    unset($clientIps[$key]);
+            if (IpUtils::checkIp($clientIp, $trustedProxies)) {
+                unset($clientIps[$key]);
 
-                    continue 2;
-                }
+                continue;
             }
         }
 

diff --git a/src/Symfony/Component/HttpFoundation/RequestMatcher.php b/src/Symfony/Component/HttpFoundation/RequestMatcher.php
@@ -153,10 +153,8 @@ public function matches(Request $request)
             return false;
         }
 
-        foreach ($this->ips as $ip) {
-            if (IpUtils::checkIp($request->getClientIp(), $ip)) {
-                return true;
-            }
+        if (IpUtils::checkIp($request->getClientIp(), $this->ips)) {
+            return true;
         }
 
         // Note to future implementors: add additional checks above the

diff --git a/src/Symfony/Component/HttpFoundation/Tests/IpUtilsTest.php b/src/Symfony/Component/HttpFoundation/Tests/IpUtilsTest.php
@@ -31,6 +31,9 @@ public function testIpv4Provider()
             array(true, '192.168.1.1', '192.168.1.0/24'),
             array(false, '192.168.1.1', '1.2.3.4/1'),
             array(false, '192.168.1.1', '192.168.1/33'),
+            array(true, '192.168.1.1', array('1.2.3.4/1', '192.168.1.0/24')),
+            array(true, '192.168.1.1', array('192.168.1.0/24', '1.2.3.4/1')),
+            array(false, '192.168.1.1', array('1.2.3.4/1', '4.3.2.1/1')),
         );
     }
 
@@ -54,6 +57,9 @@ public function testIpv6Provider()
             array(false, '2a01:198:603:0:396e:4789:8e99:890f', '::1'),
             array(true, '0:0:0:0:0:0:0:1', '::1'),
             array(false, '0:0:603:0:396e:4789:8e99:0001', '::1'),
+            array(true, '2a01:198:603:0:396e:4789:8e99:890f', array('::1', '2a01:198:603:0::/65')),
+            array(true, '2a01:198:603:0:396e:4789:8e99:890f', array('2a01:198:603:0::/65', '::1')),
+            array(false, '2a01:198:603:0:396e:4789:8e99:890f', array('::1', '1a01:198:603:0::/65')),
         );
     }
 

diff --git a/src/Symfony/Component/HttpKernel/EventListener/FragmentListener.php b/src/Symfony/Component/HttpKernel/EventListener/FragmentListener.php
@@ -80,10 +80,8 @@ protected function validateRequest(Request $request)
         // does the Request come from a trusted IP?
         $trustedIps = array_merge($this->getLocalIpAddresses(), $request->getTrustedProxies());
         $remoteAddress = $request->server->get('REMOTE_ADDR');
-        foreach ($trustedIps as $ip) {
-            if (IpUtils::checkIp($remoteAddress, $ip)) {
-                return;
-            }
+        if (IpUtils::checkIp($remoteAddress, $trustedIps)) {
+            return;
         }
 
         // is the Request signed?