Exclude pgp signtures as attachments

[kesselb]

Story: I tried to forward an email with an OpenPGP signature.

This led to an internal server error: Argument 2 passed to OCA\\Mail\\Model\\Message::addRawAttachment() must be of the type string, null given, called in mail/lib/Service/MailTransmission.php on line 412

1: Show the email body and attachments

Part.php:1457, Horde_Mime_Part->isAttachment()
IMAPMessage.php:396, OCA\Mail\Model\IMAPMessage->getPart()
IMAPMessage.php:375, OCA\Mail\Model\IMAPMessage->loadMessageBodies()
IMAPMessage.php:100, OCA\Mail\Model\IMAPMessage->__construct()
MessageMapper.php:219, OCA\Mail\IMAP\MessageMapper->OCA\Mail\IMAP\{closure:/var/www/html/apps-extra/mail/lib/IMAP/MessageMapper.php:216-233}()
MessageMapper.php:233, array_map()
MessageMapper.php:233, OCA\Mail\IMAP\MessageMapper->findByIds()
MessageMapper.php:65, OCA\Mail\IMAP\MessageMapper->find()
MailManager.php:180, OCA\Mail\Service\MailManager->getImapMessage()
MessagesController.php:249, OCA\Mail\Controller\MessagesController->getBody()
Dispatcher.php:217, OC\AppFramework\Http\Dispatcher->executeController()
Dispatcher.php:126, OC\AppFramework\Http\Dispatcher->dispatch()
App.php:157, OC\AppFramework\App::main()
Router.php:302, OC\Route\Router->match()
base.php:1000, OC::handleRequest()
index.php:36, {main}()

2: Forward an email with openpgp attachment

Part.php:1457, Horde_Mime_Part->isAttachment()
MessageMapper.php:501, OCA\Mail\IMAP\MessageMapper->getRawAttachments()
MailTransmission.php:457, OCA\Mail\Service\MailTransmission->handleForwardedAttachment()
MailTransmission.php:356, OCA\Mail\Service\MailTransmission->handleAttachments()
MailTransmission.php:158, OCA\Mail\Service\MailTransmission->sendMessage()
AccountsController.php:433, OCA\Mail\Controller\AccountsController->send()
Dispatcher.php:217, OC\AppFramework\Http\Dispatcher->executeController()
Dispatcher.php:126, OC\AppFramework\Http\Dispatcher->dispatch()
App.php:157, OC\AppFramework\App::main()
Router.php:302, OC\Route\Router->match()
base.php:1000, OC::handleRequest()
index.php:36, {main}()

loadMessageBodies is using $structure->getParts() to fetch the parts of a message.

getRawAttachments is using $structure->partIterator() to fetch the parts of a message.

The object can now be iterated through to access the subparts. partIterator() can be used to obtain a RecursiveIteratorIterator that will iterator through all descendants. The $parent property has been added; it will be set, and is only guaranteed to be accurate, during iteration.

When using partIterator the $parent property is set. getParts does not set the $parent property.

Horde_Mime_Part.isAttachment is using the $parent property to detect if an pgp-signature is an attachment or not: https://github.com/horde/Mime/blob/7f6d0aaea1a1f194f46ac4a0428b539df11b46e1/lib/Horde/Mime/Part.php#L1464-L1482

tl;dr When viewing the message we see an attachment but when forwarding the same message we don't find the attachment anymore because isAttachment return different result.

I suspect that changing $structure->getParts() to $structure->partIterator() in loadMessageBodies is the proper fix. Yet that seems like a bigger change.

[ChristophWurst]

rebase, resolve and merge

[kesselb]

Code moved from IMAPMessage to ImapMessageFetcher.

[ChristophWurst]

Code looks scary but good

[kesselb]

Thanks for the reminder 👍

Rebased the branch.

It should be much easier to review now because #8597 was merged a while ago.

I will take a look next week and mark it as ready if we still need it.

[st3iny]

Tested and works.

Christoph's advent of code is going strong 💪 🎄 🎁

[st3iny]

/backport to stable3.5

[ChristophWurst]

Christoph's advent of code is going strong 💪 🎄 🎁

Sadly this is the first PR I could really get merge 🫠