when forwarding mail, only first attachment is sent, all subsequent ones are 0 bytes

[makinghappen]

Steps to reproduce

1. send an email with several attachments(2 or more) from nextcloud mail app
2. go to send folder and forward it again
3. only first attachment is sent now, all subsequent ones are 0 bytes

Expected behavior

all attachments must be forwarded

Actual behavior

second and subsequent attachments are not sent, empty file is sent instead:
Content-Type: application/x-empty; name=photo.jpg

Content-Disposition: attachment; filename=photo.jpg

Content-Transfer-Encoding: base64

Mail app version

1.11.7

Mailserver or service

No response

Operating system

Debian

PHP engine version

PHP 8.1

Web server

Nginx

Database

MySQL

Additional info

when forwarding from mail client everything works.

[ChristophWurst]

@GretaD you +1'd. Are you able to reproduce?

[GretaD]

@GretaD you +1'd. Are you able to reproduce?

yes, i can. I dont remember seeing any error. But i can try to reproduce it again

[boreevyuri]

I think it's bug in buildAttachmentsPartsQuery in buildAttachmentsPartsQuery

When getRawAttachments tries to get second attachment from source letter, buildAttachmentsPartsQuery returns array with empty first element (or several preceding empty elements if letter has more than two attachments).

Temporary fixed this on own installation with if (!empty($decoded)) { $attachments[] = $decoded; } in getRawAttachments but it looks like something wrong.

[miaulalala]

Very interesting, thanks for taking a look!

[sazanof]

I faced this problem from the other side. I opened the email containing the attachment and tried to forward it.
After I found out that there were no attachments at all in the forwarded email.

I tried to fix the frontend, but it looks like this alone will not do. Through the "corrected frontend" I received a visualization of email attachments (forwarded email). Next - I forward them and lo and behold - the letter arrives and there are attachments in it! Only something is wrong: the icons are not the same. The pdf has an archive icon. Аnd the json containing the attachments->attachment object has an invalid mime type

I decided to dig deeper ... As a result, what I managed to dig up : =)

1. by clicking on send a message, an array with attachments is sent (correct)
   1.1) So far, the type is sewn tightly here Move attachment type declaration to backend #6227 , I have temporarily commented out these lines and made a dirty fix
   https://github.com/sazanof/nextcloud_mail/blob/bug/no-attachments-when-forwarding/lib/Controller/OutboxController.php#L122 (maybe it's creepy, but let me continue the story =))
2. the handleAttachments method is called on the server, which at one time pulls the handleForwardedAttachment method
3. in this method, in the line https://github.com/nextcloud/mail/blob/main/lib/Service/Attachment/AttachmentService.php#L314
   the $attachmentMessage message is incorrectly located, as it searches by uid (this is our (int)$attachment['messageId'])
   All due to the fact that the uid is not unique for the user. and it is unique only for mailboxId+uid
4. as a result, I realized that the attachment (attachments) are attached from another letter!

In general, as a temporary measure, I shoved mailboxId into each attachment in the getBody method and while my crutches are working ... =)

actually, it may be useful at least a little... - sazanof@e7eb3ce

Can I add the necessary fields (mailboxId at least) to the local message table?

I don't know what this monologue is about - I don't know if you already have ways to solve the problem, whether you are aware of the situation or not. And I would not really like to go separately with your approach.

I could organize a normal pullrequest, but maybe we need to discuss it first...

Thank you for finishing reading! =)

[miaulalala]

the $attachmentMessage message is incorrectly located, as it searches by uid (this is our (int)$attachment['messageId'])
All due to the fact that the uid is not unique for the user. and it is unique only for mailboxId+uid

I'm not sure that this is actually what's happening.
The DB query does indeed look for the primary ID in the MessageMapper:

mail/lib/Db/MessageMapper.php

Lines 124 to 136 in ef0c005

	public function findByUserId(string $userId, int $id): Message {
	$query = $this->db->getQueryBuilder();
	$query->select('m.*')
	->from($this->getTableName(), 'm')
	->join('m', 'mail_mailboxes', 'mb', $query->expr()->eq('m.mailbox_id', 'mb.id', IQueryBuilder::PARAM_INT))
	->join('m', 'mail_accounts', 'a', $query->expr()->eq('mb.account_id', 'a.id', IQueryBuilder::PARAM_INT))
	->where(
	$query->expr()->eq('a.user_id', $query->createNamedParameter($userId)),
	$query->expr()->eq('m.id', $query->createNamedParameter($id, IQueryBuilder::PARAM_INT), IQueryBuilder::PARAM_INT)
	);

The $uid is (confusingly named) the user id.

What could be happening is that the frontend is sending the wrong ID for the attachment. Can you take a look at the request to see if your 'messageId'' value is actually valid?

Thanks :)

[sazanof]

Thanks :)

Yes, everything is correct in the findByUserId function.

So, I tried to reproduce the problem again and that I understood (at least tried)
==FORWARDING A MESSAGE WITH AN ATTACHMENT==

0. Parsing the url /apps/mail/box/16/thread/9541
1. Open the message with the attachment in the branch and click the "forward" button. (the getBody(9541) method of the MessagesController is called)
2. The $id parameter of getBody(9541) is the id identifier in the mail_messages table (remember this)
   2.1) Received the correct message ($this->mailManager->GetMessage and so on). Now this method is looking for attachments...
   

   mail/lib/Controller/MessagesController.php

   Line 238 in ef0c005

   $message = $this->mailManager->getMessage($this->currentUserId, $id);

3. To get the attachments property, in the getBody method pulls $json = $this->mailManager->getImapMessage
   

   mail/lib/Controller/MessagesController.php

   Line 245 in ef0c005

   $json = $this->mailManager->getImapMessage(

   
   3.1) Oh yes, we received all the correct data and displayed them on the page. If you look at the attachments object, then each element of it will contain a messageId according to the uid from the mail_messages table, in a row with id 9541 (for example, messageId: 129).
   
   4)... here it is necessary to correct the backend so that these attachments are located, so for now you clearly have the 'local' type sewn up. let's say it's done.
4. OutboxController -> create is called(.,.,.,.,.,.,.,$ attachments)
   

   mail/lib/Controller/OutboxController.php

   Line 100 in ef0c005

   public function create(

   
   5.1) The attachments object is identical to the $attachments object that we received earlier in 3.1
   5.2) $this->service->saveMessage($account, $message, $to, $cc, $bcc, $attachments);pulls handleAttachments, which in our case pulls handleForwardedAttachment
   

   mail/lib/Controller/OutboxController.php

   Line 129 in ef0c005

   $this->service->saveMessage($account, $message, $to, $cc, $bcc, $attachments);

   
   5.2) Hooray, we've reached the most interesting part, Sherlock.
   5.3) $attachmentMessage = $this->mailManager->getMessage($account->getUserId(), (int)$attachment['messageId']);. The parameter $attachment['messageId'] in our example is 129
   

   mail/lib/Service/Attachment/AttachmentService.php

   Line 314 in ef0c005

   $attachmentMessage = $this->mailManager->getMessage($account->getUserId(), (int)$attachment['messageId']);

5. Here is the most interesting. Look at point 2). It turns out that we are looking for a message not with the identifier 9541, but with the identifier 129. Therefore, the wrong message is returned from the database (or nothing at all). Therefore, further ...

$attachments = $this->messageMapper->getRawAttachments(
			$client,
			$mailbox->getName(),
			$attachmentMessage->getUid(),
			[
				$attachment['id'] ?? []
			]
		);

the wrong data is being transmitted ($mailbox->getName() from mail_messages->id->129)
and accordingly, the attachments are returned the wrong ones, or they are not returned at all

messageId in attachment is an identifier on the imap server, which is unique only if the correct folder (mailbox id) is added to it. for an attachment, the messageId is not equal to the id of the message in the database

I hope I didn't bore or confuse you too much =). Have a nice day!

[kesselb]

Sounds similar to #5602.

[st3iny]

Anna and I did an investigation:

1.

We set $attachment['messageId'] to the uid of the message.

mail/lib/Model/IMAPMessage.php

Line 404 in a96e8d5

'messageId' => $this->messageId,

However, our code expects it to be the database id.

mail/lib/Service/Attachment/AttachmentService.php

Line 314 in a96e8d5

$attachmentMessage = $this->mailManager->getMessage($account->getUserId(), (int)$attachment['messageId']);

Solution: Overwrite messageId in \OCA\Mail\Controller\MessagesController::enrichDownloadUrl with the real database id. Even better: Don't set messageId in IMAPMessage and only set it in enrichDownloadUrl.

2.

Our frontend does not pass the attachments when preparing a reply/forward.

Add

attachments: original.attachments,

to data: {...} all blocks (reply, replyAll and forward) in

mail/src/store/actions.js

Line 288 in a96e8d5

data: {

[kesselb]

Thanks everyone for all the feedback 👍 I think we have more than one issue here 🙈

1. The original report and the comment by @boreevyuri for Mail 1.11 (and also 1.12). Patch over here: Skip not requested parts #6543.
2. As suggested by @sazanof , @miaulalala and @st3iny the issue for Mail 1.12 is that IMailManager.getMessage lookup a message by the database id but get's a message uid (which is called messageId to confuse everyone 🤣). Patch over here: Add attachments objects on message forward #6544 but it's still a draft as I'm unhappy with mixing our database ids with imap ids when forwarding a message.

[kesselb]

Fixed with v1.12.6 / v1.13.4