Merge pull request #9358 from piwik/exact_match_unknownurl

Exclude unknown urls: Check whether known URL starts with path if one is defined, do no longer match subdomains

3 files changed, 43 insertions, 15 deletions

diff --git a/core/Tracker/VisitExcluded.php b/core/Tracker/VisitExcluded.php
index 0bc6fa2fbb..a644d4479f 100644
--- a/core/Tracker/VisitExcluded.php
+++ b/core/Tracker/VisitExcluded.php
@@ -13,6 +13,7 @@ use Piwik\Common;
 use Piwik\DeviceDetectorFactory;
 use Piwik\Network\IP;
 use Piwik\Piwik;
+use Piwik\Plugins\SitesManager\SiteUrls;
 use Piwik\Tracker\Visit\ReferrerSpamFilter;
 
 /**
@@ -290,14 +291,17 @@ class VisitExcluded
     {
         $site = Cache::getCacheWebsiteAttributes($this->idSite);
 
-        if (!empty($site['exclude_unknown_urls']) && !empty($site['hosts'])) {
-            $trackingHost = parse_url($this->request->getParam('url'), PHP_URL_HOST);
-            foreach ($site['hosts'] as $siteHost) {
-                if ($trackingHost == $siteHost || (substr($trackingHost, -strlen($siteHost) - 1) === ('.' . $siteHost))) {
-                    return false;
-                }
-            }
-            return true;
+        if (!empty($site['exclude_unknown_urls']) && !empty($site['urls'])) {
+            $url = $this->request->getParam('url');
+            $parsedUrl = parse_url($url);
+
+            $trackingUrl = new SiteUrls();
+            $urls = $trackingUrl->groupUrlsByHost(array($this->idSite => $site['urls']));
+
+            $idSites = $trackingUrl->getIdSitesMatchingUrl($parsedUrl, $urls);
+            $isUrlExcluded = !isset($idSites) || !in_array($this->idSite, $idSites);
+
+            return $isUrlExcluded;
         }
 
         return false;
diff --git a/plugins/SitesManager/SitesManager.php b/plugins/SitesManager/SitesManager.php
index 4b686e2bde..ca3f2757b9 100644
--- a/plugins/SitesManager/SitesManager.php
+++ b/plugins/SitesManager/SitesManager.php
@@ -114,8 +114,11 @@ class SitesManager extends \Piwik\Plugin
     {
         $idSite = (int) $idSite;
 
+        $urls = API::getInstance()->getSiteUrlsFromId($idSite);
+
         // add the 'hosts' entry in the website array
-        $array['hosts'] = $this->getTrackerHosts($idSite);
+        $array['urls']  = $urls;
+        $array['hosts'] = $this->getTrackerHosts($urls);
 
         $website = API::getInstance()->getSiteFromId($idSite);
         $array['exclude_unknown_urls'] = $website['exclude_unknown_urls'];
@@ -252,9 +255,8 @@ class SitesManager extends \Piwik\Plugin
      * @param int $idSite
      * @return array
      */
-    private function getTrackerHosts($idSite)
+    private function getTrackerHosts($urls)
     {
-        $urls = API::getInstance()->getSiteUrlsFromId($idSite);
         $hosts = array();
         foreach ($urls as $url) {
             $url = parse_url($url);
diff --git a/tests/PHPUnit/Integration/Tracker/VisitTest.php b/tests/PHPUnit/Integration/Tracker/VisitTest.php
index e686146f73..f4bc1fd5ed 100644
--- a/tests/PHPUnit/Integration/Tracker/VisitTest.php
+++ b/tests/PHPUnit/Integration/Tracker/VisitTest.php
@@ -118,15 +118,37 @@ class VisitTest extends IntegrationTestCase
                 'http://x.com' => true,
             )),
             array(array('http://test.com', 'http://sub.test2.com'), true, array(
-                'http://sub.test.com' => true,
-                'http://sub.sub.test.com' => true,
+                'http://sub.test.com' => false, // we do not match subdomains
+                'http://sub.sub.test.com' => false,
                 'http://subtest.com' => false,
                 'http://test.com.org' => false,
+                'http://test2.com' => false,
                 'http://sub.test2.com' => true,
-                'http://x.sub.test2.com' => true,
+                'http://test.com' => true,
+                'http://x.sub.test2.com' => false,
                 'http://xsub.test2.com' => false,
                 'http://sub.test2.com.org' => false,
             )),
+            array(array('http://test.com/path', 'http://test2.com/sub/dir'), true, array(
+                'http://test.com/path' => true, // test matching path
+                'http://test.com/path/' => true,
+                'http://test.com/path/test' => true,
+
+                'http://test.com/path1' => false,
+                'http://test.com/' => false,
+                'http://test.com' => false,
+                'http://test.com/foo' => false,
+                'http://sub.test.com/path' => false,  // we still do not match subdomains
+
+                'http://test2.com/sub/dir' => true,
+                'http://test2.com/sub/dir/' => true,
+                'http://test2.com/sub/dir/test' => true,
+
+                'http://test2.com/sub/foo/' => false,
+                'http://test2.com/sub/' => false,
+                'http://test2.com/' => false,
+                'http://test2.com/dir/sub' => false,
+            )),
         );
     }
 
@@ -142,7 +164,7 @@ class VisitTest extends IntegrationTestCase
                 'rec'    => 1,
                 'url'    => $url
             )));
-            $this->assertEquals($isTracked, !$visitExclude->isExcluded());
+            $this->assertEquals($isTracked, !$visitExclude->isExcluded(), $url . ' is not returning expected result');
         }
     }