Browse Source

Fix bug where some messages could get malformed in an import from a MBOX file (#9510)

pull/9552/head
Aleksander Machniak 1 year ago
parent
commit
a8218b1eeb
  1. 1
      CHANGELOG.md
  2. 107
      program/actions/mail/import.php
  3. 2
      tests/ActionTestCase.php
  4. 91
      tests/Actions/Mail/ImportTest.php
  5. 30
      tests/src/import.mbox

1
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

107
program/actions/mail/import.php

@ -59,38 +59,7 @@ class rcmail_action_mail_import extends rcmail_action
}
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 @@ class rcmail_action_mail_import extends rcmail_action
$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 @@ class rcmail_action_mail_import extends rcmail_action
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 @@ class rcmail_action_mail_import extends rcmail_action
// 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();

2
tests/ActionTestCase.php

@ -34,6 +34,8 @@ class ActionTestCase extends TestCase
$rcmail->shutdown();
\html::$doctype = 'xhtml';
$_FILES = [];
}
#[\Override]

91
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." <thomas@roundcube.net>') === 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: test@rc.net') === 0);
$this->assertStringContainsString('1234', $args[1]);
$args = $storage->methodCalls[2]['args'];
$this->assertSame('Test', $args[0]);
$this->assertTrue(strpos($args[1], 'From: test1@rc.net') === 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: test2@rc.net') === 0);
$this->assertStringContainsString('XXXX', $args[1]);
// TODO: Test error handling
// TODO: Test ZIP file input
$this->markTestIncomplete();
}
}

30
tests/src/import.mbox

@ -0,0 +1,30 @@
From test@rc.net Sun Jul 16 15:06:25 2023
From: test@rc.net
To: Tom Tester <tb@tester.local>
Subject: Import 1
MIME-Version: 1.0
Content-Type: text/plain;
Date: Fri, 23 May 2014 19:44:50 +0200
Message-ID: <de75@tester.local>
1234
From test1@rc.net Sun Jul 16 15:06:25 2023
From: test1@rc.net
To: Tom Tester <tb@tester.local>
Subject: Import 2
MIME-Version: 1.0
Content-Type: text/plain;
Date: Fri, 23 May 2014 19:44:50 +0200
Message-ID: <de75@tester.local>
>From me
From test2@rc.net Sun Jul 16 15:06:25 2023
From: test2@rc.net
To: Tom Tester <tb@tester.local>
Subject: Import 3
MIME-Version: 1.0
Content-Type: text/plain;
Date: Fri, 23 May 2014 19:44:50 +0200
Message-ID: <de75@tester.local>
XXXX
Loading…
Cancel
Save