Hi,
Pigeonhole shouldn't need to know anything about this being a virtual mailbox specifically. If I understand right, the problem is that imapsieve doesn't like it when the destination folder doesn't know what the IMAP UID number is for the saved/copied mail. Earlier versions assert-crash, while a newer code just ignores this situation and doesn't execute the script. I think your patch works by returning the real folder's UID, not the virtual folder's UID. But imapsieve is still working on the virtual folder, so the UID likely points to a wrong message.
Fixing this is rather difficult. It can't even be fixed in all situations. For example what happens if you have Virtual/Flagged folder that only has mails that have the \Flagged flag, and you copy a mail there without the flag? It gets saved to the real folder, but it won't show up in the Virtual/Flagged folder itself until later when it gets a \Flagged flag. What happens when you copy/save some mails that have the \Flagged flag and some that don't? Only some of the mails will end up in the virtual folder. Neither IMAP nor the current lib-storage API allows filling saved_uids only partially.
It could be a possibility to fix this for the specific situations when the saved mails are guaranteed to end up in the virtual folder. Detecting whether this is true could be a bit tricky in itself, although it could work for most cases by just supporting it when the virtual search query is "all". The next step would be to make the virtual transaction actually save the mails directly to the virtual indexes instead of going through the generic "virtual sync" code. Although I'm not sure how it would handle a situation when the real folder already had some new mails that weren't yet in the virtual index. The syncing might be smart enough to add those mails later, I'm not sure.
A completely different way of solving it might be to never actually save mails to a virtual folder directly. So e.g. when saving to Virtual/All the folder would be early on converted to INBOX, so imapsieve code would be thinking that the mail is being saved to INBOX. Maybe something like:
- virtual_mailbox_alloc() checks if MAILBOX_FLAG_SAVEONLY is used. If yes, return the real mailbox instead of the virtual mailbox.
- But this could cause much confusion for callers that don't expect it. So probably require also a new flag MAILBOX_FLAG_BACKEND_SAVEONLY.
- Change the important places to support MAILBOX_FLAG_BACKEND_SAVEONLY properly. At least with IMAP when this is used and the mailbox isn't the expected mailbox, it shouldn't return [COPYUID] or [APPENDUID] response codes to COPY/APPEND commands since the returned UIDs would be for the real mailbox rather than the virtual mailbox.
- It's also possible that this behavior would break some plugins..
On 20. Nov 2021, at 14.02, Claudemir Todo Bom <claudemir@todobom.com> wrote:
Hi,
after some time learning some parts of the code of dovecot/pigeohole, I made a Proof of Concept that it is possible to execute imapsieve scripts when moving messages to virtual mailboxes, made it a pull request on the pigeonhole github project:
https://github.com/dovecot/pigeonhole/pull/10 <https://github.com/dovecot/pigeonhole/pull/10>
I would like that somebody take a look at this changes and make them in a propper way into the main code of pigeonhole and dovecot.
Em seg., 15 de nov. de 2021 às 11:55, Claudemir Todo Bom <claudemir@todobom.com <mailto:claudemir@todobom.com>> escreveu: Hi folks, and @stephanbosch <>
trying to debug the code, even knowing nothing about it, I discovered that @stephanbosch (cc him on this message) fixed the panic error I described in my original message on commit #27ab897f in the pigeonhole project.
In this change he only avoided running into the bug if UID of the message could not be determined, which is the case when the target mailbox of a copy or move operation is a virtualbox with a fallback real mailbox. This way the imapsieve is unusable when moving to a virtual mailbox, since what I'm trying to do is a virtualbox for Junk and some others, all with real mailboxes fallbacks and trying to activate the spamassassin learning on this movement, I need to detect when a message is being moved from a junk folder to any other folder and vice-versa.
Can anybody help me on making imapsieve to work when dropping messages into virtual mailbox that have a real mailbox fallback?
Best regards, Claudemir