Fix non-mime email message content #40

zbateson · Apr 5, 2017 · c60636e · c60636e
1 parent b69ec30
commit c60636e
Show file tree

Hide file tree

Showing 7 changed files with 90 additions and 29 deletions.
diff --git a/src/Message.php b/src/Message.php
@@ -276,6 +276,7 @@ private function moveAllPartsAsAttachmentsExcept(MimePart $from, $exceptMimeType
             $this->setMessageAsMixed();
         }
         foreach ($parts as $part) {
+            $from->removePart($part);
             $this->addPart($part);
         }
     }
@@ -300,12 +301,21 @@ private function removeAllContentPartsFromAlternative($mimeType, $alternativePar
             return false;
         }
         if ($keepOtherContent) {
-            $this->moveAllPartsAsAttachmentsExcept($alternativePart, $mimeType);
+            $this->moveAllPartsAsAttachmentsExcept($rmPart, $mimeType);
+            $alternativePart = $this->getPart(0, PartFilter::fromInlineContentType('multipart/alternative'));
+        } else {
+            $rmPart->removeAllParts();
         }
-        $rmPart->removeAllParts();
         $this->removePart($rmPart);
-        if ($alternativePart->getChildCount() === 1) {
-            $this->replacePart($alternativePart, $alternativePart->getChild(0));
+        if ($alternativePart !== null) {
+            if ($alternativePart->getChildCount() === 1) {
+                $this->replacePart($alternativePart, $alternativePart->getChild(0));
+            } elseif ($alternativePart->getChildCount() === 0) {
+                $this->removePart($alternativePart);
+            }
+        }
+        while ($this->getChildCount() === 1) {
+            $this->replacePart($this, $this->getChild(0));
         }
         return true;
     }
@@ -347,6 +357,8 @@ protected function removePartByMimeType($mimeType, $index = 0)
         $alt = $this->getPart(0, PartFilter::fromInlineContentType('multipart/alternative'));
         if ($parts === null || !isset($parts[$index])) {
             return false;
+        } elseif (count($parts) === 1) {
+            return $this->removeAllContentPartsByMimeType($mimeType, true);
         }
         $part = $parts[$index];
         $this->removePart($part);
@@ -628,12 +640,12 @@ private function enforceMime()
     private function createMultipartRelatedPartForInlineChildrenOf(MimePart $parent)
     {
         $relatedPart = $this->mimePartFactory->newMimePart();
-        $relatedPart->setRawHeader('Content-Type', 'multipart/related');
-        $parent->addPart($relatedPart, 0);
-        foreach ($parent->getAllParts(PartFilter::fromDisposition('inline')) as $part) {
+        $this->setMimeHeaderBoundaryOnPart($relatedPart, 'multipart/related');
+        foreach ($parent->getChildParts(PartFilter::fromDisposition('inline', PartFilter::FILTER_EXCLUDE)) as $part) {
             $this->removePart($part);
             $relatedPart->addPart($part);
         }
+        $parent->addPart($relatedPart, 0);
         return $relatedPart;
     }
 
@@ -654,9 +666,9 @@ private function findOtherContentPartFor($mimeType)
             PartFilter::fromInlineContentType(($mimeType === 'text/plain') ? 'text/html' : 'text/plain')
         );
         if ($altPart !== null && $altPart->getParent() !== null && $altPart->getParent()->isMultiPart()) {
-            $altPart = $altPart->getParent();
-            if ($altPart->getPartCount(PartFilter::fromDisposition('inline', PartFilter::FILTER_EXCLUDE)) !== $altPart->getChildCount()) {
-                $altPart = $this->createMultipartRelatedPartForInlineChildrenOf($altPart);
+            $altPartParent = $altPart->getParent();
+            if ($altPartParent->getPartCount(PartFilter::fromDisposition('inline', PartFilter::FILTER_EXCLUDE)) !== 1) {
+                $altPart = $this->createMultipartRelatedPartForInlineChildrenOf($altPartParent);
             }
         }
         return $altPart;

diff --git a/src/Message/MessageParser.php b/src/Message/MessageParser.php
@@ -312,17 +312,15 @@ protected function readUUEncodedOrPlainTextPart($handle, Message $message)
         $line = trim(fgets($handle));
         $end = $this->findNextUUEncodedPartPosition($handle);
 
-        $part = null;
         if (preg_match('/^begin ([0-7]{3}) (.*)$/', $line, $matches)) {
             $mode = $matches[1];
             $filename = $matches[2];
             $part = $this->partFactory->newUUEncodedPart($mode, $filename);
             $this->partStreamRegistry->attachPartStreamHandle($part, $message, $start, $end);
+            $message->addPart($part);
         } else {
-            $part = $this->partFactory->newNonMimePart();
-            $this->partStreamRegistry->attachPartStreamHandle($part, $message, $start, $end);
+            $this->partStreamRegistry->attachPartStreamHandle($message, $message, $start, $end);
         }
-        $message->addPart($part);
     }
 
     /**

diff --git a/src/Message/PartFilter.php b/src/Message/PartFilter.php
@@ -86,6 +86,16 @@ class PartFilter
      * ```
      */
     private $headers = [];
+
+    /**
+     * @var string[] map of headers and default values if the header isn't set.
+     *      This allows text/plain to match a Content-Type header that hasn't
+     *      been set for instance.
+     */
+    private $defaultHeaderValues = [
+        'Content-Type' => 'text/plain',
+        'Content-Disposition' => 'inline',
+    ];
 
     /**
      * Convenience method to filter for a specific mime type.
@@ -319,7 +329,8 @@ public function failsHeaderPartFilter(MimePart $part)
     {
         foreach ($this->headers as $type => $values) {
             foreach ($values as $name => $header) {
-                $headerValue = $part->getHeaderValue($name);
+                $default = (isset($this->defaultHeaderValues[$name])) ? $this->defaultHeaderValues[$name] : null;
+                $headerValue = $part->getHeaderValue($name, $default);
                 if (($type === static::FILTER_EXCLUDE && strcasecmp($headerValue, $header) === 0)
                     || ($type === static::FILTER_INCLUDE && strcasecmp($headerValue, $header) !== 0)) {
                     return true;

diff --git a/src/Message/Writer/MessageWriter.php b/src/Message/Writer/MessageWriter.php
@@ -83,10 +83,9 @@ public function writeMessageTo(Message $message, $handle)
             $this->recursiveWriteParts($message, $handle);
         } else {
             $this->writePartHeadersTo($message, $handle);
+            $this->writePartContentTo($message, $handle);
             foreach ($message->getChildParts() as $i => $child) {
-                if ($i !== 0) {
-                    fwrite($handle, "\r\n\r\n");
-                }
+                fwrite($handle, "\r\n\r\n");
                 $this->writePartContentTo($child, $handle);
             }
         }

diff --git a/tests/MailMimeParser/IntegrationTests/EmailFunctionalTest.php b/tests/MailMimeParser/IntegrationTests/EmailFunctionalTest.php
@@ -1336,7 +1336,7 @@ public function testRemoveTextPartm0020()
         $this->runEmailTestForMessage($messageWritten, $test1, $failMessage);
     }
 
-    public function testRemoveHtmlPartsm0020()
+    public function testRemoveAllHtmlPartsm0020()
     {
         $handle = fopen($this->messageDir . '/m0020.txt', 'r');
         $message = $this->parser->parse($handle);
@@ -1359,7 +1359,6 @@ public function testRemoveHtmlPartsm0020()
 
         $test1 = $props;
         unset($test1['html']);
-        unset($test1['attachments']);
 
         $message->removeAllHtmlParts();
         $this->assertNull($message->getHtmlPart());
@@ -1393,7 +1392,7 @@ public function testRemoveHtmlPartm0020()
             'Subject' => 'Test message from Microsoft Outlook 00',
             'text' => 'hareandtortoise.txt',
             'html' => 'hareandtortoise.txt',
-            'inlineparts' => 4,
+            'attachments' => 2,
         ];
 
         $test1 = $props;
@@ -1402,6 +1401,9 @@ public function testRemoveHtmlPartm0020()
         $firstHtmlPart = $message->getHtmlPart();
         $secondHtmlPart = $message->getHtmlPart(1);
         $thirdHtmlPart = $message->getHtmlPart(2);
+
+        $secondContent = $secondHtmlPart->getContent();
+
         $message->removeHtmlPart();
         $this->assertNotNull($message->getHtmlPart());
         $this->assertNotEquals($firstHtmlPart, $message->getHtmlPart());
@@ -1420,6 +1422,25 @@ public function testRemoveHtmlPartm0020()
         fclose($tmpSaved);
         $failMessage = 'Failed while parsing saved message for rmho_m0020';
         $this->runEmailTestForMessage($messageWritten, $test1, $failMessage);
+
+        $this->assertNotNull($messageWritten->getHtmlPart());
+        $this->assertEquals($secondContent, $messageWritten->getHtmlContent());
+
+        $this->assertNotNull($message->getPartByMimeType('multipart/alternative'));
+        $message->removeHtmlPart();
+        $this->assertNull($message->getHtmlPart());
+        $this->assertNull($message->getPartByMimeType('multipart/alternative'));
+        $tmpSaved2 = fopen(dirname(dirname(__DIR__)) . '/' . TEST_OUTPUT_DIR . "/rmha_m0020", 'w+');
+        $message->save($tmpSaved2);
+        rewind($tmpSaved2);
+
+        $messageWritten2 = $this->parser->parse($tmpSaved2);
+        fclose($tmpSaved2);
+        $failMessage = 'Failed while parsing saved message for rmha_m0020';
+        $this->runEmailTestForMessage($messageWritten2, $test1, $failMessage);
+
+        $this->assertNotNull($messageWritten->getHtmlPart());
+        $this->assertEquals($secondContent, $messageWritten->getHtmlContent());
     }
 
     public function testAddHtmlPartRemoveTextPartm0001()

diff --git a/tests/MailMimeParser/MessageTest.php b/tests/MailMimeParser/MessageTest.php
@@ -46,12 +46,22 @@ protected function getMockedPartFactory()
         return $partFactory;
     }
 
-    protected function createNewMessage()
+    protected function createNewMessage($contentType = null)
     {
         $hf = $this->getMockedHeaderFactory();
         $mw = $this->getMockedMessageWriter();
         $pf = $this->getMockedPartFactory();
-        return new Message($hf, $mw, $pf);
+        $message = $this->getMockBuilder('ZBateson\MailMimeParser\Message')
+            ->setConstructorArgs([$hf, $mw, $pf])
+            ->setMethods(['getHeaderValue'])
+            ->getMock();
+        $message->method('getHeaderValue')->will($this->returnCallback(function($param, $defaultValue = null) use ($contentType) {
+            if (strcasecmp($param, 'Content-Type') === 0 && $contentType !== null) {
+                return $contentType;
+            }
+            return $defaultValue;
+        }));
+        return $message;
     }
 
     public function testObjectId()
@@ -68,14 +78,14 @@ public function testAddHtmlPart()
     {
         $part = $this->getMockedPart();
         $part->method('getHeaderValue')->will($this->returnCallback(function($param, $defaultValue = null) {
-            if ($param === 'Content-Type') {
+            if (strcasecmp($param, 'Content-Type') === 0) {
                 return 'text/html';
             }
             return $defaultValue;
         }));
         $part->method('getContentResourceHandle')->willReturn('handle');
 
-        $message = $this->createNewMessage();
+        $message = $this->createNewMessage('multipart/alternative');
         $message->addPart($part);
 
         $this->assertNull($message->getTextPart());
@@ -97,7 +107,7 @@ public function testAddTextPart()
         }));
         $part->method('getContentResourceHandle')->willReturn('handle');
 
-        $message = $this->createNewMessage();
+        $message = $this->createNewMessage('multipart/alternative');
         $message->addPart($part);
         $this->assertNull($message->getHtmlPart());
         $this->assertNull($message->getAttachmentPart(0));
@@ -119,7 +129,7 @@ public function testAddAttachmentPart()
             return $defaultValue;
         }));
 
-        $message = $this->createNewMessage();
+        $message = $this->createNewMessage('multipart/mixed');
         $message->addPart($part);
         $this->assertNull($message->getHtmlPart());
         $this->assertNull($message->getTextPart());
@@ -148,7 +158,7 @@ public function testGetParts()
             return $defaultValue;
         }));
 
-        $message = $this->createNewMessage();
+        $message = $this->createNewMessage('multipart/mixed');
         $message->addPart($part);
         $message->addPart($part2);
         $this->assertNull($message->getTextPart());
@@ -167,4 +177,14 @@ public function testMessageIsMime()
         $message = $this->createNewMessage();
         $this->assertFalse($message->isMime());
     }
+
+    public function testGetTextPartFromMessageWithoutContentType()
+    {
+        $message = $this->createNewMessage();
+        $message->setContent('Test');
+
+        $textPart = $message->getTextPart();
+        $this->assertNotNull($textPart);
+        $this->assertSame($message, $textPart);
+    }
 }
diff --git a/version.txt b/version.txt
@@ -1 +1 @@
-0.4.2
+0.4.3