Em seg., 22 de nov. de 2021 às 20:29, Timo Sirainen timo@sirainen.com escreveu:
Hi,
Hi,
I've answered the pull request on github also.
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.
The changes I made allows imapsieve to detect it is working with a virtual folder and then gets the real UID of the message on the real folder it is stored allowing to run the script. The imapsieve script will see the real folder name and the contents of the right message. This obviously only works if the virtual folder have a real folder fallback for saving.
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.
Note that APPEND and COPY/MOVE into a virtual mailbox that doesn't have a real mailbox fallback will never work, so, a virtual mailbox that only shows flagged messages cannot have a fallback folder, so, the answer for your question is: nothing... message will not be saved, and the imapsieve script will not run.
My patch fixed the behavior when APPEND and COPY reasons are triggered on a virtual mailbox that has a real mailbox fallback for saving (configured through the ! operator on the virtual mailbox definition)
When FLAGGING a message stored on a virtual mailbox, the imapsieve script is not running, this is because my patch isn't triggered in this situation. Anyway I think it can be fixed the same way: execute the imapsieve passing the real message on the real mailbox... I may try to look at this, but I wasn't interested in FLAG action.
(I've made this tests with my patch applied on dovecot 2.3.13 on Debian Bullseye)
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: properly. At least with IMAP when this is used and the mailbox isn't the
- 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
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..
I really don't understand dovecot code very well, I only studied the code in order to implement this feature. Hope you guys will put it in a good way on the code and I will be able to stop using this quick and dirty solution I made!
Best regards, Claudemir
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
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> 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