Fixes #3932

* you can now write browserCode==ff;referrerKeyword!= to select all visitors using firefox and that have a keyword set * or you can write referrerKeyword==;browserCode==ff to select all visitors using firefox and that did not have any keyword set Also fixes #3933 Refs #2135 * fixing last bugs with segment selector encoding (working on chrome + FF + opera) - I 'hope' it will work on iE...
matomo-org · May 12, 2013 · d89a08b · d89a08b
1 parent 263892f
commit d89a08b
Show file tree

Hide file tree

Showing 20 changed files with 132 additions and 46 deletions.
diff --git a/core/API/Proxy.php b/core/API/Proxy.php
@@ -161,6 +161,7 @@ public function call($className, $methodName, $parametersRequest)
 
         // Temporarily sets the Request array to this API call context
         $saveGET = $_GET;
+        $saveQUERY_STRING = @$_SERVER['QUERY_STRING'];
         foreach ($parametersRequest as $param => $value) {
             $_GET[$param] = $value;
         }
@@ -199,6 +200,7 @@ public function call($className, $methodName, $parametersRequest)
 
             // Restore the request
             $_GET = $saveGET;
+            $_SERVER['QUERY_STRING'] = $saveQUERY_STRING;
 
             // log the API Call
             try {

diff --git a/core/API/Request.php b/core/API/Request.php
@@ -48,6 +48,12 @@ class Piwik_API_Request
     static public function getRequestArrayFromString($request)
     {
         $defaultRequest = $_GET + $_POST;
+
+        $requestRaw = self::getRequestParametersGET();
+        if(!empty($requestRaw['segment'])) {
+            $defaultRequest['segment'] = $requestRaw['segment'];
+        }
+
         $requestArray = $defaultRequest;
 
         if (!is_null($request)) {
@@ -63,9 +69,8 @@ static public function getRequestArrayFromString($request)
             $request = str_replace(array("\n", "\t"), '', $request);
 
             $requestParsed = Piwik_Common::getArrayFromQueryString($request);
-
-//            parse_str($request, $requestArray);
             $requestArray = $requestParsed + $defaultRequest;
+
         }
 
         foreach ($requestArray as &$element) {
@@ -209,6 +214,9 @@ public static function processRequest($method, $paramOverride = array())
      */
     public static function getRequestParametersGET()
     {
+        if(empty($_SERVER['QUERY_STRING'])) {
+            return array();
+        }
         $GET = Piwik_Common::getArrayFromQueryString($_SERVER['QUERY_STRING']);
         return $GET;
     }

diff --git a/core/Segment.php b/core/Segment.php
@@ -37,9 +37,9 @@ public function __construct($string, $idSites)
         // First try with url decoded value. If that fails, try with raw value.
         // If that also fails, it will throw the exception
         try {
-            $this->initializeSegment($string, $idSites);
-        } catch(Exception $e) {
             $this->initializeSegment( urldecode($string), $idSites);
+        } catch(Exception $e) {
+            $this->initializeSegment($string, $idSites);
         }
     }
 
@@ -50,7 +50,6 @@ public function __construct($string, $idSites)
      */
     protected function initializeSegment($string, $idSites)
     {
-        $string = Piwik_Common::unsanitizeInputValue($string);
         // As a preventive measure, we restrict the filter size to a safe limit
         $string = substr($string, 0, self::SEGMENT_TRUNCATE_LIMIT);
 
@@ -111,7 +110,8 @@ protected function getCleanedExpression($expression)
             // apply presentation filter
             if (isset($segment['sqlFilter'])
                 && !empty($segment['sqlFilter'])
-                && $matchType != Piwik_SegmentExpression::MATCH_IS_NOT_NULL
+                && $matchType != Piwik_SegmentExpression::MATCH_IS_NOT_NULL_NOR_EMPTY
+                && $matchType != Piwik_SegmentExpression::MATCH_IS_NULL_OR_EMPTY
             ) {
                 $value = call_user_func($segment['sqlFilter'], $value, $segment['sqlSegment'], $matchType, $name);
 

diff --git a/core/SegmentExpression.php b/core/SegmentExpression.php
@@ -27,8 +27,11 @@ class Piwik_SegmentExpression
     const MATCH_CONTAINS = '=@';
     const MATCH_DOES_NOT_CONTAIN = '!@';
 
-    // Note: undocumented for now, only used in API.getSuggestedValuesForSegment
-    const MATCH_IS_NOT_NULL = '::';
+    // Note: you can't write this in the API, but access this feature
+    // via field!=        <- IS NOT NULL
+    // or via field==     <- IS NULL / empty
+    const MATCH_IS_NOT_NULL_NOR_EMPTY = '::NOT_NULL';
+    const MATCH_IS_NULL_OR_EMPTY = '::NULL';
 
     // Special case, since we look up Page URLs/Page titles in a sub SQL query
     const MATCH_ACTIONS_CONTAINS = 'IN';
@@ -72,9 +75,8 @@ public function parseSubExpressions()
                 . self::MATCH_LESS_OR_EQUAL . '|'
                 . self::MATCH_LESS . '|'
                 . self::MATCH_CONTAINS . '|'
-                . self::MATCH_IS_NOT_NULL . '|'
                 . self::MATCH_DOES_NOT_CONTAIN
-                . '){1}(.+)/';
+                . '){1}(.*)/';
             $match = preg_match($pattern, $operand, $matches);
             if ($match == 0) {
                 throw new Exception('The segment \'' . $operand . '\' is not valid.');
@@ -83,6 +85,19 @@ public function parseSubExpressions()
             $leftMember = $matches[1];
             $operation = $matches[2];
             $valueRightMember = urldecode($matches[3]);
+
+            // is null / is not null
+            if ($valueRightMember === '') {
+                if($operation == self::MATCH_NOT_EQUAL) {
+                    $operation = self::MATCH_IS_NOT_NULL_NOR_EMPTY;
+                } elseif($operation == self::MATCH_EQUAL) {
+                    $operation = self::MATCH_IS_NULL_OR_EMPTY;
+                } else {
+                    throw new Exception('The segment \'' . $operand . '\' has no value specified. You can leave this value empty ' .
+                        'only when you use the operators: ' . self::MATCH_NOT_EQUAL . ' (is not) or ' . self::MATCH_EQUAL . ' (is)');
+                }
+            }
+
             $parsedSubExpressions[] = array(
                 self::INDEX_BOOL_OPERATOR => $operator,
                 self::INDEX_OPERAND       => array(
@@ -158,12 +173,14 @@ protected function getSqlMatchFromDefinition($def, &$availableTables)
         $matchType = $def[1];
         $value = $def[2];
 
+        $alsoMatchNULLValues = false;
         switch ($matchType) {
             case self::MATCH_EQUAL:
                 $sqlMatch = '=';
                 break;
             case self::MATCH_NOT_EQUAL:
                 $sqlMatch = '<>';
+                $alsoMatchNULLValues = true;
                 break;
             case self::MATCH_GREATER:
                 $sqlMatch = '>';
@@ -184,13 +201,19 @@ protected function getSqlMatchFromDefinition($def, &$availableTables)
             case self::MATCH_DOES_NOT_CONTAIN:
                 $sqlMatch = 'NOT LIKE';
                 $value = '%' . $this->escapeLikeString($value) . '%';
+                $alsoMatchNULLValues = true;
                 break;
 
-            case self::MATCH_IS_NOT_NULL:
+            case self::MATCH_IS_NOT_NULL_NOR_EMPTY:
                 $sqlMatch = 'IS NOT NULL AND ('.$field.' <> \'\' OR '.$field.' = 0)';
                 $value = null;
                 break;
 
+            case self::MATCH_IS_NULL_OR_EMPTY:
+                $sqlMatch = 'IS NULL OR '.$field.' = \'\' ';
+                $value = null;
+                break;
+
             case self::MATCH_ACTIONS_CONTAINS:
                 // this match type is not accessible from the outside
                 // (it won't be matched in self::parseSubExpressions())
@@ -204,11 +227,18 @@ protected function getSqlMatchFromDefinition($def, &$availableTables)
                 break;
         }
 
+        // We match NULL values when rows are excluded only when we are not doing a
+        $alsoMatchNULLValues = $alsoMatchNULLValues && !empty($value);
+
         if ($matchType === self::MATCH_ACTIONS_CONTAINS
             || is_null($value)) {
-            $sqlExpression = "$field $sqlMatch";
+            $sqlExpression = "( $field $sqlMatch )";
         } else {
-            $sqlExpression = "$field $sqlMatch ?";
+            if($alsoMatchNULLValues) {
+                $sqlExpression = "( $field IS NULL OR $field $sqlMatch ? )";
+            } else {
+                $sqlExpression = "$field $sqlMatch ?";
+            }
         }
 
         $this->checkFieldIsAvailable($field, $availableTables);

diff --git a/core/ViewDataTable.php b/core/ViewDataTable.php
@@ -583,13 +583,26 @@ protected function getRequestString()
             }
         }
 
+        $segment = $this->getRawSegmentFromRequest();
+        if(!empty($segment)) {
+            $requestString .= '&segment=' . $segment;
+        }
+        return $requestString;
+    }
+
+    /**
+     * @return array|bool
+     */
+    static public function getRawSegmentFromRequest()
+    {
         // we need the URL encoded segment parameter, we fetch it from _SERVER['QUERY_STRING'] instead of default URL decoded _GET
+        $segmentRaw = false;
         $segment = Piwik_Common::getRequestVar('segment', '', 'string');
         if (!empty($segment)) {
-            $requestRaw = Piwik_API_Request::getRequestParametersGET();
-            $requestString .= '&segment=' . $requestRaw['segment'];
+            $request = Piwik_API_Request::getRequestParametersGET();
+            $segmentRaw = $request['segment'];
         }
-        return $requestString;
+        return $segmentRaw;
     }
 
     /**
@@ -768,6 +781,11 @@ protected function getJavascriptVariablesToSet()
                 unset($javascriptVariablesToSet[$name]);
             }
         }
+
+        $rawSegment = $this->getRawSegmentFromRequest();
+        $javascriptVariablesToSet['segment'] = $rawSegment;
+
+
         return $javascriptVariablesToSet;
     }
 

diff --git a/core/ViewDataTable/GenerateGraphData/ChartEvolution.php b/core/ViewDataTable/GenerateGraphData/ChartEvolution.php
@@ -191,7 +191,7 @@ protected function initChartObjectData()
                     'idSite'  => $idSite,
                     'period'  => $period->getLabel(),
                     'date'    => $dateInUrl->toString(),
-                    'segment' => Piwik_Common::unsanitizeInputValue(Piwik_Common::getRequestVar('segment', false))
+                    'segment' => Piwik_ViewDataTable::getRawSegmentFromRequest()
                 );
                 $hash = '';
                 if (!empty($queryStringAsHash)) {

diff --git a/plugins/API/API.php b/plugins/API/API.php
@@ -1659,7 +1659,7 @@ public function getSuggestedValuesForSegment($segmentName, $idSite)
 
         // Select non empty fields only
         // Note: this optimization has only a very minor impact
-        $requestLastVisits.= "&segment=$segmentName" . Piwik_SegmentExpression::MATCH_IS_NOT_NULL . "null";
+        $requestLastVisits.= "&segment=$segmentName".urlencode('!=');
 
         // By default Live fetches all actions for all visitors, but we'd rather do this only when required
         if($this->doesSegmentNeedActionsData($segmentName)) {

diff --git a/plugins/CoreHome/DataTableRowAction/RowEvolution.php b/plugins/CoreHome/DataTableRowAction/RowEvolution.php
@@ -91,7 +91,7 @@ public function __construct($idSite, $date, $graphType = null)
             list($this->date, $lastN) =
                 Piwik_ViewDataTable_GenerateGraphHTML_ChartEvolution::getDateRangeAndLastN($this->period, $end);
         }
-        $this->segment = Piwik_Common::getRequestVar('segment', '', 'string');
+        $this->segment = Piwik_ViewDataTable::getRawSegmentFromRequest();
 
         $this->loadEvolutionReport();
     }

diff --git a/plugins/CoreHome/templates/broadcast.js b/plugins/CoreHome/templates/broadcast.js
@@ -462,7 +462,10 @@ var broadcast = {
             hashStr = url.substring(url.indexOf("#"), url.length);
         }
         else {
-            hashStr = (location.hash);
+            locationSplit = location.href.split('#');
+            if(typeof locationSplit[1] != 'undefined') {
+                hashStr = '#' + locationSplit[1];
+            }
         }
 
         return hashStr;

diff --git a/plugins/CoreHome/templates/datatable.js b/plugins/CoreHome/templates/datatable.js
@@ -1367,7 +1367,7 @@ dataTable.prototype =
                 // doing AJAX request
                 var menuItem = null;
                 $("#root").find(">ul.nav a").each(function () {
-                    if ($(this).attr('name') == url) {
+                    if ($(this).attr('href') == url) {
                         menuItem = this;
                         return false
                     }

diff --git a/plugins/CoreHome/templates/datatable_rowactions.js b/plugins/CoreHome/templates/datatable_rowactions.js
@@ -213,6 +213,7 @@ DataTable_RowAction.prototype.getLabelFromTr = function (tr) {
     if (!value) {
         value = label.text();
     }
+    value = value.trim();
 
     return encodeURIComponent(value);
 };

diff --git a/plugins/CoreHome/templates/menu.js b/plugins/CoreHome/templates/menu.js
@@ -29,7 +29,7 @@ menu.prototype =
 
     onItemClick: function (item) {
         $('ul.nav').trigger('piwikSwitchPage', item);
-        broadcast.propagateAjax($(item).attr('name'));
+        broadcast.propagateAjax( $(item).attr('href').substr(1) );
         return false;
     },
 
@@ -45,7 +45,7 @@ menu.prototype =
         // for all sub menu we want to have a unique id based on their module and action
         // for main menu we want to add just the module as its id.
         this.menuNode.find('li').each(function () {
-            var url = $(this).find('a').attr('name');
+            var url = $(this).find('a').attr('href').substr(1);
             var module = broadcast.getValueFromUrl("module", url);
             var action = broadcast.getValueFromUrl("action", url);
             var moduleId = broadcast.getValueFromUrl("idGoal", url) || broadcast.getValueFromUrl("idDashboard", url);

diff --git a/plugins/CoreHome/templates/menu.tpl b/plugins/CoreHome/templates/menu.tpl
@@ -6,7 +6,7 @@
             <ul>
                 {foreach from=$level2 key=name item=urlParameters name=level2}
                     {if strpos($name, '_') !== 0}
-                        <li><a name='{$urlParameters._url|@urlRewriteWithParameters}' href='#{$urlParameters._url|@urlRewriteWithParameters|substr:1}'
+                        <li><a href='#{$urlParameters._url|@urlRewriteWithParameters|substr:1}'
                                onclick='return piwikMenu.onItemClick(this);'>{$name|translate|escape:'html'}</a></li>
                     {/if}
                 {/foreach}

diff --git a/plugins/Live/Controller.php b/plugins/Live/Controller.php
@@ -130,7 +130,7 @@ public function getLastVisitsStart($fetch = false)
 
     private function setCounters($view)
     {
-        $segment = Piwik_Common::getRequestVar('segment', false, 'string');
+        $segment = Piwik_ViewDataTable::getRawSegmentFromRequest();
         $last30min = Piwik_Live_API::getInstance()->getCounters($this->idSite, $lastMinutes = 30, $segment);
         $last30min = $last30min[0];
         $today = Piwik_Live_API::getInstance()->getCounters($this->idSite, $lastMinutes = 24 * 60, $segment);

diff --git a/plugins/SegmentEditor/templates/Segmentation.js b/plugins/SegmentEditor/templates/Segmentation.js
@@ -948,12 +948,7 @@ $(document).ready( function(){
     var changeSegment = function(segmentDefinition){
         $('#segmentEditorPanel a.close').click();
         segmentDefinition = cleanupSegmentDefinition(segmentDefinition);
-
-//        if($.browser.mozilla ) {
-            segmentDefinition = encodeURIComponent(segmentDefinition);
-//        }
-//        alert('new segment to reload='+segmentDefinition);
-
+        segmentDefinition = encodeURIComponent(segmentDefinition);
         return broadcast.propagateNewPage('segment=' + segmentDefinition, true);
     };
 

diff --git a/plugins/UserCountryMap/Controller.php b/plugins/UserCountryMap/Controller.php
@@ -73,7 +73,7 @@ public function visitorMap()
                                                               'date'                        => $date,
                                                               'token_auth'                  => $token_auth,
                                                               'format'                      => 'json',
-                                                              'segment'                     => Piwik_Common::unsanitizeInputValue(Piwik_Common::getRequestVar('segment', '')),
+                                                              'segment'                     => Piwik_ViewDataTable::getRawSegmentFromRequest(),
                                                               'showRawMetrics'              => 1,
                                                               'enable_filter_excludelowpop' => 1,
                                                               'filter_excludelowpop_value'  => -1
@@ -144,7 +144,7 @@ public function realtimeMap($standalone = false)
                                                 'date'           => self::REAL_TIME_WINDOW,
                                                 'token_auth'     => $token_auth,
                                                 'format'         => 'json',
-                                                'segment'        => Piwik_Common::unsanitizeInputValue(Piwik_Common::getRequestVar('segment', '')),
+                                                'segment'        => Piwik_ViewDataTable::getRawSegmentFromRequest(),
                                                 'showRawMetrics' => 1
                                            ));
 
@@ -192,7 +192,7 @@ private function getApiRequestUrl($module, $action, $idSite, $period, $date, $to
             . "&period=" . $period
             . "&date=" . $date
             . "&token_auth=" . $token_auth
-            . "&segment=" . Piwik_Common::unsanitizeInputValue(Piwik_Common::getRequestVar('segment', ''))
+            . "&segment=" . Piwik_ViewDataTable::getRawSegmentFromRequest()
             . "&enable_filter_excludelowpop=1"
             . "&showRawMetrics=1";
 

diff --git a/plugins/VisitsSummary/Controller.php b/plugins/VisitsSummary/Controller.php
@@ -129,7 +129,7 @@ protected function setSparklinesAndNumbers($view)
         $dataTableVisit = self::getVisitsSummary();
         $dataRow = $dataTableVisit->getRowsCount() == 0 ? new Piwik_DataTable_Row() : $dataTableVisit->getFirstRow();
 
-        $dataTableActions = Piwik_Actions_API::getInstance()->get($idSite, Piwik_Common::getRequestVar('period'), Piwik_Common::getRequestVar('date'), Piwik_Common::getRequestVar('segment', false));
+        $dataTableActions = Piwik_Actions_API::getInstance()->get($idSite, Piwik_Common::getRequestVar('period'), Piwik_Common::getRequestVar('date'), Piwik_ViewDataTable::getRawSegmentFromRequest());
         $dataActionsRow =
             $dataTableActions->getRowsCount() == 0 ? new Piwik_DataTable_Row() : $dataTableActions->getFirstRow();