Merge pull request #10762 from piwik/10755

Fix integrity filecheck may falsely mark piwikjs as invalid

core/Filechecks.php

@@ -9,6 +9,8 @@
 namespace Piwik;
 
 use Piwik\Exception\MissingFilePermissionException;
+use Piwik\Plugins\CustomPiwikJs\Exception\AccessDeniedException;
+use Piwik\Plugins\CustomPiwikJs\TrackerUpdater;
 
 class Filechecks
 {
@@ -102,6 +104,40 @@ class Filechecks
         throw $ex;
     }
 
+    private static function isModifiedPathValid($path)
+    {
+        if ($path === 'piwik.js') {
+            // we could have used a postEvent hook to enrich "\Piwik\Manifest::$files;" which would also benefit plugins
+            // that want to check for file integrity but we do not want to risk to break anything right now. It is not
+            // as trivial because piwik.js might be already updated, or updated on the next request. We cannot define
+            // 2 or 3 different filesizes and md5 hashes for one file so we check it here.
+
+            if (Plugin\Manager::getInstance()->isPluginActivated('CustomPiwikJs')) {
+                $trackerUpdater = new TrackerUpdater();
+
+                if ($trackerUpdater->getCurrentTrackerFileContent() === $trackerUpdater->getUpdatedTrackerFileContent()) {
+                    // file was already updated, eg manually or via custom piwik.js, this is a valid piwik.js file as
+                    // it was enriched by tracker plugins
+                    return true;
+                }
+
+                try {
+                    // the piwik.js tracker file was not updated yet, but may be updated just after the update by
+                    // one of the events CustomPiwikJs is listening to or by a scheduled task.
+                    // In this case, we check whether such an update will succeed later and if it will, the file is
+                    // valid as well as it will be updated on the next request
+                    $trackerUpdater->checkWillSucceed();
+                    return true;
+                } catch (AccessDeniedException $e) {
+                    return false;
+                }
+
+            }
+        }
+
+        return false;
+    }
+
     /**
      * Get file integrity information (in PIWIK_INCLUDE_PATH).
      *
@@ -136,6 +172,11 @@ class Filechecks
             if (!file_exists($file) || !is_readable($file)) {
                 $messages[] = Piwik::translate('General_ExceptionMissingFile', $file);
             } elseif (filesize($file) != $props[0]) {
+
+                if (self::isModifiedPathValid($path)) {
+                    continue;
+                }
+
                 if (!$hasMd5 || in_array(substr($path, -4), array('.gif', '.ico', '.jpg', '.png', '.swf'))) {
                     // files that contain binary data (e.g., images) must match the file size
                     $messages[] = Piwik::translate('General_ExceptionFilesizeMismatch', array($file, $props[0], filesize($file)));
@@ -150,6 +191,10 @@ class Filechecks
                     }
                 }
             } elseif ($hasMd5file && (@md5_file($file) !== $props[1])) {
+                if (self::isModifiedPathValid($path)) {
+                    continue;
+                }
+
                 $messages[] = Piwik::translate('General_ExceptionFileIntegrity', $file);
             }
         }

plugins/CustomPiwikJs/TrackerUpdater.php

@@ -62,16 +62,28 @@ class TrackerUpdater
         $this->toFile->checkWritable();
     }
 
+    public function getCurrentTrackerFileContent()
+    {
+        return $this->toFile->getContent();
+    }
+
+    public function getUpdatedTrackerFileContent()
+    {
+        $trackingCode = new PiwikJsManipulator($this->fromFile->getContent(), $this->trackerFiles);
+        $newContent = $trackingCode->manipulateContent();
+
+        return $newContent;
+    }
+
     public function update()
     {
         if (!$this->toFile->hasWriteAccess() || !$this->fromFile->hasReadAccess()) {
             return;
         }
 
-        $trackingCode = new PiwikJsManipulator($this->fromFile->getContent(), $this->trackerFiles);
-        $newContent = $trackingCode->manipulateContent();
+        $newContent = $this->getUpdatedTrackerFileContent();
 
-        if ($newContent !== $this->toFile->getContent()) {
+        if ($newContent !== $this->getCurrentTrackerFileContent()) {
             $this->toFile->save($newContent);
         }
     }

plugins/CustomPiwikJs/tests/Integration/TrackerUpdaterTest.php

@@ -74,6 +74,58 @@ class TrackerUpdaterTest extends IntegrationTestCase
         $updater->checkWillSucceed();
     }
 
+    public function test_getCurrentTrackerFileContent()
+    {
+        $targetFile = $this->dir . 'testpiwik.js';
+
+        $updater = $this->makeUpdater(null, $targetFile);
+        $content = $updater->getCurrentTrackerFileContent();
+
+        $this->assertSame(file_get_contents($targetFile), $content);
+    }
+
+    public function test_getUpdatedTrackerFileContent_returnsGeneratedPiwikJsWithMergedTrackerFiles_WhenTheyExist()
+    {
+        $source = $this->dir . 'testpiwik.js';
+        $target = $this->dir . 'MyTestTarget.js';
+
+        $updater = $this->makeUpdater($source, $target);
+        $updater->setTrackerFiles(new PluginTrackerFilesMock(array(
+            $this->dir . 'tracker.js', $this->dir . 'tracker.min.js'
+        )));
+        $content = $updater->getUpdatedTrackerFileContent();
+
+        $this->assertSame('/** MyHeader*/
+var PiwikJs = "mytest";
+
+/*!!! pluginTrackerHook */
+
+/* GENERATED: tracker.min.js */
+
+/* END GENERATED: tracker.min.js */
+
+
+/* GENERATED: tracker.js */
+
+/* END GENERATED: tracker.js */
+
+
+var myArray = [];
+', $content);
+    }
+
+    public function test_getUpdatedTrackerFileContent_returnsSourceFile_IfNoTrackerFilesFound()
+    {
+        $source = $this->dir . 'testpiwik.js';
+        $target = $this->dir . 'MyTestTarget.js';
+
+        $updater = $this->makeUpdater($source, $target);
+        $updater->setTrackerFiles(new PluginTrackerFilesMock(array()));
+        $content = $updater->getUpdatedTrackerFileContent();
+
+        $this->assertSame(file_get_contents($source), $content);
+    }
+
     public function test_update_shouldNotThrowAnError_IfTargetFileIsNotWritable()
     {
         $updater = $this->makeUpdater(null, $this->dir . 'MyNotExisIngFilessss.js');