From dd69cfa1fe3100d851bd29b31e73d2dd5457f6cf Mon Sep 17 00:00:00 2001
From: Thomas Steur <thomas.steur@gmail.com>
Date: Tue, 27 Oct 2015 00:44:29 +0000
Subject: [PATCH] make sure to return correct ip when all IPs are excluded
---
core/IP.php | 4 +++-
tests/PHPUnit/Unit/IPTest.php | 34 +++++++++++++++++++++++-----------
2 files changed, 26 insertions(+), 12 deletions(-)
diff --git a/core/IP.php b/core/IP.php
index 6e27832e48..670c674544 100644
--- a/core/IP.php
+++ b/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));
}
diff --git a/tests/PHPUnit/Unit/IPTest.php b/tests/PHPUnit/Unit/IPTest.php
index 2225144557..139687f619 100644
--- a/tests/PHPUnit/Unit/IPTest.php
+++ b/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')));
+ }
}
--
GitLab