Add tests for contacts export action, fix PHP8 issues

roundcube · Nov 2, 2020 · e99fb39 · e99fb39
1 parent 4a3bed3
commit e99fb39
Show file tree

Hide file tree

Showing 13 changed files with 147 additions and 15 deletions.
diff --git a/program/actions/contacts/export.php b/program/actions/contacts/export.php
@@ -119,12 +119,12 @@ public function run($args = [])
         $result = $plugin['result'];
 
         if ($plugin['abort']) {
-            exit;
+            $rcmail->output->sendExit();
         }
 
         // send downlaod headers
-        header('Content-Type: text/vcard; charset=' . RCUBE_CHARSET);
-        header('Content-Disposition: attachment; filename="contacts.vcf"');
+        $rcmail->output->header('Content-Type: text/vcard; charset=' . RCUBE_CHARSET);
+        $rcmail->output->header('Content-Disposition: attachment; filename="contacts.vcf"');
 
         while ($result && ($row = $result->next())) {
             if (!empty($CONTACTS)) {
@@ -138,7 +138,7 @@ public function run($args = [])
             echo rcube_vcard::rfc2425_fold($row['vcard']) . rcube_vcard::$eol;
         }
 
-        exit;
+        $rcmail->output->sendExit();
     }
 
     /**

diff --git a/program/include/rcmail.php b/program/include/rcmail.php
@@ -335,7 +335,7 @@ public function get_address_book($id, $writeable = false)
             $plugin = $this->plugins->exec_hook('addressbook_get', array('id' => $id, 'writeable' => $writeable));
 
             // plugin returned instance of a rcube_addressbook
-            if ($plugin['instance'] instanceof rcube_addressbook) {
+            if (!empty($plugin['instance']) && $plugin['instance'] instanceof rcube_addressbook) {
                 $contacts = $plugin['instance'];
             }
         }
@@ -483,7 +483,7 @@ public function get_address_sources($writeable = false, $skip_hidden = false)
                 unset($list[$idx]);
             }
             // remove from list if hidden as requested
-            else if ($skip_hidden && $item['hidden']) {
+            else if ($skip_hidden && !empty($item['hidden'])) {
                 unset($list[$idx]);
             }
         }

diff --git a/program/include/rcmail_output.php b/program/include/rcmail_output.php
@@ -132,7 +132,7 @@ public function header($header, $replace = true)
      * @param string $body    The output body
      * @param array  $headers Headers
      */
-    public function sendExit($body, $headers = [])
+    public function sendExit($body = '', $headers = [])
     {
         foreach ($headers as $header) {
             header($header);

diff --git a/program/include/rcmail_output_html.php b/program/include/rcmail_output_html.php
@@ -537,8 +537,8 @@ public function show_message($message, $type='notice', $vars=null, $override=tru
     public function reset($all = false)
     {
         $framed = $this->framed;
-        $task   = $this->env['task'];
-        $env    = $all ? null : array_intersect_key($this->env, array('extwin'=>1, 'framed'=>1));
+        $task   = isset($this->env['task']) ? $this->env['task'] : '';
+        $env    = $all ? null : array_intersect_key($this->env, array('extwin' => 1, 'framed' => 1));
 
         // keep jQuery-UI files
         $css_files = $script_files = array();

diff --git a/program/lib/Roundcube/rcube.php b/program/lib/Roundcube/rcube.php
@@ -1019,10 +1019,9 @@ public function check_request($mode = rcube_utils::INPUT_POST)
         }
 
         // default method of securing requests
-        $token   = rcube_utils::get_input_value('_token', $mode);
-        $sess_id = $_COOKIE[ini_get('session.name')];
+        $token = rcube_utils::get_input_value('_token', $mode);
 
-        if (empty($sess_id) || $token !== $sess_tok) {
+        if (empty($_COOKIE[ini_get('session.name')]) || $token !== $sess_tok) {
             $this->request_status = self::REQUEST_ERROR_TOKEN;
             return false;
         }
@@ -1416,6 +1415,9 @@ public static function raise_error($arg = array(), $log = false, $terminate = fa
 
         // terminate script
         if ($terminate) {
+            if (defined('ROUNDCUBE_TEST_MODE') && ROUNDCUBE_TEST_MODE) {
+                throw new Exception('Error raised');
+            }
             exit(1);
         }
     }

diff --git a/program/lib/Roundcube/rcube_addressbook.php b/program/lib/Roundcube/rcube_addressbook.php
@@ -67,6 +67,11 @@ abstract class rcube_addressbook
         'email'     => array('limit'=>1)
     );
 
+    /**
+     * vCard additional fields mapping
+     */
+    public $vcard_map = [];
+
     protected $error;
 
     /**

diff --git a/tests/ActionTestCase.php b/tests/ActionTestCase.php
@@ -28,6 +28,13 @@ static function tearDownAfterClass()
         $rcmail->shutdown();
     }
 
+    public function setUp()
+    {
+        $_GET     = [];
+        $_POST    = [];
+        $_REQUEST = [];
+    }
+
     /**
      * Initialize the testing suite
      */
@@ -182,10 +189,19 @@ protected function runAndAssert($action, $expected_code)
         $rcmail->output->reset(true);
 
         try {
+            StderrMock::start();
             $action->run();
+            StderrMock::stop();
         }
         catch (ExitException $e) {
             $this->assertSame($expected_code, $e->getCode());
         }
+        catch (Exception $e) {
+            if ($e->getMessage() == 'Error raised' && $expected_code == OutputHtmlMock::E_EXIT) {
+                return;
+            }
+
+            throw $e;
+        }
     }
 }
diff --git a/tests/Actions/Contacts/Export.php b/tests/Actions/Contacts/Export.php
@@ -16,4 +16,104 @@ function test_class()
 
         $this->assertInstanceOf('rcmail_action', $object);
     }
+
+    /**
+     * Test exporting all contacts
+     */
+    function test_export_all()
+    {
+        $action = new rcmail_action_contacts_export;
+        $output = $this->initOutput(rcmail_action::MODE_HTTP, 'contacts', 'export');
+
+        $this->assertTrue($action->checks());
+
+        self::initDB('contacts');
+
+        $_GET = ['_source' => '0'];
+        $_POST = [];
+
+        // Here we expect request security check error
+        $this->runAndAssert($action, OutputHtmlMock::E_EXIT);
+
+        $this->assertSame('ERROR: Request security check failed', trim(StderrMock::$output));
+
+        // Now we'll try with the proper token
+        $_SESSION['request_token']           = 'secure';
+        $_SERVER['HTTP_X_ROUNDCUBE_REQUEST'] = 'secure';
+
+        ob_start();
+        $this->runAndAssert($action, OutputHtmlMock::E_EXIT);
+        $vcf = ob_get_contents();
+        ob_end_clean();
+
+        $this->assertSame([
+                'Content-Type: text/vcard; charset=UTF-8',
+                'Content-Disposition: attachment; filename="contacts.vcf"'
+            ], $output->headers
+        );
+        $this->assertSame(6, substr_count($vcf, 'BEGIN:VCARD'));
+        $this->assertSame(6, substr_count($vcf, 'END:VCARD'));
+        $this->assertSame(1, substr_count($vcf, 'FN:Jane Stalone'));
+    }
+
+    /**
+     * Test exporting selected contacts
+     *
+     * @depends test_export_all
+     */
+    function test_export_selected()
+    {
+        $action = new rcmail_action_contacts_export;
+        $output = $this->initOutput(rcmail_action::MODE_HTTP, 'contacts', 'export');
+
+        $this->assertTrue($action->checks());
+
+        $cids   = [];
+        $db     = rcmail::get_instance()->get_dbh();
+        $query  = $db->query("SELECT `contact_id` FROM `contacts` WHERE `email` IN ('j.rian@gmail.com', 'g.bush@gov.com')");
+        while ($result = $db->fetch_assoc($query)) {
+            $cids[] = $result['contact_id'];
+        }
+
+        $_GET = ['_source' => '0', '_cid' => implode(',', $cids)];
+        // TODO: This really shouldn't be needed
+        $_REQUEST = ['_cid' => implode(',', $cids)];
+
+        $_SESSION['request_token']           = 'secure';
+        $_SERVER['HTTP_X_ROUNDCUBE_REQUEST'] = 'secure';
+
+        ob_start();
+        $this->runAndAssert($action, OutputHtmlMock::E_EXIT);
+        $vcf = ob_get_contents();
+        ob_end_clean();
+
+        $this->assertSame([
+                'Content-Type: text/vcard; charset=UTF-8',
+                'Content-Disposition: attachment; filename="contacts.vcf"'
+            ], $output->headers
+        );
+        $this->assertSame(2, substr_count($vcf, 'BEGIN:VCARD'));
+        $this->assertSame(2, substr_count($vcf, 'END:VCARD'));
+        $this->assertSame(0, substr_count($vcf, 'FN:Jane Stalone'));
+        $this->assertSame(1, substr_count($vcf, 'FN:Jack Rian'));
+        $this->assertSame(1, substr_count($vcf, 'FN:George Bush'));
+    }
+
+    /**
+     * Test exporting search result
+     *
+     * @depends test_export_all
+     */
+    function test_export_search()
+    {
+        $this->markTestIncomplete();
+    }
+
+    /**
+     * Test prepare_for_export() method
+     */
+    function test_prepare_for_export()
+    {
+        $this->markTestIncomplete();
+    }
 }
diff --git a/tests/OutputHtmlMock.php b/tests/OutputHtmlMock.php
@@ -74,7 +74,7 @@ public function send($templ = null, $exit = true)
      * @param string $body    The output body
      * @param array  $headers Headers
      */
-    public function sendExit($body, $headers = [])
+    public function sendExit($body = '', $headers = [])
     {
         foreach ($headers as $header) {
             $this->header($header);

diff --git a/tests/OutputJsonMock.php b/tests/OutputJsonMock.php
@@ -69,7 +69,7 @@ public function send()
      * @param string $body    The output body
      * @param array  $headers Headers
      */
-    public function sendExit($body, $headers = [])
+    public function sendExit($body = '', $headers = [])
     {
         foreach ($headers as $header) {
             $this->header($header);

diff --git a/tests/StderrMock.php b/tests/StderrMock.php
@@ -24,6 +24,7 @@
  */
 class StderrMock extends php_user_filter
 {
+    public static $registered = false;
     public static $redirect;
     public static $output = '';
 
@@ -40,7 +41,11 @@ function filter($in, $out, &$consumed, $closing)
 
     public static function start()
     {
-        stream_filter_register("redirect", "StderrMock") or die("Failed to register filter");
+        if (!self::$registered) {
+            stream_filter_register("redirect", "StderrMock") or die("Failed to register filter");
+            self::$registered = true;
+        }
+
         self::$output = '';
         self::$redirect = stream_filter_prepend(STDERR, "redirect", STREAM_FILTER_WRITE);
     }

diff --git a/tests/bootstrap.php b/tests/bootstrap.php
@@ -24,6 +24,7 @@
 
 if (!defined('INSTALL_PATH')) define('INSTALL_PATH', realpath(__DIR__ . '/..') . '/' );
 
+define('ROUNDCUBE_TEST_MODE', true);
 define('TESTS_DIR', __DIR__ . '/');
 
 if (@is_dir(TESTS_DIR . 'config')) {

diff --git a/tests/phpunit.xml b/tests/phpunit.xml
@@ -2,6 +2,9 @@
     backupGlobals="false"
     bootstrap="bootstrap.php"
     colors="true"
+    convertErrorsToExceptions="true"
+    convertNoticesToExceptions="false"
+    convertWarningsToExceptions="true"
 >
     <testsuites>
         <testsuite name="Framework">