strtolower(): Argument #1 ($string) must be of type string, array given in Date.php #18639

First problem:

Error: {"message":"strtolower(): Argument #1 ($string) must be of type string, array given","file":"\/core\/Date.php","line":1079,"request_id":"ba369","backtrace":" on \/core\/Date.php(1079)\n#0 \/core\/Date.php(1079): strtolower(Array)\n#1 \/core\/Period\/Range.php(255): Piwik\Date->addPeriod(-29, Array)\n#2 \/core\/Period.php(131): Piwik\Period\Range->generate()\n#3 \/core\/Period\/Range.php(152): Piwik\Period->getDateStart()\n#4 \/core\/Period\/Range.php(533): Piwik\Period\Range->getDateStart()\n#5 \/core\/Plugin\/Controller.php(502): Piwik\Period\Range::getRelativeToEndDate(Array, 'last30', Object(Piwik\Date), Object(Piwik\Site))\n#6 \/plugins\/MultiSites\/Controller.php(97): Piwik\Plugin\Controller->getGraphParamsModified()\n#7 \/plugins\/MultiSites\/Controller.php(37): Piwik\Plugins\MultiSites\Controller->getSitesInfo(false)\n#8 [internal function]: Piwik\Plugins\MultiSites\Controller->index()\n#9 \/core\/FrontController.php(619): call_user_func_array(Array, Array)\n#10 \/core\/FrontController.php(168): Piwik\FrontController->doDispatch('MultiSites', 'index', Array)\n#11 \/core\/dispatch.php(32): Piwik\FrontController->dispatch()\n#12

Happens on

• /index.php?action=index&date=yesterday&idSite=1&module=MultiSites&period[]=day

Second problem

Error: {"message":"urldecode(): Argument #1 ($string) must be of type string, array given","file":"\/core\/Date.php","line":138,"request_id":"83d50","backtrace":" on \/core\/Date.php(138)\n#0 \/core\/Date.php(138): urldecode(Array)\n#1 \/core\/Period.php(121): Piwik\Date::factory(Array)\n#2 \/core\/Plugin\/Controller.php(644): Piwik\Period::checkDateFormat(Array)\n#3 \/core\/Plugin\/Controller.php(616): Piwik\Plugin\Controller->setGeneralVariablesViewAs(Object(Piwik\View), 'basic')\n#4 \/plugins\/MultiSites\/Controller.php(100): Piwik\Plugin\Controller->setGeneralVariablesView(Object(Piwik\View))\n#5 \/plugins\/MultiSites\/Controller.php(37): Piwik\Plugins\MultiSites\Controller->getSitesInfo(false)\n#6 [internal function]: Piwik\Plugins\MultiSites\Controller->index()\n#7 \/core\/FrontController.php(619):

Happens on

• index.php?action=index&date[]=yesterday&idSite=1&module=MultiSites&period=day

Expected outcome

No fatal error

@tsteur I would assume we have that problem at multiple other places as well. It's nowhere expected that date or period is given as an array in the URL. Wondering if we should try to have some kind of global solution, that maybe already throws an exception when parsing the request array or something like that.

I would first check if it's needed. If it can be done eg in checkDateFormat which is called as part of setGeneralVariablesViewAs then it be this way already covered in most places and we can avoid any global solution?

I suppose the problem might be period where it's more complicated.

If we can avoid a global solution that be good as it gives us more flexibility and it could in theory cause regressions etc (although I doubt it would). It's hard to say though without looking too much into it. A global solution could potentially work as well though, it will I suppose only restrict from using period[] etc which might not be used so far (but hard to say what plugins do). Guess even a non-global fix could break things.

I would first check if it's needed. If it can be done eg in checkDateFormat which is called as part of setGeneralVariablesViewAs then it be this way already covered in most places and we can avoid any global solution?

The problem is, that there are many places where we call Common::getRequestVar('period') without specifying the expected type. And those are often called before the view is rendered, so changing checkDateFormat wouldn't have an effect.

We could go through all usages of Common::getRequestVar('period') and Common::getRequestVar('date') and ensure the variable type 'string' is provided. This would cause an exception to be thrown as soon as an array would be provided.

@sgiehl not 100% sure but from a quick test below change would probably already cover 90% of the cases with very little effort (the API already handles things well by the looks and does not result in fatal error):

diff --git a/core/Period.php b/core/Period.php
index 4059a80155..00bb717607 100644
--- a/core/Period.php
+++ b/core/Period.php
@@ -118,6 +118,10 @@ abstract class Period
             return;
         }

+        if (!is_string($dateString)) {
+            throw new \Exception('Date parameter needs to be a string');
+        }
+
         Date::factory($dateString);
     }

Period might be bit different and might require 2 or 3 changes to cover most of the errors like below (although are likely quite a few more):

diff --git a/core/Plugin/Controller.php b/core/Plugin/Controller.php
index 241ef0632b..e19ccda1c8 100644
--- a/core/Plugin/Controller.php
+++ b/core/Plugin/Controller.php
@@ -477,7 +477,7 @@ abstract class Controller
     protected function getGraphParamsModified($paramsToSet = array())
     {
         if (!isset($paramsToSet['period'])) {
-            $period = Common::getRequestVar('period');
+            $period = Common::getRequestVar('period', 'day', 'string');
         } else {
             $period = $paramsToSet['period'];
         }
@@ -924,6 +924,9 @@ abstract class Controller
         $periodValidator = new PeriodValidator();

         $currentPeriod = Common::getRequestVar('period');
+        if (!is_string($currentPeriod)) {
+            throw new Exception('Expected period to be a string');
+        }
         $view->displayUniqueVisitors = SettingsPiwik::isUniqueVisitorsEnabled($currentPeriod);
         $availablePeriods = $periodValidator->getPeriodsAllowedForUI();

diff --git a/plugins/MultiSites/Controller.php b/plugins/MultiSites/Controller.php
index 86792958f6..62382b9fd4 100644
--- a/plugins/MultiSites/Controller.php
+++ b/plugins/MultiSites/Controller.php
@@ -75,7 +75,7 @@ class Controller extends \Piwik\Plugin\Controller
         Piwik::checkUserHasSomeViewAccess();

         $date   = Common::getRequestVar('date', 'today');
-        $period = Common::getRequestVar('period', 'day');
+        $period = Common::getRequestVar('period', 'day', 'string');

         $view = new View("<a class='mention' href='https://github.com/MultiSites'>@MultiSites</a>/getSitesInfo");

Generally sounds good though to add a Piwik::getDate() and Piwik::getPeriod similar to getModule and getAction. We just want to avoid spending too much time on this edge case

right, so we want to add 2 functions Piwik::getDate() and Piwik::getPeriod() then replace those once.

  $date   = Common::getRequestVar('date', 'today');
  $period = Common::getRequestVar('period', 'day');

Also just checking do we have a param validator sth like this

 $validatedData = $request->validate([
    'title' => ['required', 'unique:posts', 'max:255'],
    'body' => ['required'],
]);

Also just checking do we have a param validator s

We don't @peterhashair