diff --git a/src/Email.php b/src/Email.php index 51f1c1f..2712fd6 100644 --- a/src/Email.php +++ b/src/Email.php @@ -297,7 +297,7 @@ private function generateBody(): AbstractPart } } - return $part ?? new TextPart($this->text, $this->textCharset); + return $part ?? new TextPart((string) $this->text, $this->textCharset); } private function prepareParts(): array diff --git a/src/Service/MailService.php b/src/Service/MailService.php index e7d8bd9..da943a0 100644 --- a/src/Service/MailService.php +++ b/src/Service/MailService.php @@ -70,6 +70,12 @@ public function send(): ResultInterface //trigger error event $this->getEventManager()->triggerEvent($this->createMailEvent(MailEvent::EVENT_MAIL_SEND_ERROR, $result)); throw new MailException($result->getMessage()); + } finally { + // Reset the Symfony Message body after every send attempt (success or failure). + // attachFiles() embeds DataParts into Message::$body; without this reset those + // DataParts survive a failed send and get wrapped into the next email's body, + // causing attachments from a failed send to leak into subsequent emails. + $this->getMessage()->setBody(null); } if ($result->isValid()) { diff --git a/test/Service/MailServiceTest.php b/test/Service/MailServiceTest.php index f69d4bc..685711f 100644 --- a/test/Service/MailServiceTest.php +++ b/test/Service/MailServiceTest.php @@ -17,8 +17,11 @@ use PHPUnit\Framework\MockObject\Exception; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use ReflectionClass; use Symfony\Component\Mailer\Transport\Smtp\EsmtpTransport; use Symfony\Component\Mailer\Transport\TransportInterface; +use Symfony\Component\Mime\Message; +use Symfony\Component\Mime\Part\AbstractPart; use Symfony\Component\Mime\Part\DataPart; use Symfony\Component\Mime\Part\Multipart\MixedPart; use Symfony\Component\Mime\Part\TextPart; @@ -292,4 +295,136 @@ public function testAttachFilesNoNestedMixedParts(): void $this->assertNotInstanceOf(MixedPart::class, $part); } } + + /** + * Found an attachment leak where DataParts + * embedded into Message::$body by attachFiles() survived a failed send + * and were wrapped into the next email. The fix must live in dot-mail: + * after send() (success OR failure) the underlying Message::$body must + * be reset so the next send rebuilds the body from scratch. + */ + public function testMessageBodyIsResetAfterFailedSendToPreventAttachmentLeak(): void + { + $this->mailService->setSubject('First'); + $this->message->html('First body'); + $this->mailService->addAttachment( + $this->fileSystem->url() . '/data/mail/attachments/testPdfAttachment.pdf' + ); + + $this->transportInterface + ->method('send') + ->willThrowException(new RuntimeException('SMTP failure')); + + try { + $this->mailService->send(); + $this->fail('Expected MailException was not thrown'); + } catch (MailException) { + // expected + } + + $this->assertNull( + $this->readPrivateMessageBody($this->message), + 'After a failed send the Symfony Message::$body must be reset; ' + . 'otherwise attachFiles() will wrap the leaked MixedPart into the next email.' + ); + } + + /** + * End-to-end regression for the same leak: after a failed send with + * attachments, configure a fresh email with no attachments, and verify + * the message handed to the transport contains no DataPart from the + * previous send. + */ + public function testAttachmentsDoNotLeakIntoSubsequentSendAfterFailure(): void + { + $this->mailService->setSubject('First'); + $this->message->html('First email'); + $this->mailService->addAttachment( + $this->fileSystem->url() . '/data/mail/attachments/testPdfAttachment.pdf' + ); + + $capturedSecondBody = null; + $callCount = 0; + $this->transportInterface + ->method('send') + ->willReturnCallback(function ($email) use (&$capturedSecondBody, &$callCount) { + $callCount++; + if ($callCount === 1) { + throw new RuntimeException('first send fails'); + } + $capturedSecondBody = $email->getBody(); + return null; + }); + + try { + $this->mailService->send(); + } catch (MailException) { + // expected + } + + // Reconfigure: brand new email with NO attachments. + $this->mailService->setAttachments([]); + $this->mailService->setSubject('Second'); + $this->message->html('Second email - must not carry first email\'s attachment'); + + $this->mailService->send(); + + $this->assertInstanceOf( + AbstractPart::class, + $capturedSecondBody, + 'Transport did not receive a second message body' + ); + $this->assertFalse( + $this->bodyContainsDataPart($capturedSecondBody), + 'Second email leaked a DataPart from the failed first send' + ); + } + + /** + * Confirms the success path also resets the body, so a future refactor + * does not silently regress the success case. + */ + public function testMessageBodyIsResetAfterSuccessfulSend(): void + { + $this->mailService->setSubject('Hello'); + $this->message->html('Body'); + $this->mailService->addAttachment( + $this->fileSystem->url() . '/data/mail/attachments/testPdfAttachment.pdf' + ); + + $this->transportInterface->expects($this->once())->method('send'); + + $this->mailService->send(); + + $this->assertNull( + $this->readPrivateMessageBody($this->message), + 'Message body should be null after a successful send so the next email starts clean.' + ); + } + + private function readPrivateMessageBody(Message $message): ?AbstractPart + { + $reflection = new ReflectionClass(Message::class); + $property = $reflection->getProperty('body'); + $property->setAccessible(true); + + return $property->getValue($message); + } + + private function bodyContainsDataPart(AbstractPart $part): bool + { + if ($part instanceof DataPart) { + return true; + } + + if ($part instanceof MixedPart) { + foreach ($part->getParts() as $child) { + if ($this->bodyContainsDataPart($child)) { + return true; + } + } + } + + return false; + } }