imap2_fetchbody ignores UID flag

[coxlr]

Currently the imap2_fetchbody ignores the given flags. Looking at the code the function Massage::fetchBody always sets isUid to true on line 134 when calling fetch:

$messages = $client->fetch($imap->getMailboxName(), $messageNum, true, ['BODY['.$section.']']);

Is there a specific reason for this?

I have tested updating this to be:

$isUid = boolval($flags & FT_UID);

$messages = $client->fetch($imap->getMailboxName(), $messageNum, $isUid, ['BODY['.$section.']']);

And then in the fetch function in the file ImapClient.php added the following to line 2629.

if($is_uid){
    $result[$result[$id]->uid] = $result[$id];
    unset($result[$id]);
}

And I now get the expected message body returned for the given message UID.

I am happy to create a pull request for this, but wanted to checked that wasn't a reason for not using the UID before I went ahead and created the pull request

[bago]

For reference, this is part of a larger issue, see #15

[IZSkiSurfer]

Also check #32
This is a design flaw by roundcubes library. Check my ticket for how to handle it the "roundcube way"

[haiderpro]

I have made a simple workaround for this without touching IMAP2 code for it:

 // HA convert UID to normal messagenumber because IMAP2 library can only fetch partnumbers / attachments with normal msgnumbers not with UIDs:
  $messageNumber = imap2_msgno($imap, $UID); // this will provide msgnum using the UID

  $attachment    = imap2_fetchbody($imap, $messageNumber, $partNumber);

[IZSkiSurfer]

I have made a simple workaround for this without touching IMAP2 code for it:

 // HA convert UID to normal messagenumber because IMAP2 library can only fetch partnumbers / attachments with normal msgnumbers not with UIDs:
  $messageNumber = imap2_msgno($imap, $UID); // this will provide msgnum using the UID

  $attachment    = imap2_fetchbody($imap, $messageNumber, $partNumber);

Have you checked the ticket I referred to?!
Your workaround won't work under some conditions.
You are using message numbers instead of UID this way. Message numbers might/will change whenever e.g. a mail is deleted, moved and sometimes even if a new mail arrives.
This way it can happen, that you won't either get attachments for a specific email or get the attachments of an other email if the numbering of messages changed on the serverside between subsequent function calls.

[haiderpro]

I have made a simple workaround for this without touching IMAP2 code for it:

 // HA convert UID to normal messagenumber because IMAP2 library can only fetch partnumbers / attachments with normal msgnumbers not with UIDs:
  $messageNumber = imap2_msgno($imap, $UID); // this will provide msgnum using the UID

  $attachment    = imap2_fetchbody($imap, $messageNumber, $partNumber);

Have you checked the ticket I referred to?! Your workaround won't work under some conditions. You are using message numbers instead of UID this way. Message numbers might/will change whenever e.g. a mail is deleted, moved and sometimes even if a new mail arrives. This way it can happen, that you won't either get attachments for a specific email or get the attachments of an other email if the numbering of messages changed on the serverside between subsequent function calls.

For your information, in Gmail and other modern email services UID also changes with some actions so it is also not fully reliable and as I wrote, it was a workaround not a fix for the code but you downvoted without confirming if it is working or not :-)

I am using this in an environment of around 20 email accounts and it is working fine. It is the easiest workaround in which the code converts UID to MSGNUM just before fetching the part so MSG NUM is fresh and it works. It fetches the MSG NUM of the UID in real-time when fetching the object. Here is the updated code and it works and we only need to use it where fetch command is given and it also detects if it is UID or MSG NUM and converts to MSG NUM only if it is UID:

function MyOwnFetchbody($msgnum, $partnum) { 
//  only fetch msg number if it is already NOT a msg number (which means it is a UID) 
 if ($msgnum > 3000) {$msgnum = imap2_msgno($imap, $msgnum) ?:  $msgnum;}

            return imap2_fetchbody($this->conn, $msgnum, $partnum);
}

Hope this easy code will help someone who is unable to implement the complicated fix others have suggested and if something is working correctly if implemented wisely should not be downvoted just because some people don't understand that how it can be safely applied :-)

I applied this workaround because if any library is updated or fixed, I will not be forced to change any code because this code will work in all conditions.

[IZSkiSurfer]

Sorry but comparing a $msgnum against 3000 makes no sense at all. there is no such general rule which says "a number > 3000 must be an UID".

For your information, in Gmail and other modern email services UID also changes with some actions so it is also not fully reliable

Source?!
RFC 3501
"The unique identifier of a message MUST NOT change during the session, and SHOULD NOT change between sessions."
The UID does not change during a session! (except for move/delete/..)

But your solution will not only fail in some cases it will/can even give you the body from another mail in certain circumstances without you noticing e.g. if the sequence numbers are changed between your two function calls what totally can happen in a busy mail environment.

It's fine if it works for you so far (even so you might not have noticed if a wrong body was fetched) but it's absolutely not a "good" advice for others - hence the downvote.

[haiderpro]

Like I mentioned in the post, I am using Gmail and the code is a sample code to give an idea to people how they can make it work so I did not remove the code I inserted for Gmail based email system. In Gmail, there is option in Gmail settings to select how many emails to fetch and mostly people select between 1000 to 3000 messages. That code is based on this setting so that we don't have to fetch for msg number on those messages for which we know that they are already msg number. This is called code optimization.
I also mentioned that UID also changes like you admitted so even if we use your code, it will still not work if UID changes.
In my code, the msg number is fetched just before performing a fetch request using the UID. So till that I am also using UID and server takes less than a second to fetch msg number using the UID so there is no chance of mistake. However, I also think that the code in the library should be fixed instead of these workarounds but the sample code is for those who need immediate remedy and can't wait for the library code to be fixed. Hope this makes sense :-)

[IZSkiSurfer]

Sorry but the possible settings in gmail are 1000, 2000, 5000 or 10000 and that's still a very bad idea to just say "msg num is > 3000 - oh that must be an UID"
I haven't admitted UID changes DURING A SESSION (except for move/delete/..) and that are tasks you must do on purpose from within that session. Even if the UID changes you won't accidently hit a wrong mail because the ID being unique in that context/in the session.
Where msgno CAN absolutely change IN YOUR SESSION when a second client (or the server) performs actions.
Even when you resolve the UID to msgno and yes even if it's just a couple of milliseconds there is absolutely no guarantee that between the resolve and the next IMAP command that message no. will represent the same message you queried for.

there is no chance of mistake
That's simply wrong! The chances might be very low but it depends on the account if it's used rarely and only from one process or e.g. a dozen of different processes performing different tasks in parallel multiple times per second.

That's the hole idea behind having an UID that it's quite fixed and even IF it changes you won't accidentially retrieve a wrong message.

2.3.1.2. Message Sequence Number Message Attribute

A relative position from 1 to the number of messages in the mailbox.
This position MUST be ordered by ascending unique identifier. As
each new message is added, it is assigned a message sequence number
that is 1 higher than the number of messages in the mailbox before
that new message was added.

Message sequence numbers can be reassigned during the session. For
example, when a message is permanently removed (expunged) from the
mailbox, the message sequence number for all subsequent messages is
decremented. The number of messages in the mailbox is also
decremented. Similarly, a new message can be assigned a message
sequence number that was once held by some other message prior to an
expunge.

An example where your code can/will badly fail:

Mailbox has let's say 10 mails.

Process A:
Iterate all 10 mails.
Within each iteration get the msg. no. for the UID.
Use that msg. no. to perform two tasks "tA" and "tB". E.g. "tA" retrieve header, "tB" based on header information get attachments and save in an folder for the recipient.

While in the 5th iteration between "tA" and "tB" e.g. your fictive UID "uid" will be resolved to 5 where process B kicks in:

Process B:
expunge mail with msg. no. 3

The server will/might rebase all subsequent mail's msg. no. so mails with msg. no 4 now will become 3, mail 5 will become 4, mail 6 will become 5, and so on.

Now back in process A what will happen?
You initially resolved UID "uid" to msg. no. 5.
In "tA" you retrieve header of mail no. 5. and the retriever in that mail is "foo".
In "tB" you retrieve attachment of mail no. 5 which is due to the expunge now what was initially mail no. 6. You save those attachments in the folder of "foo" even if the receiver of the former mail no. 6 would have been "bar".

Even if you resolve the UID "uid" before "tA" and "tB" each time again you might get before "tB" the changed msg. no. "4" but even then there is absolutely no guarantee that between the resolve and the next IMAP command your msg. no. "4" is still the mail with UID "uid" which you actuallly wanted to work with.

I'm glad if it works for your use case but all should be warned properly.