diff --git a/lib/Service/MailTransmission.php b/lib/Service/MailTransmission.php index b90ca05d38..be808fc6fb 100644 --- a/lib/Service/MailTransmission.php +++ b/lib/Service/MailTransmission.php @@ -14,6 +14,8 @@ use Horde_Imap_Client_Data_Fetch; use Horde_Imap_Client_Fetch_Query; use Horde_Imap_Client_Ids; +use Horde_Mail_Rfc822_Address; +use Horde_Mail_Rfc822_List; use Horde_Mail_Transport_Null; use Horde_Mail_Transport_Smtphorde; use Horde_Mime_Exception; @@ -42,7 +44,6 @@ use OCA\Mail\Exception\ServiceException; use OCA\Mail\IMAP\IMAPClientFactory; use OCA\Mail\IMAP\MessageMapper; -use OCA\Mail\Model\Message as ModelMessage; use OCA\Mail\Model\NewMessageData; use OCA\Mail\Service\DataUri\DataUriParser; use OCA\Mail\SMTP\SmtpClientFactory; @@ -107,13 +108,13 @@ public function sendMessage(Account $account, LocalMessage $localMessage): void $transport = $this->smtpClientFactory->create($account); // build mime body - $headers = [ - 'From' => $from->toHorde(), - 'To' => $to->toHorde(), - 'Cc' => $cc->toHorde(), - 'Bcc' => $bcc->toHorde(), - 'Subject' => $localMessage->getSubject(), - ]; + $headers = $this->buildHeaders( + $from, + $to, + $cc, + $bcc, + $localMessage->getSubject() + ); // The table (oc_local_messages) currently only allows for a single reply to message id // but we already set the 'references' header for an email so we could support multiple references @@ -197,41 +198,27 @@ public function saveLocalDraft(Account $account, LocalMessage $message): void { $perfLogger = $this->performanceLogger->start('save local draft'); - $imapMessage = new ModelMessage(); - $imapMessage->setTo($to); - $imapMessage->setSubject($message->getSubject()); - $from = new AddressList([ - Address::fromRaw($account->getName(), $account->getEMailAddress()), - ]); - $imapMessage->setFrom($from); - $imapMessage->setCC($cc); - $imapMessage->setBcc($bcc); - if ($message->isHtml() === true) { - $imapMessage->setContent($message->getBodyHtml()); - } else { - $imapMessage->setContent($message->getBodyPlain()); - } + $from = Address::fromRaw($account->getName(), $account->getEMailAddress()); foreach ($attachments as $attachment) { $this->transmissionService->handleAttachment($account, $attachment); } // build mime body - $headers = [ - 'From' => $imapMessage->getFrom()->first()->toHorde(), - 'To' => $imapMessage->getTo()->toHorde(), - 'Cc' => $imapMessage->getCC()->toHorde(), - 'Bcc' => $imapMessage->getBCC()->toHorde(), - 'Subject' => $imapMessage->getSubject(), - 'Date' => Horde_Mime_Headers_Date::create(), - ]; + $headers = $this->buildHeaders( + $from, + $to, + $cc, + $bcc, + $message->getSubject() + ); $mail = new Horde_Mime_Mail(); $mail->addHeaders($headers); if ($message->isHtml()) { - $mail->setHtmlBody($imapMessage->getContent()); + $mail->setHtmlBody($message->getBodyHtml()); } else { - $mail->setBody($imapMessage->getContent()); + $mail->setBody($message->getBodyPlain()); } $mail->addHeaderOb(Horde_Mime_Headers_MessageId::create()); $perfLogger->step('build local draft message'); @@ -297,33 +284,22 @@ public function saveDraft(NewMessageData $message, ?Message $previousDraft = nul $perfLogger->step('emit pre event'); $account = $message->getAccount(); - $imapMessage = new ModelMessage(); - $imapMessage->setTo($message->getTo()); - $imapMessage->setSubject($message->getSubject()); - $from = new AddressList([ - Address::fromRaw($account->getName(), $account->getEMailAddress()), - ]); - $imapMessage->setFrom($from); - $imapMessage->setCC($message->getCc()); - $imapMessage->setBcc($message->getBcc()); - $imapMessage->setContent($message->getBody()); - - // build mime body - $headers = [ - 'From' => $imapMessage->getFrom()->first()->toHorde(), - 'To' => $imapMessage->getTo()->toHorde(), - 'Cc' => $imapMessage->getCC()->toHorde(), - 'Bcc' => $imapMessage->getBCC()->toHorde(), - 'Subject' => $imapMessage->getSubject(), - 'Date' => Horde_Mime_Headers_Date::create(), - ]; + $from = Address::fromRaw($account->getName(), $account->getEMailAddress()); + + $headers = $this->buildHeaders( + $from, + $message->getTo(), + $message->getCc(), + $message->getBcc(), + $message->getSubject() + ); $mail = new Horde_Mime_Mail(); $mail->addHeaders($headers); if ($message->isHtml()) { - $mail->setHtmlBody($imapMessage->getContent()); + $mail->setHtmlBody($message->getBody()); } else { - $mail->setBody($imapMessage->getContent()); + $mail->setBody($message->getBody()); } $mail->addHeaderOb(Horde_Mime_Headers_MessageId::create()); $perfLogger->step('build draft message'); @@ -365,6 +341,32 @@ public function saveDraft(NewMessageData $message, ?Message $previousDraft = nul return [$account, $draftsMailbox, $newUid]; } + /** + * @return array{ + * From: Horde_Mail_Rfc822_Address, + * To: Horde_Mail_Rfc822_List, + * Subject: string|null, + * Cc?: Horde_Mail_Rfc822_List, + * Bcc?: Horde_Mail_Rfc822_List, + * } + */ + private function buildHeaders(Address $from, AddressList $to, AddressList $cc, AddressList $bcc, ?string $subject): array { + $headers = [ + 'From' => $from->toHorde(), + 'To' => $to->toHorde(), + 'Subject' => $subject, + ]; + + if (count($cc) > 0) { + $headers['Cc'] = $cc->toHorde(); + } + if (count($bcc) > 0) { + $headers['Bcc'] = $bcc->toHorde(); + } + + return $headers; + } + #[\Override] public function sendMdn(Account $account, Mailbox $mailbox, Message $message): void { $query = new Horde_Imap_Client_Fetch_Query(); diff --git a/tests/Unit/Service/MailTransmissionTest.php b/tests/Unit/Service/MailTransmissionTest.php index 6fbfe4d752..6a4cd20f30 100644 --- a/tests/Unit/Service/MailTransmissionTest.php +++ b/tests/Unit/Service/MailTransmissionTest.php @@ -13,6 +13,8 @@ use Horde_Imap_Client_Socket; use Horde_Mail_Transport; use OCA\Mail\Account; +use OCA\Mail\Address; +use OCA\Mail\AddressList; use OCA\Mail\Contracts\IMailManager; use OCA\Mail\Db\Alias; use OCA\Mail\Db\LocalAttachment; @@ -433,4 +435,89 @@ public function testCreateDraftsMailboxAndSave(): void { $this->transmission->saveLocalDraft(new Account($mailAccount), $localMessage); } + + public function testSendMessageCc() { + // Arrange + $mailAccount = new MailAccount(); + $mailAccount->setName('Bob'); + $mailAccount->setEmail('bob@mail.example'); + $mailAccount->setUserId('bob'); + $mailAccount->setSentMailboxId(123); + $account = new Account($mailAccount); + $localMessage = new LocalMessage(); + $localMessage->setSubject('Test'); + $localMessage->setBodyPlain('Test'); + $localMessage->setHtml(false); + $transport = $this->createMock(Horde_Mail_Transport::class); + $this->smtpClientFactory->expects($this->once()) + ->method('create') + ->willReturn($transport); + $this->transmissionService->expects(self::once()) + ->method('getSignMimePart') + ->willReturnCallback(static fn ($localMessage, $account, $mimePart) => $mimePart); + $this->transmissionService->expects(self::once()) + ->method('getEncryptMimePart') + ->willReturnCallback(static fn ($localMessage, $to, $cc, $bcc, $account, $mimePart) => $mimePart); + $this->transmissionService->expects(self::exactly(3)) + ->method('getAddressList') + ->willReturnCallback(function ($localMessage, $type) { + $addresses = []; + + if ($type === Recipient::TYPE_CC) { + $addresses[] = Address::fromRaw('Alice', 'alice@mail.example'); + } + + if ($type === Recipient::TYPE_BCC) { + $addresses[] = Address::fromRaw('Jane', 'jane@mail.example'); + } + + return new AddressList($addresses); + }); + + + // Act + $this->transmission->sendMessage($account, $localMessage); + + // Assert + $this->assertEquals(LocalMessage::STATUS_RAW, $localMessage->getStatus()); + $this->assertStringContainsString('From: Bob ', $localMessage->getRaw()); + $this->assertStringContainsString('Cc: Alice ', $localMessage->getRaw()); + $this->assertStringNotContainsString('Bcc:', $localMessage->getRaw()); + } + + public function testSendMessageOmitCc() { + // Arrange + $mailAccount = new MailAccount(); + $mailAccount->setName('Bob'); + $mailAccount->setEmail('bob@example.org'); + $mailAccount->setUserId('bob'); + $mailAccount->setSentMailboxId(123); + $account = new Account($mailAccount); + $localMessage = new LocalMessage(); + $localMessage->setSubject('Test'); + $localMessage->setBodyPlain('Test'); + $localMessage->setHtml(false); + $transport = $this->createMock(Horde_Mail_Transport::class); + $this->smtpClientFactory->expects($this->once()) + ->method('create') + ->willReturn($transport); + $this->transmissionService->expects(self::once()) + ->method('getSignMimePart') + ->willReturnCallback(static fn ($localMessage, $account, $mimePart) => $mimePart); + $this->transmissionService->expects(self::once()) + ->method('getEncryptMimePart') + ->willReturnCallback(static fn ($localMessage, $to, $cc, $bcc, $account, $mimePart) => $mimePart); + $this->transmissionService->expects(self::exactly(3)) + ->method('getAddressList') + ->willReturnCallback(static fn ($message, $type) => new AddressList([])); + + // Act + $this->transmission->sendMessage($account, $localMessage); + + // Assert + $this->assertEquals(LocalMessage::STATUS_RAW, $localMessage->getStatus()); + $this->assertStringContainsString('From: Bob ', $localMessage->getRaw()); + $this->assertStringNotContainsString('Cc:', $localMessage->getRaw()); + $this->assertStringNotContainsString('Bcc:', $localMessage->getRaw()); + } }