javanile / php-imap2

PHP IMAP with OAUTH2
https://php-imap2.javanile.org/
GNU General Public License v3.0
48 stars 28 forks source link

bug in Roundcube's ImapClient's fetch method #32

Closed IZSkiSurfer closed 1 year ago

IZSkiSurfer commented 1 year ago

I came across a bug in the fetch method of Roundcube\ImapClient class or it's usage in the Message class. In Message class' fetchBody method e.g. you do the following: ` $messages = $client->fetch($imap->getMailboxName(), $messageNum, $isUid, ['BODY[TEXT]']);

    return $messages[$messageNum]->bodypart['TEXT'];`

Let's say $messageNum is 12345 and $isUid is true. You await an element in the $messages array with the key 12345.

In ImapClient this happens: A???? UID FETCH 12345 ... is sent and you'll (at least I do) get back * ???? FETCH (UID 12345 ... Afaik ???? is the same so you'll know to what request that fetch response belongs. You then grab that ???? and use it as the key in your return array. But outside of the function you await that the key is equal to the $messageNum you passed to the method.

So I would suggest adding near the end of Roundcube\ImapClient's fetch method something like: if ($is_uid && !empty($result[$id]->uid)) { $result[$result[$id]->uid] = $result[$id]; } or better collect all data in a temporary variable first and do the $result assignment with the correct key at the end.

IZSkiSurfer commented 1 year ago

Okay I looked into Roundcube's codebase and they "fix" it outside of the fetch method kinda like this: $results = $client->fetch($imap->getMailboxName(), $messageNum, $isUid, ['BODY[TEXT]']); $messages = []; if ($isUid && !empty($results)) { foreach ($results as $message) { $messages[$message->uid] = $message; } } Whenever and wherever you call the fetch* function(s) of Roundcube

nettunodev commented 1 year ago

Hi @IZSkiSurfer, are you suggesting that the javanile library should be changed using the instructions that you posted every time the fetch() function is called?

IZSkiSurfer commented 1 year ago

Hi @IZSkiSurfer, are you suggesting that the javanile library should be changed using the instructions that you posted every time the fetch() function is called?

Yes and that's exactly what @pjio has done in 25adff6 after my recommendation. I have it currently stable in a huge environment with about 20 shared O365 mailboxes - not a single problem anymore.