Fix bug where some messages could get malformed in an import from a M…

…BOX file (#9510)
roundcube · Jul 17, 2024 · a8218b1 · a8218b1
1 parent 54de62c
commit a8218b1
Show file tree

Hide file tree

Showing 5 changed files with 193 additions and 38 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -61,6 +61,7 @@
 - Fix fatal error when parsing some TNEF attachments (#9462)
 - Fix double scrollbar when composing a mail with many plain text lines (#7760)
 - Fix decoding mail parts with multiple base64-encoded text blocks (#9290)
+- Fix bug where some messages could get malformed in an import from a MBOX file (#9510)
 
 ## Release 1.6.7
 

diff --git a/program/actions/mail/import.php b/program/actions/mail/import.php
@@ -59,38 +59,7 @@ public function run($args = [])
                     }
 
                     foreach ((array) $filepath as $file) {
-                        // read the first few lines to detect header-like structure
-                        $fp = fopen($file, 'r');
-                        do {
-                            $line = fgets($fp);
-                        } while ($line !== false && trim($line) == '');
-
-                        if (!preg_match('/^From .+/', $line) && !preg_match('/^[a-z-_]+:\s+.+/i', $line)) {
-                            continue;
-                        }
-
-                        $message = $lastline = '';
-                        fseek($fp, 0);
-
-                        while (($line = fgets($fp)) !== false) {
-                            // importing mbox file, split by From - lines
-                            if ($lastline === '' && strncmp($line, 'From ', 5) === 0 && strlen($line) > 5) {
-                                if (!empty($message)) {
-                                    $imported += (int) self::save_message($folder, $message);
-                                }
-
-                                $message = $line;
-                                $lastline = '';
-                                continue;
-                            }
-
-                            $message .= $line;
-                            $lastline = rtrim($line);
-                        }
-
-                        if (!empty($message)) {
-                            $imported += (int) self::save_message($folder, $message);
-                        }
+                        $imported += self::import_file($file, $folder);
 
                         // remove temp files extracted from zip
                         if (is_array($filepath)) {
@@ -116,6 +85,13 @@ public function run($args = [])
         $rcmail->output->send('iframe');
     }
 
+    /**
+     * Extract files from a zip archive
+     *
+     * @param string $path ZIP file location
+     *
+     * @return array List of extracted files
+     */
     public static function zip_extract($path)
     {
         if (!class_exists('ZipArchive', false)) {
@@ -148,11 +124,69 @@ public static function zip_extract($path)
         return $files;
     }
 
-    public static function save_message($folder, &$message)
+    /**
+     * Parse input file in mbox or eml format, save email messages
+     *
+     * @param string $file   Filename
+     * @param string $folder IMAP folder
+     *
+     * @return int Number of imported messages
+     */
+    public static function import_file($file, $folder)
+    {
+        $fp = fopen($file, 'r');
+
+        // read the first few lines to detect header-like structure
+        do {
+            $line = fgets($fp);
+        } while ($line !== false && trim($line) == '');
+
+        $format = null;
+        if (strncmp($line, 'From ', 5) === 0) {
+            $format = 'mbox';
+        } elseif (preg_match('/^[a-z-_]+:\s+.+/i', $line)) {
+            $format = 'eml';
+        } else {
+            return 0;
+        }
+
+        $imported = 0;
+        $message = '';
+
+        fseek($fp, 0);
+
+        while (($line = fgets($fp)) !== false) {
+            // importing mbox file, split by From - lines
+            if ($format == 'mbox' && strncmp($line, 'From ', 5) === 0 && strlen($line) > 5) {
+                if (strlen($message)) {
+                    $imported += (int) self::save_message($folder, $message, $format);
+                }
+
+                $message = $line;
+                $lastline = '';
+                continue;
+            }
+
+            $message .= $line;
+        }
+
+        if (strlen($message)) {
+            $imported += (int) self::save_message($folder, $message, $format);
+        }
+
+        fclose($fp);
+
+        return $imported;
+    }
+
+    /**
+     * Append the message to an IMAP folder
+     */
+    public static function save_message($folder, &$message, $format = 'mbox')
     {
         $date = null;
 
-        if (strncmp($message, 'From ', 5) === 0) {
+        if ($format == 'mbox') {
             // Extract the mbox from_line
             $pos = strpos($message, "\n");
             $from = substr($message, 0, $pos);
@@ -176,10 +210,11 @@ public static function save_message($folder, &$message)
                     // ignore
                 }
             }
+
+            // unquote ">From " lines in message body
+            $message = preg_replace('/\n>([>]*)From /', "\n\\1From ", $message);
         }
 
-        // unquote ">From " lines in message body
-        $message = preg_replace('/\n>([>]*)From /', "\n\\1From ", $message);
         $message = rtrim($message);
         $rcmail = rcmail::get_instance();
 

diff --git a/tests/ActionTestCase.php b/tests/ActionTestCase.php
@@ -34,6 +34,8 @@ public static function tearDownAfterClass(): void
         $rcmail->shutdown();
 
         \html::$doctype = 'xhtml';
+
+        $_FILES = [];
     }
 
     #[\Override]

diff --git a/tests/Actions/Mail/ImportTest.php b/tests/Actions/Mail/ImportTest.php
@@ -3,6 +3,7 @@
 namespace Roundcube\Tests\Actions\Mail;
 
 use Roundcube\Tests\ActionTestCase;
+use Roundcube\Tests\OutputJsonMock;
 
 /**
  * Test class to test rcmail_action_mail_import
@@ -14,8 +15,94 @@ class ImportTest extends ActionTestCase
      */
     public function test_class()
     {
-        $object = new \rcmail_action_mail_import();
+        $action = new \rcmail_action_mail_import();
+        $output = $this->initOutput(\rcmail_action::MODE_AJAX, 'mail', 'import');
 
-        $this->assertInstanceOf(\rcmail_action::class, $object);
+        $this->assertInstanceOf(\rcmail_action::class, $action);
+        $this->assertTrue($action->checks());
+
+        $_SERVER['REQUEST_METHOD'] = 'POST';
+
+        // No files uploaded case
+        $this->runAndAssert($action, OutputJsonMock::E_EXIT);
+
+        $result = $output->getOutput();
+
+        $this->assertSame(['Content-Type: application/json; charset=UTF-8'], $output->headers);
+        $this->assertSame('import', $result['action']);
+        // TODO: Assert error message
+        // $this->assertTrue(strpos($result['exec'], '') !== false);
+
+        // Upload a EML file
+        $_POST = [
+            '_mbox' => 'Test',
+        ];
+        $_FILES['_file'] = [
+            'name' => ['import.eml'],
+            'type' => ['message/rfc822'],
+            'tmp_name' => [__DIR__ . '/../../src/filename.eml'],
+            'error' => [null],
+            'size' => [123],
+            'id' => [123],
+        ];
+
+        // Set expected storage function calls/results
+        $storage = self::mockStorage()
+            ->registerFunction('get_folder', 'Test')
+            ->registerFunction('save_message', 123);
+
+        $this->runAndAssert($action, OutputJsonMock::E_EXIT);
+
+        $result = $output->getOutput();
+
+        $this->assertTrue(strpos($result['exec'], 'Successfully imported 1 messages') !== false);
+        $this->assertTrue(strpos($result['exec'], 'this.command("list")') !== false);
+
+        $args = $storage->methodCalls[1]['args'];
+        $this->assertSame('Test', $args[0]);
+        $this->assertTrue(strpos($args[1], 'From: "Thomas B." <[email protected]>') === 0);
+
+        // Upload a MBOX file
+        $_FILES['_file'] = [
+            'name' => ['import.eml'],
+            'type' => ['text/plain'],
+            'tmp_name' => [__DIR__ . '/../../src/import.mbox'],
+            'error' => [null],
+            'size' => [123],
+            'id' => [123],
+        ];
+
+        // Set expected storage function calls/results
+        $storage = self::mockStorage()
+            ->registerFunction('get_folder', 'Test')
+            ->registerFunction('save_message', 1)
+            ->registerFunction('save_message', 2)
+            ->registerFunction('save_message', 3);
+
+        $this->runAndAssert($action, OutputJsonMock::E_EXIT);
+
+        $result = $output->getOutput();
+
+        $this->assertTrue(strpos($result['exec'], 'Successfully imported 3 messages') !== false);
+        $this->assertTrue(strpos($result['exec'], 'this.command("list")') !== false);
+
+        $args = $storage->methodCalls[1]['args'];
+        $this->assertSame('Test', $args[0]);
+        $this->assertTrue(strpos($args[1], 'From: [email protected]') === 0);
+        $this->assertStringContainsString('1234', $args[1]);
+
+        $args = $storage->methodCalls[2]['args'];
+        $this->assertSame('Test', $args[0]);
+        $this->assertTrue(strpos($args[1], 'From: [email protected]') === 0);
+        $this->assertTrue(strpos($args[1], "\nFrom me") !== false);
+
+        $args = $storage->methodCalls[3]['args'];
+        $this->assertSame('Test', $args[0]);
+        $this->assertTrue(strpos($args[1], 'From: [email protected]') === 0);
+        $this->assertStringContainsString('XXXX', $args[1]);
+
+        // TODO: Test error handling
+        // TODO: Test ZIP file input
+        $this->markTestIncomplete();
     }
 }
diff --git a/tests/src/import.mbox b/tests/src/import.mbox
@@ -0,0 +1,30 @@
+From [email protected] Sun Jul 16 15:06:25 2023
+From: [email protected]
+To: Tom Tester <[email protected]>
+Subject: Import 1
+MIME-Version: 1.0
+Content-Type: text/plain;
+Date: Fri, 23 May 2014 19:44:50 +0200
+Message-ID: <[email protected]>
+
+1234
+From [email protected] Sun Jul 16 15:06:25 2023
+From: [email protected]
+To: Tom Tester <[email protected]>
+Subject: Import 2
+MIME-Version: 1.0
+Content-Type: text/plain;
+Date: Fri, 23 May 2014 19:44:50 +0200
+Message-ID: <[email protected]>
+
+>From me
+From [email protected] Sun Jul 16 15:06:25 2023
+From: [email protected]
+To: Tom Tester <[email protected]>
+Subject: Import 3
+MIME-Version: 1.0
+Content-Type: text/plain;
+Date: Fri, 23 May 2014 19:44:50 +0200
+Message-ID: <[email protected]>
+
+XXXX