Prevent possible error when action is provided as array (#18706)

* Prevent possible error when action is provided as array

* Use Piwik::getModule / Piwik::getAction in a couple more places

* fix test

5 files changed, 12 insertions, 8 deletions

diff --git a/config/environment/test.php b/config/environment/test.php
index e6cbbd62f0..0a9aea959a 100644
--- a/config/environment/test.php
+++ b/config/environment/test.php
@@ -1,5 +1,6 @@
 <?php
 
+use Piwik\Piwik;
 use Psr\Container\ContainerInterface;
 use Piwik\Common;
 use Piwik\Tests\Framework\Mock\FakeAccess;
@@ -121,7 +122,7 @@ return array(
         })),
 
         array('Test.Mail.send', \DI\value(function (\PHPMailer\PHPMailer\PHPMailer $mail) {
-            $outputFile = PIWIK_INCLUDE_PATH . '/tmp/' . Common::getRequestVar('module', '') . '.' . Common::getRequestVar('action', '') . '.mail.json';
+            $outputFile = PIWIK_INCLUDE_PATH . '/tmp/' . Piwik::getModule() . '.' . Piwik::getAction() . '.mail.json';
             $outputContent = str_replace("=\n", "", $mail->Body ?: $mail->AltBody);
             $outputContent = str_replace("=0A", "\n", $outputContent);
             $outputContent = str_replace("=3D", "=", $outputContent);
diff --git a/core/FrontController.php b/core/FrontController.php
index 7ac273c089..97f7d47eb6 100644
--- a/core/FrontController.php
+++ b/core/FrontController.php
@@ -460,6 +460,10 @@ class FrontController extends Singleton
 
         if (is_null($action)) {
             $action = Common::getRequestVar('action', false);
+            if ($action !== false) {
+                // If a value was provided, check it has the correct type.
+                $action = Common::getRequestVar('action', null, 'string');
+            }
         }
 
         if (Session::isSessionStarted()) {
diff --git a/plugins/CoreUpdater/CoreUpdater.php b/plugins/CoreUpdater/CoreUpdater.php
index 79821d287c..af95ae1c09 100644
--- a/plugins/CoreUpdater/CoreUpdater.php
+++ b/plugins/CoreUpdater/CoreUpdater.php
@@ -41,8 +41,8 @@ class CoreUpdater extends \Piwik\Plugin
             return;
         }
 
-        $module = Common::getRequestVar('module', '', 'string');
-        $action = Common::getRequestVar('action', '', 'string');
+        $module = Piwik::getModule();
+        $action = Piwik::getAction();
 
         if ($module == 'CoreUpdater'
             // Proxy module is used to redirect users to piwik.org, should still work when Piwik must be updated
diff --git a/plugins/Login/Login.php b/plugins/Login/Login.php
index 8b0c299350..df2e740656 100644
--- a/plugins/Login/Login.php
+++ b/plugins/Login/Login.php
@@ -175,8 +175,8 @@ class Login extends \Piwik\Plugin
 
     private function shouldHandleRememberMe()
     {
-        $module = Common::getRequestVar('module', false);
-        $action = Common::getRequestVar('action', false);
+        $module = Piwik::getModule();
+        $action = Piwik::getAction();
         return ($module == 'Login' || $module == 'CoreHome') && (empty($action) || $action == 'index' || $action == 'login');
     }
 
@@ -221,8 +221,7 @@ class Login extends \Piwik\Plugin
         $login = StaticContainer::get(\Piwik\Auth::class)->getLogin();
         if (empty($login) || $login == 'anonymous') {
             $login = Common::getRequestVar('form_login', false);
-            $action = Common::getRequestVar('action', false);
-            if ($action == 'logme') {
+            if (Piwik::getAction() === 'logme') {
                 $login = Common::getRequestVar('login', $login);
             }
         }
diff --git a/tests/PHPUnit/Integration/FrontControllerTest.php b/tests/PHPUnit/Integration/FrontControllerTest.php
index 5def1a1b63..4aed67aa9b 100644
--- a/tests/PHPUnit/Integration/FrontControllerTest.php
+++ b/tests/PHPUnit/Integration/FrontControllerTest.php
@@ -48,7 +48,7 @@ FORMAT;
         $this->assertEquals('error', $response['result']);
 
         $expectedFormat = <<<FORMAT
-test message on {includePath}/tests/resources/trigger-fatal-exception.php(23) #0 [internal function]: {closure}('CoreHome', 'index', Array) #1 {includePath}/core/EventDispatcher.php(141): call_user_func_array(Object(Closure), Array) #2 {includePath}/core/Piwik.php(845): Piwik\EventDispatcher-&gt;postEvent('Request.dispatc...', Array, false, Array) #3 {includePath}/core/FrontController.php(602): Piwik\Piwik::postEvent('Request.dispatc...', Array) #4 {includePath}/core/FrontController.php(168): Piwik\FrontController-&gt;doDispatch('CoreHome', 'index', Array) #5 {includePath}/tests/resources/trigger-fatal-exception.php(31): Piwik\FrontController-&gt;dispatch('CoreHome', 'index') #6 {main}
+test message on {includePath}/tests/resources/trigger-fatal-exception.php(23) #0 [internal function]: {closure}('CoreHome', 'index', Array) #1 {includePath}/core/EventDispatcher.php(141): call_user_func_array(Object(Closure), Array) #2 {includePath}/core/Piwik.php(845): Piwik\EventDispatcher-&gt;postEvent('Request.dispatc...', Array, false, Array) #3 {includePath}/core/FrontController.php(606): Piwik\Piwik::postEvent('Request.dispatc...', Array) #4 {includePath}/core/FrontController.php(168): Piwik\FrontController-&gt;doDispatch('CoreHome', 'index', Array) #5 {includePath}/tests/resources/trigger-fatal-exception.php(31): Piwik\FrontController-&gt;dispatch('CoreHome', 'index') #6 {main}
 FORMAT;
         $this->assertStringMatchesFormat($expectedFormat, $response['message']);
     }