mail attachments: handle multipart/related attachments

src/Event/SystemEvents.php

@@ -50,4 +50,12 @@ final class SystemEvents
      * @see CommitRepository::addIssues
      */
     const SCM_COMMIT_ASSOCIATED = 'scm.commit.associated';
+
+    /**
+     * Event Fired when MailMessage was created from IMAP Connection.
+     *
+     * @since 3.4.0
+     * @see ImapMessage::createFromImap
+     */
+    const MAIL_LOADED_IMAP = 'mail.loaded.imap';
 }

src/Mail/Helper/TextMessage.php

@@ -32,7 +32,7 @@ public function __construct(MailMessage $message)
 
     public function getMessageBody()
     {
-        $text = $html = [];
+        $text = $alttext = $html = [];
         foreach ($this->message as $part) {
             $headers = $part->getHeaders();
             $ctype = $part->getHeaderField('Content-Type');
@@ -44,6 +44,15 @@ public function getMessageBody()
             $charset = $part->getHeaderField('Content-Type', 'charset');
 
             switch ($ctype) {
+                case 'multipart/related':
+                    // multipart/related is likely a container for html with image multiparts
+                    // see https://tools.ietf.org/html/rfc2387
+                    //
+                    // from multipart related, extract body if text parts are missing.
+                    $alttext[] = (new self($part))->getMessageBody();
+
+                    break;
+
                 case 'multipart/alternative':
                     $text[] = (new self($part))->getMessageBody();
                     break;
@@ -87,6 +96,11 @@ public function getMessageBody()
             }
         }
 
+        // alternative text present but no main text, fill it
+        if ($alttext && !$text) {
+            $text = $alttext;
+        }
+
         if ($text) {
             return implode("\n\n", $text);
         }

src/Mail/ImapMessage.php

@@ -15,7 +15,10 @@
 
 use Date_Helper;
 use DateTime;
+use Eventum\Event\SystemEvents;
+use Eventum\EventDispatcher\EventManager;
 use InvalidArgumentException;
+use Symfony\Component\EventDispatcher\GenericEvent;
 use Zend\Mail\Header\GenericHeader;
 use Zend\Mail\Storage as ZendMailStorage;
 use Zend\Mail\Storage\Message;
@@ -87,7 +90,8 @@ public static function createFromImap($mbox, $num, $info)
             }
         }
 
-        $message = new self(['root' => true, 'headers' => $headers, 'content' => $content, 'flags' => $flags]);
+        $parameters = ['root' => true, 'headers' => $headers, 'content' => $content, 'flags' => $flags];
+        $message = new self($parameters);
 
         // set MailDate to $message object, as it's not available in message headers, only in IMAP itself
         // this likely "message received date"
@@ -100,6 +104,9 @@ public static function createFromImap($mbox, $num, $info)
         $message->info = $info;
         $message->imapheaders = $imapheaders;
 
+        $event = new GenericEvent($message, $parameters);
+        EventManager::dispatch(SystemEvents::MAIL_LOADED_IMAP, $event);
+
         return $message;
     }
 

src/Mail/MailAttachment.php

@@ -16,12 +16,13 @@
 use Eventum\Mail\Helper\DecodePart;
 use Zend\Mail;
 use Zend\Mail\Header\ContentType;
+use Zend\Mail\Storage;
 use Zend\Mail\Storage\Message;
 use Zend\Mime\Part;
 
 class MailAttachment
 {
-    /** @var MailMessage */
+    /** @var MailMessage|Storage\Part\PartInterface */
     private $message;
 
     public function __construct(MailMessage $message)
@@ -34,6 +35,7 @@ public function __construct(MailMessage $message)
      * inline text messages are not accounted as attachments.
      *
      * TODO: handle application/pgp-signature, application/ms-tnef?
+     *
      * @see https://github.com/eventum/eventum/blob/v3.2.1/lib/eventum/class.mime_helper.php#L740-L753
      *
      * @return  bool
@@ -45,39 +47,13 @@ public function hasAttachments()
             return false;
         }
 
-        $has_attachments = 0;
-
-        // check what really the attachments are
-        /** @var MailMessage $part */
         foreach ($this->message as $part) {
-            $is_attachment = 0;
-            $disposition = $filename = null;
-
-            $ctype = $part->getHeaderField('Content-Type');
-            if ($part->getHeaders()->has('Content-Disposition')) {
-                $disposition = $part->getHeaderField('Content-Disposition');
-                $filename = $part->getHeaderField('Content-Disposition', 'filename');
-                $is_attachment = $disposition === 'attachment' || $filename;
-            }
-
-            if (in_array($ctype, ['text/plain', 'text/html', 'text/enriched'])) {
-                $has_attachments |= $is_attachment;
-            } elseif ($ctype === 'multipart/related') {
-                // multipart/related may have subparts (inline html)
-                $attachment = new self($part);
-                $has_attachments |= $attachment->hasAttachments();
-            } else {
-                // avoid treating forwarded messages as attachments
-                $is_attachment |= ($disposition === 'inline' && $ctype !== 'message/rfc822');
-                // handle inline images
-                $type = current(explode('/', $ctype));
-                $is_attachment |= $type === 'image';
-
-                $has_attachments |= $is_attachment;
+            if ($this->isAttachment($part)) {
+                return true;
             }
         }
 
-        return (bool)$has_attachments;
+        return false;
     }
 
     /**
@@ -102,6 +78,28 @@ public function getAttachments()
                 continue;
             }
 
+            if ($type === 'multipart/related') {
+                // get attachments from multipart/related
+                $subpart = new self($part);
+
+                // only include non text/html
+                // this will resemble previous eventum behavior
+                // whether that's correct is another topic
+                foreach ($subpart->getAttachments() as $attachment) {
+                    if ($attachment['filetype'] === 'text/html') {
+                        continue;
+                    }
+                    $attachments[] = $attachment;
+                }
+
+                // don't add related part itself
+                continue;
+            }
+
+            if (!$this->isAttachment($part)) {
+                continue;
+            }
+
             // attempt to extract filename
             // 1. try Content-Type: name parameter
             // 2. try Content-Disposition: filename parameter
@@ -126,23 +124,44 @@ public function getAttachments()
                 'filetype' => $type,
                 'blob' => (new DecodePart($part))->decode(),
             ];
+        }
 
-            if ($type === 'multipart/related') {
-                // get attachments from multipart/related
-                $subpart = new self($part);
+        return $attachments;
+    }
 
-                // only include non text/html
-                // this will resemble previous eventum behavior
-                // whether that's correct is another topic
-                foreach ($subpart->getAttachments() as $attachment) {
-                    if ($attachment['filetype'] === 'text/html') {
-                        continue;
-                    }
-                    $attachments[] = $attachment;
-                }
-            }
+    /**
+     * @param MailMessage $part
+     * @return bool
+     */
+    private function isAttachment(MailMessage $part)
+    {
+        $is_attachment = false;
+        $disposition = $filename = null;
+
+        $ctype = $part->getHeaderField('Content-Type');
+        if ($part->getHeaders()->has('Content-Disposition')) {
+            $disposition = $part->getHeaderField('Content-Disposition');
+            $filename = $part->getHeaderField('Content-Disposition', 'filename');
+            $is_attachment = $disposition === 'attachment' || $filename;
         }
 
-        return $attachments;
+        if (in_array($ctype, ['text/plain', 'text/html', 'text/enriched'], true)) {
+            return $is_attachment;
+        }
+
+        if ($ctype === 'multipart/related') {
+            // multipart/related may have subparts (inline html)
+            return (new self($part))->hasAttachments();
+        }
+
+        // avoid treating forwarded messages as attachments
+        if ($disposition === 'inline' && $ctype !== 'message/rfc822') {
+            return true;
+        }
+
+        // handle inline images
+        $type = current(explode('/', $ctype));
+
+        return $type === 'image';
     }
 }

tests/Mail/AttachmentTest.php

@@ -118,4 +118,19 @@ public function testMultipartAlternative()
         $attachments = $attachment->getAttachments();
         $this->assertCount(1, $attachments);
     }
+
+    /**
+     * process multipart/related,
+     * should not consider text/plain and multipart/related as attachments.
+     */
+    public function testMultipartRelatedWithText()
+    {
+        $content = $this->readDataFile('multipart-related.eml');
+        $mail = MailMessage::createFromString($content);
+        $attachment = $mail->getAttachment();
+
+        $this->assertTrue($attachment->hasAttachments());
+        $attachments = $attachment->getAttachments();
+        $this->assertCount(1, $attachments);
+    }
 }

tests/Mail/TextMessageTest.php

@@ -24,28 +24,33 @@ class TextMessageTest extends TestCase
     public function testTextMessage($dataFile, $expectedText)
     {
         $mail = MailMessage::createFromFile($this->getDataFile($dataFile));
-        $this->assertEquals($expectedText, $mail->getMessageBody());
+        $textBody = trim($mail->getMessageBody());
+        $this->assertEquals($expectedText, $textBody);
     }
 
     public function testCases()
     {
         return [
             'Test that HTML entities used in text/html part get decoded' => [
                 'encoding.txt',
-                "\npöördumise töötaja.\n<b>Võtame</b> töösse võimalusel.\npöördumisele süsteemis\n\n",
+                "pöördumise töötaja.\n<b>Võtame</b> töösse võimalusel.\npöördumisele süsteemis",
             ],
             'testBug684922' => [
                 'bug684922.txt',
                 '',
             ],
             'test $structure->body getting textual mail body from multipart message' => [
                 'multipart-text-html.txt',
-                "Commit in MAIN\n",
+                'Commit in MAIN',
             ],
 
             'test with multipart/mixed mail with multipart/alternative attachment' => [
                 'multipart-mixed-alternative.eml',
-                "No one has ever seen God.\n",
+                'No one has ever seen God.',
+            ],
+            'process multipart/related, add it unless plain text content already present' => [
+                'multipart-related.eml',
+                "Labas,\n\nsu pšventėmis :)",
             ],
         ];
     }

tests/data/multipart-related.eml

@@ -0,0 +1,53 @@
+Subject: Fwd: [#97563] Updated
+To: "support@example.org" <support@example.org>
+From: Alina <alina@example.org>
+Message-ID: <22e40752-d4c6-8a00-29b9-ec16fab4e78a@example.org>
+Date: Wed, 27 Dec 2017 14:44:19 +0200
+MIME-Version: 1.0
+Content-Type: multipart/alternative;
+	boundary="------------7E9A30E282AA98E24563DF61"
+
+This is a multi-part message in MIME format.
+--------------7E9A30E282AA98E24563DF61
+Content-Type: text/plain; charset=utf-8; format=flowed
+Content-Transfer-Encoding: 8bit
+
+Labas,
+
+su pšventėmis :)
+
+
+--------------7E9A30E282AA98E24563DF61
+Content-Type: multipart/related;
+ boundary="------------AF85922D7263D4EB6025A866"
+
+
+--------------AF85922D7263D4EB6025A866
+Content-Type: text/html; charset=utf-8
+Content-Transfer-Encoding: 8bit
+
+<html>
+  <head>
+
+    <meta http-equiv="content-type" content="text/html; charset=utf-8">
+  </head>
+  <body text="#000000" bgcolor="#FFFFFF">
+    <p>Labas,</p>
+    <p>su pšventėmis :) <br>
+    </p>
+  </body>
+</html>
+
+--------------AF85922D7263D4EB6025A866
+Content-Type: image/png;
+ name="s.gif"
+Content-Transfer-Encoding: base64
+Content-ID: <part3.CEDAD0F3.C6CEC096@example.org>
+Content-Disposition: inline;
+ filename="s.gif"
+
+R0lGODlhAQABAID/AMDAwAAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==
+
+--------------AF85922D7263D4EB6025A866--
+
+--------------7E9A30E282AA98E24563DF61--

tests/src/TestCase.php

@@ -52,9 +52,12 @@ protected function getExtensionManager($config)
         return $stub;
     }
 
-    protected function getDataFile($filename)
+    protected function getDataFile($fileName)
     {
-        return __DIR__ . '/../data/' . $filename;
+        $dataFile = dirname(__DIR__) . '/data/' . $fileName;
+        $this->assertFileExists($dataFile);
+
+        return $dataFile;
     }
 
     /**