From 3a306e06e7b57af452a47a02fc6c654793fc735e Mon Sep 17 00:00:00 2001
From: mattab <matthieu.aubry@gmail.com>
Date: Thu, 29 Sep 2016 09:52:25 +1300
Subject: [PATCH] remove extra two parameters in VisitExcluded constructor to
 prevent confusion

---
 core/Tracker/VisitExcluded.php                | 17 ++++---------
 .../Tracker/VisitRequestProcessor.php         |  3 +--
 .../Mock/Tracker/RequestAuthenticated.php     | 10 ++++++++
 .../PHPUnit/Integration/Tracker/VisitTest.php | 24 ++++++++++---------
 4 files changed, 28 insertions(+), 26 deletions(-)
 create mode 100644 tests/PHPUnit/Framework/Mock/Tracker/RequestAuthenticated.php

diff --git a/core/Tracker/VisitExcluded.php b/core/Tracker/VisitExcluded.php
index a644d4479f1..859a84992db 100644
--- a/core/Tracker/VisitExcluded.php
+++ b/core/Tracker/VisitExcluded.php
@@ -28,25 +28,16 @@ class VisitExcluded
 
     /**
      * @param Request $request
-     * @param bool|string $ip
-     * @param bool|string $userAgent
      */
-    public function __construct(Request $request, $ip = false, $userAgent = false)
+    public function __construct(Request $request)
     {
         $this->spamFilter = new ReferrerSpamFilter();
 
-        if (false === $ip) {
-            $ip = $request->getIp();
-        }
-
-        if (false === $userAgent) {
-            $userAgent = $request->getUserAgent();
-        }
-
         $this->request   = $request;
         $this->idSite    = $request->getIdSite();
-        $this->userAgent = $userAgent;
-        $this->ip = $ip;
+        $userAgent       = $request->getUserAgent();
+        $this->userAgent = Common::unsanitizeInputValue($userAgent);
+        $this->ip        = $request->getIp();
     }
 
     /**
diff --git a/plugins/CoreHome/Tracker/VisitRequestProcessor.php b/plugins/CoreHome/Tracker/VisitRequestProcessor.php
index 019965a9583..033399e196a 100644
--- a/plugins/CoreHome/Tracker/VisitRequestProcessor.php
+++ b/plugins/CoreHome/Tracker/VisitRequestProcessor.php
@@ -87,8 +87,7 @@ public function processRequestParams(VisitProperties $visitProperties, Request $
         // the IP is needed by isExcluded() and GoalManager->recordGoals()
         $visitProperties->setProperty('location_ip', $request->getIp());
 
-        // TODO: move VisitExcluded logic to here (or move to service class stored in DI)
-        $excluded = new VisitExcluded($request, $visitProperties->getProperty('location_ip'));
+        $excluded = new VisitExcluded($request);
         if ($excluded->isExcluded()) {
             return true;
         }
diff --git a/tests/PHPUnit/Framework/Mock/Tracker/RequestAuthenticated.php b/tests/PHPUnit/Framework/Mock/Tracker/RequestAuthenticated.php
new file mode 100644
index 00000000000..0b3c0a35554
--- /dev/null
+++ b/tests/PHPUnit/Framework/Mock/Tracker/RequestAuthenticated.php
@@ -0,0 +1,10 @@
+<?php
+namespace Piwik\Tests\Framework\Mock\Tracker;
+
+use Piwik\Tracker\Request;
+
+class RequestAuthenticated extends Request
+{
+    protected $isAuthenticated = true;
+
+}
\ No newline at end of file
diff --git a/tests/PHPUnit/Integration/Tracker/VisitTest.php b/tests/PHPUnit/Integration/Tracker/VisitTest.php
index f4bc1fd5edc..b1b42eb5b25 100644
--- a/tests/PHPUnit/Integration/Tracker/VisitTest.php
+++ b/tests/PHPUnit/Integration/Tracker/VisitTest.php
@@ -16,6 +16,7 @@
 use Piwik\Plugins\SitesManager\API;
 use Piwik\Tests\Framework\Fixture;
 use Piwik\Tests\Framework\Mock\FakeAccess;
+use Piwik\Tests\Framework\Mock\Tracker\RequestAuthenticated;
 use Piwik\Tracker\Request;
 use Piwik\Tracker\Visit;
 use Piwik\Tracker\VisitExcluded;
@@ -88,14 +89,14 @@ public function testIsVisitorIpExcluded($excludedIp, $tests)
         $idsite = API::getInstance()->addSite("name", "http://piwik.net/", $ecommerce = 0,
             $siteSearch = 1, $searchKeywordParameters = null, $searchCategoryParameters = null, $excludedIp);
 
-        $request = new Request(array('idsite' => $idsite));
+        $request = new RequestAuthenticated(array('idsite' => $idsite));
 
         // test that IPs within the range, or the given IP, are excluded
         foreach ($tests as $ip => $expected) {
-            $testIpIsExcluded = IPUtils::stringToBinaryIP($ip);
+            $request->setParam('cip', $ip);
 
-            $excluded = new VisitExcluded_public($request, $testIpIsExcluded);
-            $this->assertSame($expected, $excluded->public_isVisitorIpExcluded($testIpIsExcluded));
+            $excluded = new VisitExcluded_public($request);
+            $this->assertSame($expected, $excluded->public_isVisitorIpExcluded($ip));
         }
     }
 
@@ -176,13 +177,12 @@ public function testVisitShouldNotBeExcluded_IfMadeViaChromeDataSaverCompression
         $idsite = API::getInstance()->addSite("name", "http://piwik.net/", $ecommerce = 0,
             $siteSearch = 1, $searchKeywordParameters = null, $searchCategoryParameters = null);
 
-        $request = new Request(array('idsite' => $idsite));
 
-        $testIpIsExcluded = IPUtils::stringToBinaryIP($ip);
+        $request = new RequestAuthenticated(array('idsite' => $idsite, 'cip' => $ip));
 
         $_SERVER['HTTP_VIA'] = '1.1 Chrome-Compression-Proxy';
-        $excluded = new VisitExcluded_public($request, $testIpIsExcluded);
-        $isBot = $excluded->public_isNonHumanBot($testIpIsExcluded);
+        $excluded = new VisitExcluded_public($request);
+        $isBot = $excluded->public_isNonHumanBot();
         unset($_SERVER['HTTP_VIA']);
         $this->assertSame($isNonHumanBot, $isBot);
     }
@@ -239,7 +239,8 @@ public function testIsVisitorUserAgentExcluded($excludedUserAgent, $tests)
 
         // test that user agents that contain excluded user agent strings are excluded
         foreach ($tests as $ua => $expected) {
-            $excluded = new VisitExcluded_public($request, $ip = false, $ua);
+            $request->setParam('ua', $ua);
+            $excluded = new VisitExcluded_public($request);
 
             $this->assertSame($expected, $excluded->public_isUserAgentExcluded(), "Result if isUserAgentExcluded('$ua') was not " . ($expected ? 'true' : 'false') . ".");
         }
@@ -306,10 +307,11 @@ public function testIsVisitor_ipIsKnownBot()
         );
 
         $idsite = API::getInstance()->addSite("name", "http://piwik.net/");
-        $request = new Request(array('idsite' => $idsite, 'bots' => 0));
+        $request = new RequestAuthenticated(array('idsite' => $idsite, 'bots' => 0));
 
         foreach ($isIpBot as $ip => $isBot) {
-            $excluded = new VisitExcluded_public($request, IPUtils::stringToBinaryIP($ip));
+            $request->setParam('cip', $ip);
+            $excluded = new VisitExcluded_public($request);
 
             $this->assertSame($isBot, $excluded->public_isNonHumanBot(), $ip);
         }