fmbiete / Z-Push-contrib

Z-Push fork with changes that I will try to contrib
GNU Affero General Public License v3.0
134 stars 61 forks source link

Change Delete behavior #280

Open tickerguy opened 7 years ago

tickerguy commented 7 years ago

Yes, I know, clients shouldn't do this. But most phone clients are evil. They physically delete when you say delete, they don't move to the Trash and they don't allow configuration of what to do either! This is not nice and can lead to big, unrecoverable mistakes (like if you accidentally mass-select and then delete!), since the default behavior is to set the delete flag and then immediately push an EXPUNGE command to the IMAP server (which of course does exactly what it's told to do.)

This not only immediately removes the message it can do some especially-evil things in the scenario where you have multiple clients in a given mailbox and only some of them are Exchange. For example, let's say I do the following from Thunderbird while I have an Exchange client connected, and Thunderbird is not configured to move to Trash (that is, it just marks deleted):

  1. Read messages in Thunderbird. Hit delete for each; they get the deleted flag.
  2. Now read a message on the phone. Hit delete. Not only is the one message I deleted removed so are all the other delete-marked ones, which is not what I intended, and that deletion is irreversible!

The following diff changes this behavior; now if you execute a delete it moves the email to the trash unless you're in the trash folder, then it does a physical delete. This can be parameterized in the config file (and maybe should be), but I didn't bother since I want it that way for all clients. This way the only non-recoverable "Delete" is one that is executed in the trash folder; if you delete something in there it's a fair bet you really mean it and it's ok to physically remove it.

That eliminates not only the accidental "mass delete" done by the phone but also removes the possibility in a multi-client environment of having marked things for deletion in some client that has the ability to do so and a separate "expunge" function (as is the case for true IMAP clients in the general case) and then when a message is deleted on the phone all the marked messages are likewise expunged although you almost-certainly didn't intend for that to happen. The remaining exception (if you're in the trash folder) leaves you able to physically remove a message, but now it requires two operations and one of them has to take place in a folder that is arguably a "wastebasket" that temporarily holds messages that are going to be "shredded" anyway.

root@NewFS:/usr/share/z-push/backend/imap # diff -u imap.php.noremapdelete imap.php
--- imap.php.noremapdelete      2016-08-04 17:03:19.069377000 -0500
+++ imap.php    2016-08-04 17:26:17.536042000 -0500
@@ -1553,6 +1553,12 @@
         ZLog::Write(LOGLEVEL_DEBUG, sprintf("BackendIMAP->DeleteMessage('%s','%s')", $folderid, $id));

         $folderImapid = $this->getImapIdFromFolderId($folderid);
+
+        if (strcasecmp($folderImapid, $this->create_name_folder(IMAP_FOLDER_TRASH)) != 0) {
+               $s1 = MoveMessage($folderid, $id, $this->create_name_folder(IMAP_FOLDER_TRASH), $contentparameters);
+               return($s1);
+       }
+
         $this->imap_reopen_folder($folderImapid);

         if ($this->imap_inside_cutoffdate(Utils::GetCutOffDate($contentparameters->GetFilterType()), $id)) {