when matching Goals against patterns, match both the RAW pattern and URL decoded pattern

Fixes #3146

1 files changed, 54 insertions, 34 deletions

diff --git a/core/Tracker/GoalManager.php b/core/Tracker/GoalManager.php
index 913522696a..2b9de403a2 100644
--- a/core/Tracker/GoalManager.php
+++ b/core/Tracker/GoalManager.php
@@ -765,41 +765,15 @@ class GoalManager
         $url = Common::unsanitizeInputValue($url);
         $goal['pattern'] = Common::unsanitizeInputValue($goal['pattern']);
 
-        switch ($pattern_type) {
-            case 'regex':
-                $pattern = $goal['pattern'];
-                if (strpos($pattern, '/') !== false
-                    && strpos($pattern, '\\/') === false
-                ) {
-                    $pattern = str_replace('/', '\\/', $pattern);
-                }
-                $pattern = '/' . $pattern . '/';
-                if (!$goal['case_sensitive']) {
-                    $pattern .= 'i';
-                }
-                $match = (@preg_match($pattern, $url) == 1);
-                break;
-            case 'contains':
-                if ($goal['case_sensitive']) {
-                    $matched = strpos($url, $goal['pattern']);
-                } else {
-                    $matched = stripos($url, $goal['pattern']);
-                }
-                $match = ($matched !== false);
-                break;
-            case 'exact':
-                if ($goal['case_sensitive']) {
-                    $matched = strcmp($goal['pattern'], $url);
-                } else {
-                    $matched = strcasecmp($goal['pattern'], $url);
-                }
-                $match = ($matched == 0);
-                break;
-            default:
-                throw new Exception(Piwik::translate('General_ExceptionInvalidGoalPattern', array($pattern_type)));
-                break;
-        }
+        $match = $this->isGoalPatternMatchingUrl($goal, $pattern_type, $url);
+
+        if(!$match) {
+            // Users may set Goal matching URL as URL encoded
+            $goal['pattern'] = urldecode($goal['pattern']);
+
+            $match = $this->isGoalPatternMatchingUrl($goal, $pattern_type, $url);
 
+        }
         return $match;
     }
 
@@ -852,4 +826,50 @@ class GoalManager
 
         return $goal;
     }
+
+    /**
+     * @param $goal
+     * @param $pattern_type
+     * @param $url
+     * @return bool
+     * @throws Exception
+     */
+    protected function isGoalPatternMatchingUrl($goal, $pattern_type, $url)
+    {
+        switch ($pattern_type) {
+            case 'regex':
+                $pattern = $goal['pattern'];
+                if (strpos($pattern, '/') !== false
+                    && strpos($pattern, '\\/') === false
+                ) {
+                    $pattern = str_replace('/', '\\/', $pattern);
+                }
+                $pattern = '/' . $pattern . '/';
+                if (!$goal['case_sensitive']) {
+                    $pattern .= 'i';
+                }
+                $match = (@preg_match($pattern, $url) == 1);
+                break;
+            case 'contains':
+                if ($goal['case_sensitive']) {
+                    $matched = strpos($url, $goal['pattern']);
+                } else {
+                    $matched = stripos($url, $goal['pattern']);
+                }
+                $match = ($matched !== false);
+                break;
+            case 'exact':
+                if ($goal['case_sensitive']) {
+                    $matched = strcmp($goal['pattern'], $url);
+                } else {
+                    $matched = strcasecmp($goal['pattern'], $url);
+                }
+                $match = ($matched == 0);
+                break;
+            default:
+                throw new Exception(Piwik::translate('General_ExceptionInvalidGoalPattern', array($pattern_type)));
+                break;
+        }
+        return $match;
+    }
 }