Merge pull request #7990 from piwik/7989

Page URL are stored with HTML entities encoded (sanitized)

core/Tracker/PageUrl.php

@@ -38,7 +38,7 @@ class PageUrl
      * @static
      * @param $originalUrl
      * @param $idSite
-     * @return bool|string
+     * @return bool|string Returned URL is HTML entities decoded
      */
     public static function excludeQueryParametersFromUrl($originalUrl, $idSite)
     {

core/Tracker/TableLogAction.php

@@ -235,7 +235,7 @@ class TableLogAction
     /**
      * This function will sanitize or not if it's needed for the specified action type
      *
-     * URLs (Page URLs, Downloads, Outlinks) are stored raw (unsanitized)
+     * URLs (Download URL, Outlink URL) are stored raw (unsanitized)
      * while other action types are stored Sanitized
      *
      * @param $actionType
@@ -246,26 +246,25 @@ class TableLogAction
     {
         $actionString = Common::unsanitizeInputValue($actionString);
 
-        if (self::isActionTypeStoredSanitized($actionType)) {
-            return Common::sanitizeInputValue($actionString);
+        if (self::isActionTypeStoredUnsanitized($actionType)) {
+            return $actionString;
         }
-        return $actionString;
+
+        return Common::sanitizeInputValue($actionString);
     }
 
     /**
      * @param $actionType
      * @return bool
      */
-    private static function isActionTypeStoredSanitized($actionType)
+    private static function isActionTypeStoredUnsanitized($actionType)
     {
         $actionsTypesStoredUnsanitized = array(
-            $actionType == Action::TYPE_PAGE_URL,
             $actionType == Action::TYPE_DOWNLOAD,
             $actionType == Action::TYPE_OUTLINK,
         );
 
-        $isStoredUnsanitized = in_array($actionType, $actionsTypesStoredUnsanitized);
-        return !$isStoredUnsanitized;
+        return in_array($actionType, $actionsTypesStoredUnsanitized);
     }
 
 }

plugins/Live/templates/_actionsList.twig

@@ -101,7 +101,7 @@
                     and previousAction.url|default(false) == action.url %}
                     {# For events, do not show (url) if the Event URL is the same as the URL last displayed #}
                 {% else %}
-                    <a href="{{ action.url }}" rel="noreferrer"  target="_blank" rel="noreferrer" class="{% if action.eventCategory|default(false) is empty %}action-list-url{# don't put URL on new line for events #}{% endif %} truncated-text-line"
+                    <a href="{{ action.url }}" rel="noreferrer" target="_blank" class="{% if action.eventCategory|default(false) is empty %}action-list-url{# don't put URL on new line for events #}{% endif %} truncated-text-line"
                        {% if overrideLinkStyle is not defined or overrideLinkStyle %}style="{% if action.type=='action' and action.pageTitle|default(false) is not empty %}margin-left: 9px;{% endif %}text-decoration:underline;"{% endif %}>
                        {% if action.eventCategory|default(false) is not empty %}
                            (url)

tests/PHPUnit/Fixtures/TwoVisitsNoKeywordWithBot.php

@@ -53,8 +53,10 @@ class TwoVisitsNoKeywordWithBot extends Fixture
         // VISIT 1 = Referrer is "Keyword not defined"
         // Alsotrigger goal to check that attribution goes to this keyword
         $t->setUrlReferrer('http://www.google.com/url?sa=t&rct=j&q=&esrc=s&source=web&cd=1&ved=0CC&url=http%3A%2F%2Fpiwik.org%2F&ei=&usg=');
-        $t->setUrl('http://example.org/this%20is%20cool!');
-        self::checkResponse($t->doTrackPageView('incredible title!'));
+
+        $t->setUrl('http://example.org/this%20is%20cool!?filter=<script>alert(1);</script>{"place":{"place":"0c5b2444-70a0-4932-980c-b4dc0d3f02b5"}}');
+        self::checkResponse($t->doTrackPageView('incredible title! (Page URL contains a HTML entity)'));
+
         $idGoal = 1;
         if (!self::goalExists($idSite, $idGoal)) {
             $idGoal = API::getInstance()->addGoal($idSite, 'triggered js', 'manually', '', '');

tests/PHPUnit/System/expected/test_OneVisitor_NoKeywordSpecified__Live.getLastVisitsDetails_day.xml

@@ -8,7 +8,7 @@
 		<actionDetails>
 			<row>
 				<type>action</type>
-				<url>http://example.org/this%20is%20cool!</url>
+				<url>http://example.org/this%20is%20cool!?filter=&lt;script&gt;alert(1);&lt;/script&gt;{&quot;place&quot;:{&quot;place&quot;:&quot;0c5b2444-70a0-4932-980c-b4dc0d3f02b5&quot;}}</url>
 				<pageTitle />
 				<pageIdAction>2</pageIdAction>
 				
@@ -109,8 +109,8 @@
 		<actionDetails>
 			<row>
 				<type>action</type>
-				<url>http://example.org/this%20is%20cool!</url>
-				<pageTitle>incredible title!</pageTitle>
+				<url>http://example.org/this%20is%20cool!?filter=&lt;script&gt;alert(1);&lt;/script&gt;{&quot;place&quot;:{&quot;place&quot;:&quot;0c5b2444-70a0-4932-980c-b4dc0d3f02b5&quot;}}</url>
+				<pageTitle>incredible title! (Page URL contains a HTML entity)</pageTitle>
 				<pageIdAction>2</pageIdAction>
 				
 				<pageId>1</pageId>
@@ -124,7 +124,7 @@
 				<revenue>0</revenue>
 				<goalPageId />
 				
-				<url>http://example.org/this%20is%20cool!</url>
+				<url>http://example.org/this%20is%20cool!?filter=&lt;script&gt;alert(1);&lt;/script&gt;{&quot;place&quot;:{&quot;place&quot;:&quot;0c5b2444-70a0-4932-980c-b4dc0d3f02b5&quot;}}</url>
 				<icon>plugins/Morpheus/images/goal.png</icon>
 				
 			</row>