Fix PHP8 warning (#9235)

And added tests for utils/modcss action
roundcube · Dec 3, 2023 · 17eec91 · 17eec91
1 parent b19689e
commit 17eec91
Show file tree

Hide file tree

Showing 7 changed files with 126 additions and 27 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -32,6 +32,7 @@
 - Enigma: Fix finding of a private key when decrypting a message using GnuPG v2.3
 - Fix page jump menu flickering on click (#9196)
 - Update to TinyMCE 5.10.9 security release (#9228)
+- Fix PHP8 warning (#9235)
 
 ## Release 1.6.5
 

diff --git a/program/actions/utils/modcss.php b/program/actions/utils/modcss.php
@@ -27,17 +27,20 @@ class rcmail_action_utils_modcss extends rcmail_action
      */
     public function run($args = [])
     {
-        $url = preg_replace('![^a-z0-9.-]!i', '', $_GET['_u']);
+        $rcmail = rcmail::get_instance();
 
-        if ($url === null || !($realurl = $_SESSION['modcssurls'][$url])) {
-            http_response_code(403);
-            exit("Unauthorized request");
+        $url = rcube_utils::get_input_string('_u', rcube_utils::INPUT_GET);
+        $url = preg_replace('![^a-z0-9.-]!i', '', $url);
+
+        if ($url === null || empty($_SESSION['modcssurls'][$url])) {
+            $rcmail->output->sendExitError(403, "Unauthorized request");
         }
 
+        $realurl = $_SESSION['modcssurls'][$url];
+
         // don't allow any other connections than http(s)
         if (!preg_match('~^https?://~i', $realurl, $matches)) {
-            http_response_code(403);
-            exit("Invalid URL");
+            $rcmail->output->sendExitError(403, "Invalid URL");
         }
 
         $source = false;
@@ -57,17 +60,20 @@ public function run($args = [])
             rcube::raise_error($e, true, false);
         }
 
+        $cid    = rcube_utils::get_input_string('_c', rcube_utils::INPUT_GET);
+        $prefix = rcube_utils::get_input_string('_p', rcube_utils::INPUT_GET);
+
+        $container_id = preg_replace('/[^a-z0-9]/i', '', $cid);
+        $css_prefix   = preg_replace('/[^a-z0-9]/i', '', $prefix);
         $ctype_regexp = '~^text/(css|plain)~i';
-        $container_id = isset($_GET['_c']) ? preg_replace('/[^a-z0-9]/i', '', $_GET['_c']) : '';
-        $css_prefix   = isset($_GET['_p']) ? preg_replace('/[^a-z0-9]/i', '', $_GET['_p']) : '';
 
         if ($source !== false && $ctype && preg_match($ctype_regexp, $ctype)) {
-            header('Content-Type: text/css');
-            echo rcube_utils::mod_css_styles($source, $container_id, false, $css_prefix);
-            exit;
+            $rcmail->output->sendExit(
+                rcube_utils::mod_css_styles($source, $container_id, false, $css_prefix),
+                ['Content-Type: text/css']
+            );
         }
 
-        http_response_code(404);
-        exit("Invalid response returned by server");
+        $rcmail->output->sendExitError(404, "Invalid response returned by server");
     }
 }
diff --git a/program/include/rcmail_output.php b/program/include/rcmail_output.php
@@ -147,4 +147,18 @@ public function sendExit($body = '', $headers = [])
         print $body;
         exit;
     }
+
+    /**
+     * A helper to send HTTP error code and message to the browser, and exit.
+     *
+     * @param int    $code    The HTTP error code
+     * @param string $message The HTTP error message
+     *
+     * @return void
+     */
+    public function sendExitError($code, $message = '')
+    {
+        http_response_code($code);
+        exit($message);
+    }
 }
diff --git a/tests/Actions/Utils/Html2text.php b/tests/Actions/Utils/Html2text.php
@@ -7,16 +7,6 @@
  */
 class Actions_Utils_Html2text extends ActionTestCase
 {
-    /**
-     * Class constructor
-     */
-    function test_class()
-    {
-        $object = new rcmail_action_utils_html2text;
-
-        $this->assertInstanceOf('rcmail_action', $object);
-    }
-
     /**
      * Test for run()
      */
@@ -28,6 +18,7 @@ function test_run()
 
         $output = $this->initOutput(rcmail_action::MODE_HTTP, 'utils', 'html2text');
 
+        $this->assertInstanceOf('rcmail_action', $object);
         $this->assertTrue($object->checks());
 
         $this->runAndAssert($object, OutputHtmlMock::E_EXIT);

diff --git a/tests/Actions/Utils/Modcss.php b/tests/Actions/Utils/Modcss.php
@@ -8,12 +8,61 @@
 class Actions_Utils_Modcss extends ActionTestCase
 {
     /**
-     * Class constructor
+     * Test for run()
      */
-    function test_class()
+    function test_run()
     {
-        $object = new rcmail_action_utils_modcss;
+        $action = new rcmail_action_utils_modcss;
+        $output = $this->initOutput(rcmail_action::MODE_HTTP, 'utils', 'modcss');
 
-        $this->assertInstanceOf('rcmail_action', $object);
+        $this->assertInstanceOf('rcmail_action', $action);
+        $this->assertTrue($action->checks());
+
+        // No input parameters
+        $this->runAndAssert($action, OutputHtmlMock::E_EXIT);
+
+        $this->assertSame(403, $output->getProperty('errorCode'));
+        $this->assertSame("Unauthorized request", $output->getProperty('errorMessage'));
+        $this->assertSame(null, $output->getOutput());
+
+        // Invalid url
+        $_GET['_u'] = '****';
+        $this->runAndAssert($action, OutputHtmlMock::E_EXIT);
+
+        $this->assertSame(403, $output->getProperty('errorCode'));
+        $this->assertSame("Unauthorized request", $output->getProperty('errorMessage'));
+        $this->assertSame(null, $output->getOutput());
+
+        // Valid url but not "registered"
+        $url = 'https://raw.githubusercontent.com/roundcube/roundcubemail/master/aaaaaaaaaa';
+        $key = 'tmp-123.css';
+        $_GET['_u'] = $key;
+
+        $this->runAndAssert($action, OutputHtmlMock::E_EXIT);
+
+        $this->assertSame(403, $output->getProperty('errorCode'));
+        $this->assertSame("Unauthorized request", $output->getProperty('errorMessage'));
+        $this->assertSame(null, $output->getOutput());
+
+        // Valid url pointing to non-existing resource
+        $_SESSION['modcssurls'][$key] = $url;
+
+        $this->runAndAssert($action, OutputHtmlMock::E_EXIT);
+
+        $this->assertSame(404, $output->getProperty('errorCode'));
+        $this->assertSame("Invalid response returned by server", $output->getProperty('errorMessage'));
+        $this->assertSame(null, $output->getOutput());
+
+        // Valid url pointing to an existing resource
+        $url = 'https://raw.githubusercontent.com/roundcube/roundcubemail/master/program/resources/tinymce/content.css';
+        $_SESSION['modcssurls'][$key] = $url;
+        $_GET['_p'] = 'prefix';
+        $_GET['_c'] = 'cid';
+
+        $this->runAndAssert($action, OutputHtmlMock::E_EXIT);
+
+        $this->assertSame(null, $output->getProperty('errorCode'));
+        $this->assertSame(['Content-Type: text/css'], $output->getProperty('headers'));
+        $this->assertStringContainsString('#cid div.prefixpre', $output->getOutput());
     }
 }
diff --git a/tests/OutputHtmlMock.php b/tests/OutputHtmlMock.php
@@ -29,6 +29,8 @@ class OutputHtmlMock extends rcmail_output_html
 
     public $output;
     public $headers  = [];
+    public $errorCode;
+    public $errorMessage;
     public $template = '';
 
     /**
@@ -85,6 +87,20 @@ public function sendExit($body = '', $headers = [])
         throw new ExitException("Output sent", self::E_EXIT);
     }
 
+    /**
+     * A helper to send HTTP error code and message to the browser, and exit.
+     *
+     * @param int    $code    The HTTP error code
+     * @param string $message The HTTP error message
+     */
+    public function sendExitError($code, $message = '')
+    {
+        $this->errorCode = $code;
+        $this->errorMessage = $message;
+
+        throw new ExitException("Output sent (error)", self::E_EXIT);
+    }
+
     /**
      * Process template and write to stdOut
      *
@@ -129,6 +145,9 @@ public function reset($all = false)
         $this->headers  = [];
         $this->output   = null;
         $this->template = null;
+
+        $this->errorCode    = null;
+        $this->errorMessage = null;
     }
 
     /**

diff --git a/tests/OutputJsonMock.php b/tests/OutputJsonMock.php
@@ -29,6 +29,8 @@ class OutputJsonMock extends rcmail_output_json
 
     public $output;
     public $headers = [];
+    public $errorCode;
+    public $errorMessage;
 
     /**
      * Redirect to a certain url
@@ -80,6 +82,20 @@ public function sendExit($body = '', $headers = [])
         throw new ExitException("Output sent", self::E_EXIT);
     }
 
+    /**
+     * A helper to send HTTP error code and message to the browser, and exit.
+     *
+     * @param int    $code    The HTTP error code
+     * @param string $message The HTTP error message
+     */
+    public function sendExitError($code, $message = '')
+    {
+        $this->errorCode = $code;
+        $this->errorMessage = $message;
+
+        throw new ExitException("Output sent (error)", self::E_EXIT);
+    }
+
     /**
      * Show error page and terminate script execution
      *
@@ -112,6 +128,9 @@ public function reset()
         $this->headers     = [];
         $this->output      = null;
         $this->header_sent = false;
+
+        $this->errorCode    = null;
+        $this->errorMessage = null;
     }
 
     /**