Validate goal pattern and type in API (#12788)

* Validate goal pattern and type

* allow empty pattern type

* Use validator classes to validate API params

* tweak error message

2 files changed, 48 insertions, 0 deletions

diff --git a/plugins/Goals/API.php b/plugins/Goals/API.php
index d3e845330a..adb24c9541 100644
--- a/plugins/Goals/API.php
+++ b/plugins/Goals/API.php
@@ -28,6 +28,8 @@ use Piwik\Site;
 use Piwik\Tracker\Cache;
 use Piwik\Tracker\GoalManager;
 use Piwik\Plugins\VisitFrequency\API as VisitFrequencyAPI;
+use Piwik\Validators\Regex;
+use Piwik\Validators\WhitelistedValue;
 
 /**
  * Goals API lets you Manage existing goals, via "updateGoal" and "deleteGoal", create new Goals via "addGoal",
@@ -136,6 +138,7 @@ class API extends \Piwik\Plugin\API
         $this->checkPatternIsValid($patternType, $pattern, $matchAttribute);
         $name        = $this->checkName($name);
         $pattern     = $this->checkPattern($pattern);
+        $patternType = $this->checkPatternType($patternType);
         $description = $this->checkDescription($description);
 
         $revenue = Common::forceDotAsSeparatorForDecimalPoint((float)$revenue);
@@ -188,6 +191,7 @@ class API extends \Piwik\Plugin\API
 
         $name        = $this->checkName($name);
         $description = $this->checkDescription($description);
+        $patternType = $this->checkPatternType($patternType);
         $pattern     = $this->checkPattern($pattern);
         $this->checkPatternIsValid($patternType, $pattern, $matchAttribute);
 
@@ -218,6 +222,11 @@ class API extends \Piwik\Plugin\API
         ) {
             throw new Exception(Piwik::translate('Goals_ExceptionInvalidMatchingString', array("http:// or https://", "http://www.yourwebsite.com/newsletter/subscribed.html")));
         }
+
+        if ($patternType == 'regex') {
+            $validator = new Regex();
+            $validator->validate(GoalManager::formatRegex($pattern));
+        }
     }
 
     private function checkName($name)
@@ -230,6 +239,20 @@ class API extends \Piwik\Plugin\API
         return urldecode($description);
     }
 
+    private function checkPatternType($patternType)
+    {
+        if (empty($patternType)) {
+            return '';
+        }
+
+        $patternType = strtolower($patternType);
+
+        $validator = new WhitelistedValue(['exact', 'contains', 'regex']);
+        $validator->validate($patternType);
+
+        return $patternType;
+    }
+
     private function checkPattern($pattern)
     {
         return urldecode($pattern);
diff --git a/plugins/Goals/tests/Integration/APITest.php b/plugins/Goals/tests/Integration/APITest.php
index c31047132d..c6492697ad 100644
--- a/plugins/Goals/tests/Integration/APITest.php
+++ b/plugins/Goals/tests/Integration/APITest.php
@@ -69,6 +69,31 @@ class APITest extends IntegrationTestCase
         $this->assertGoal($idGoal, 'MyName', '', 'title', 'normal title', 'exact', 1, 50, 1);
     }
 
+    public function test_addGoal_ShouldSucceed_IfRegexPageTitle()
+    {
+        $idGoal = $this->api->addGoal($this->idSite, 'MyName', 'title', 'rere(.*)', 'regex', true, 50, true);
+
+        $this->assertGoal($idGoal, 'MyName', '', 'title', 'rere(.*)', 'regex', 1, 50, 1);
+    }
+
+    /**
+     * @expectedException \Exception
+     * @expectedExceptionMessage General_ValidatorErrorXNotWhitelisted
+     */
+    public function test_addGoal_shouldThrowException_IfPatternTypeIsInvalid()
+    {
+        $this->api->addGoal($this->idSite, 'MyName', 'external_website', 'www.test.de', 'invalid');
+    }
+
+    /**
+     * @expectedException \Exception
+     * @expectedExceptionMessage General_ValidatorErrorNoValidRegex
+     */
+    public function test_addGoal_shouldThrowException_IfPatternRegexIsInvalid()
+    {
+        $this->api->addGoal($this->idSite, 'MyName', 'url', '/(%$f', 'regex');
+    }
+
     /**
      * @expectedException \Exception
      * @expectedExceptionMessage Goals_ExceptionInvalidMatchingString