make sure to return correct ip when all IPs are excluded

core/IP.php

@@ -101,7 +101,7 @@ class IP
      *
      * @param string $csv Comma separated list of elements.
      * @param array $excludedIps Optional list of excluded IP addresses (or IP address ranges).
-     * @return string Last (non-excluded) IP address in the list.
+     * @return string Last (non-excluded) IP address in the list or an empty string if all given IPs are excluded.
      */
     public static function getLastIpFromList($csv, $excludedIps = null)
     {
@@ -115,6 +115,8 @@ class IP
                     return $element;
                 }
             }
+
+            return '';
         }
         return trim(Common::sanitizeInputValue($csv));
     }

tests/PHPUnit/Unit/IPTest.php

@@ -10,11 +10,12 @@
 
 namespace Piwik\Tests\Unit;
 
-use Piwik\Common;
 use Piwik\Config;
 use Piwik\IP;
-use Piwik\Tests\Framework\Mock\TestConfig;
 
+/**
+ * @group Core
+ */
 class IPTest extends \PHPUnit_Framework_TestCase
 {
     /**
@@ -83,7 +84,6 @@ class IPTest extends \PHPUnit_Framework_TestCase
 
     /**
      * @dataProvider getIpFromHeaderTestData
-     * @group Core
      */
     public function testGetIpFromHeader($description, $test)
     {
@@ -111,8 +111,6 @@ class IPTest extends \PHPUnit_Framework_TestCase
     }
 
     /**
-     * @group Core
-     *
      * @dataProvider getIpTestData
      */
     public function testGetNonProxyIpFromHeader($ip)
@@ -121,8 +119,6 @@ class IPTest extends \PHPUnit_Framework_TestCase
     }
 
     /**
-     * @group Core
-     *
      * @dataProvider getIpTestData
      */
     public function testGetNonProxyIpFromHeader2($ip)
@@ -134,8 +130,6 @@ class IPTest extends \PHPUnit_Framework_TestCase
     }
 
     /**
-     * @group Core
-     *
      * @dataProvider getIpTestData
      */
     public function testGetNonProxyIpFromHeader3($ip)
@@ -154,6 +148,20 @@ class IPTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals($ip, IP::getNonProxyIpFromHeader('1.1.1.1', array('HTTP_X_FORWARDED_FOR')));
     }
 
+    /**
+     * See https://github.com/piwik/piwik/issues/8721
+     */
+    public function testGetNonProxyIpFromHeader4_ShouldReturnDefaultIp_IfDefaultIpIsGivenMultipleTimes()
+    {
+        // 1.1.1.1 is a trusted proxy
+        $_SERVER['REMOTE_ADDR'] = '1.1.1.1';
+        $_SERVER['HTTP_X_FORWARDED_FOR'] = $_SERVER['REMOTE_ADDR'];
+        $_SERVER['HTTP_CF_CONNECTING_IP'] = $_SERVER['REMOTE_ADDR'];
+
+        $this->assertEquals('1.1.1.1', IP::getNonProxyIpFromHeader('1.1.1.1', array('HTTP_X_FORWARDED_FOR', 'HTTP_CF_CONNECTING_IP')));
+        unset($_SERVER['HTTP_CF_CONNECTING_IP']);
+    }
+
     /**
      * Dataprovider for testGetLastIpFromList
      */
@@ -170,8 +178,6 @@ class IPTest extends \PHPUnit_Framework_TestCase
     }
 
     /**
-     * @group Core
-     *
      * @dataProvider getLastIpFromListTestData
      */
     public function testGetLastIpFromList($csv, $expected)
@@ -182,4 +188,10 @@ class IPTest extends \PHPUnit_Framework_TestCase
         // with excluded Ips
         $this->assertEquals($expected, IP::getLastIpFromList($csv . ', 10.10.10.10', array('10.10.10.10')));
     }
+
+    public function testGetLastIpFromList_shouldReturnAnEmptyString_IfMultipleIpsAreGivenButAllAreExcluded()
+    {
+        // with excluded Ips
+        $this->assertEquals('', IP::getLastIpFromList('10.10.10.10, 10.10.10.10', array('10.10.10.10')));
+    }
 }