diff --git a/config/global.ini.php b/config/global.ini.php
index edf9b867a26a01dc6af2af9558b631d655dd2126..1cdf1cb302356b30b55829bb3014c8fb61090afd 100644
--- a/config/global.ini.php
+++ b/config/global.ini.php
@@ -443,6 +443,7 @@ multisites_refresh_after_seconds = 300
assume_secure_protocol = 0
; List of proxy headers for client IP addresses
+; Piwik will determine the user IP by extracting the first IP address found in this proxy header.
;
; CloudFlare (CF-Connecting-IP)
;proxy_client_headers[] = HTTP_CF_CONNECTING_IP
diff --git a/core/IP.php b/core/IP.php
index 919e5483533d5d711be4f763be1d4130b90981fe..9f584b1761c995707e37bcb9a77008256920ede7 100644
--- a/core/IP.php
+++ b/core/IP.php
@@ -86,7 +86,7 @@ class IP
// this may be buggy if someone has proxy IPs and proxy host headers configured as
// `$_SERVER[$proxyHeader]` could be eg $_SERVER['HTTP_X_FORWARDED_HOST'] and
// include an actual host name, not an IP
- $proxyIp = self::getLastIpFromList($_SERVER[$proxyHeader], $proxyIps);
+ $proxyIp = self::getFirstIpFromList($_SERVER[$proxyHeader], $proxyIps);
if (strlen($proxyIp) && stripos($proxyIp, 'unknown') === false) {
return $proxyIp;
}
@@ -103,13 +103,16 @@ class IP
* @param array $excludedIps Optional list of excluded IP addresses (or IP address ranges).
* @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)
+ public static function getFirstIpFromList($csv, $excludedIps = null)
{
$p = strrpos($csv, ',');
if ($p !== false) {
$elements = explode(',', $csv);
- for ($i = count($elements); $i--;) {
- $element = trim(Common::sanitizeInputValue($elements[$i]));
+ foreach ($elements as $ipString) {
+ $element = trim(Common::sanitizeInputValue($ipString));
+ if(empty($element)) {
+ continue;
+ }
$ip = \Piwik\Network\IP::fromStringIP(IPUtils::sanitizeIp($element));
if (empty($excludedIps) || (!in_array($element, $excludedIps) && !$ip->isInRanges($excludedIps))) {
return $element;
diff --git a/tests/PHPUnit/Unit/IPTest.php b/tests/PHPUnit/Unit/IPTest.php
index 139687f619c200554cf29169757f248cd815cc29..0b6ca8b026fe7f709365e72d1786832ac0b6b2e6 100644
--- a/tests/PHPUnit/Unit/IPTest.php
+++ b/tests/PHPUnit/Unit/IPTest.php
@@ -74,7 +74,7 @@ class IPTest extends \PHPUnit_Framework_TestCase
array('localhost inside LAN', array('127.0.0.1', '', null, null, '127.0.0.1')),
array('outside LAN, no proxy', array('128.252.135.4', '', null, null, '128.252.135.4')),
array('outside LAN, no (trusted) proxy', array('128.252.135.4', '137.18.2.13, 128.252.135.4', '', null, '128.252.135.4')),
- array('outside LAN, one trusted proxy', array('192.168.1.10', '137.18.2.13, 128.252.135.4, 192.168.1.10', 'HTTP_X_FORWARDED_FOR', null, '128.252.135.4')),
+ array('outside LAN, one trusted proxy', array('137.18.2.13', '137.18.2.13, 128.252.135.4, 192.168.1.10', 'HTTP_X_FORWARDED_FOR', null, '128.252.135.4')),
array('outside LAN, proxy', array('192.168.1.10', '128.252.135.4, 192.168.1.10', 'HTTP_X_FORWARDED_FOR', null, '128.252.135.4')),
array('outside LAN, misconfigured proxy', array('192.168.1.10', '128.252.135.4, 192.168.1.10, 192.168.1.10', 'HTTP_X_FORWARDED_FOR', null, '128.252.135.4')),
array('outside LAN, multiple proxies', array('192.168.1.10', '128.252.135.4, 192.168.1.20, 192.168.1.10', 'HTTP_X_FORWARDED_FOR', '192.168.1.*', '128.252.135.4')),
@@ -138,14 +138,14 @@ class IPTest extends \PHPUnit_Framework_TestCase
$_SERVER['REMOTE_ADDR'] = '1.1.1.1';
$_SERVER['HTTP_X_FORWARDED_FOR'] = $ip;
- $this->assertEquals($ip, IP::getNonProxyIpFromHeader('1.1.1.1', array('HTTP_X_FORWARDED_FOR')));
+ $this->assertEquals($ip, IP::getNonProxyIpFromHeader('1.1.1.1', array('HTTP_X_FORWARDED_FOR')), 'case 1');
$_SERVER['HTTP_X_FORWARDED_FOR'] = '1.2.3.4, ' . $ip;
- $this->assertEquals($ip, IP::getNonProxyIpFromHeader('1.1.1.1', array('HTTP_X_FORWARDED_FOR')));
+ $this->assertEquals('1.2.3.4', IP::getNonProxyIpFromHeader('1.1.1.1', array('HTTP_X_FORWARDED_FOR')), 'case 2');
// misconfiguration
$_SERVER['HTTP_X_FORWARDED_FOR'] = $ip . ', 1.1.1.1';
- $this->assertEquals($ip, IP::getNonProxyIpFromHeader('1.1.1.1', array('HTTP_X_FORWARDED_FOR')));
+ $this->assertEquals($ip, IP::getNonProxyIpFromHeader('1.1.1.1', array('HTTP_X_FORWARDED_FOR')), 'case 3');
}
/**
@@ -163,35 +163,37 @@ class IPTest extends \PHPUnit_Framework_TestCase
}
/**
- * Dataprovider for testGetLastIpFromList
+ * Dataprovider for testGetFirstIpFromList
*/
- public function getLastIpFromListTestData()
+ public function getFirstIpFromListTestData()
{
return array(
array('', ''),
array('127.0.0.1', '127.0.0.1'),
array(' 127.0.0.1 ', '127.0.0.1'),
- array(' 192.168.1.1, 127.0.0.1', '127.0.0.1'),
- array('192.168.1.1 ,127.0.0.1 ', '127.0.0.1'),
- array('192.168.1.1,', ''),
+ array(' 192.168.1.1, 127.0.0.1', '192.168.1.1'),
+ array('192.168.1.1 ,127.0.0.1 ', '192.168.1.1'),
+ array('2001:db8:cafe::17 , 192.168.1.1', '2001:db8:cafe::17'),
+ array('192.168.1.1,', '192.168.1.1'),
+ array(',192.168.1.1,', '192.168.1.1'),
);
}
/**
- * @dataProvider getLastIpFromListTestData
+ * @dataProvider getFirstIpFromListTestData
*/
- public function testGetLastIpFromList($csv, $expected)
+ public function testGetFirstIpFromList($csv, $expected)
{
// without excluded IPs
- $this->assertEquals($expected, IP::getLastIpFromList($csv));
+ $this->assertEquals($expected, IP::getFirstIpFromList($csv));
// with excluded Ips
- $this->assertEquals($expected, IP::getLastIpFromList($csv . ', 10.10.10.10', array('10.10.10.10')));
+ $this->assertEquals($expected, IP::getFirstIpFromList($csv . ', 10.10.10.10', array('10.10.10.10')));
}
- public function testGetLastIpFromList_shouldReturnAnEmptyString_IfMultipleIpsAreGivenButAllAreExcluded()
+ public function testGetFirstIpFromList_shouldReturnAnEmptyString_IfMultipleIpsAreGivenButAllAreExcluded()
{
// with excluded Ips
- $this->assertEquals('', IP::getLastIpFromList('10.10.10.10, 10.10.10.10', array('10.10.10.10')));
+ $this->assertEquals('', IP::getFirstIpFromList('10.10.10.10, 10.10.10.10', array('10.10.10.10')));
}
}