ProtonMail / proton-bridge

Proton Mail Bridge application
GNU General Public License v3.0
1.16k stars 157 forks source link

Bridge corrupts inbox when spam is moved by spam filter or mail is deleted from "All Mail" #62

Closed kortschak closed 3 years ago

kortschak commented 4 years ago

There are three behaviours here that are likely due to the same underlying issue, so this is reported as a single bug.

Expected Behavior

  1. Deleting mail from "Spam" or "All Mail" using the bridge should leave the inbox in a usable state,
  2. mail deleted from the inbox should not remain in "All Mail", and
  3. mail in "All Mail" should be findable in other folders.

Current Behavior

  1. If mail is filtered as spam it is moved to "Spam". The way that this is done leaves an inconsistency in the state of the inbox according to the bridge that causes an error when trying to delete any item from another folder in the inbox.

    On attempting to delete an item when an item has been filtered to spam an error, "Error syncing changes: mailbox 4 does not exist" is reported. This can only be cleared by using the web client to purge the "Spam" folder.

    In the broken state I get the following error:

    Failed to refresh folder "<user>@protonmail.com : Trash”.
    
    The reported error was “Error syncing changes: mailbox 4 does not exist”.

    then when navigating away from the trash folder,

    Error while Storing folder “Trash”.
    
    Error syncing changes: mailbox 4 does not exist

    From this point on, evolution will not sync to the server until the web client clears the "Spam" folder.

  2. Occasionally, mail remains in the "All Mail" folder. This is visible both in the local client and with the web client (result shown in 3. in image below). Attempting to delete emails that are in "All Mail" in this state results in errors being reported by the local client when navigating to the "Trash" folder.

    Error while Storing folder “All Mail”.
    
    Unable to move deleted messages: Error moving messages: Label does not exist
  3. In the case where emails have been deleted but remail in "All Mail" they are clearly not present in any other location.

    borked

Steps to Reproduce

  1. Install ProtonMail Bridge and set up for use with evolution mail client.

  2. Spam issue

    1. Receive spam.
    2. Attempt to delete email from another location (say "Inbox").
    3. See error.
  3. All Mail issue

    1. Delete mail from "All Mail".
    2. Navigate to to "Trash".
    3. See error.

Context (Environment)

This fundamentally breaks the utility of having a local desktop client since a web client must be available to clean up after the bridge's corruption.

~ $ lsb_release -d
Description:    Ubuntu 18.04.4 LTS
~ $ dpkg -l evolution protonmail-bridge 
Desired=Unknown/Install/Remove/Purge/Hold
| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name                 Version         Architecture    Description
+++-====================-===============-===============-=============================================
ii  evolution            3.28.5-0ubuntu0 amd64           groupware suite with mail client and organize
ii  protonmail-bridge    1.2.7-1         amd64           ProtonMail Bridge

Additional information

I have an analysis from one of the devs of evolutions quoted here:

[imapx:A] I/O: 'A00226 SELECT Spam' [imapx:A] I/O: ' OK [UIDNEXT 14] Predicted next UID' [imapx:A] I/O: ' OK [UIDVALIDITY 4] UIDs valid' [imapx:A] I/O: ' FLAGS ()' [imapx:A] I/O: ' OK [PERMANENTFLAGS (\Seen \SEEN \Flagged \FLAGGED \Deleted \DELETED \Draft \DRAFT $junk junk nonjunk)] Flags permitted.' [imapx:A] I/O: ' 3 EXISTS' [imapx:A] I/O: ' 0 RECENT' [imapx:A] I/O: ' OK [UNSEEN 1] ' [imapx:A] I/O: 'A00226 OK [READ-WRITE] SELECT completed' ... [imapx:A] I/O: 'A00237 UID MOVE 3647 Spam' [imapx:A] I/O: ' 3182 FETCH (FLAGS ($junk junk) UID 3647)' [imapx:A] I/O: ' 3183 EXISTS' [imapx:A] I/O: ' OK [UNSEEN 4] ' [imapx:A] I/O: 'A00237 NO Label does not exist'

the above shows that the 'Spam' folder can be selected, but one cannot move messages into it. Is it auto-populated by the server with the Junk flag? Maybe it is a server-side virtual folder.

[imapx:A] I/O: 'A00270 UID MOVE 13 Trash' [imapx:A] I/O: '* 3 EXPUNGE' [imapx:A] I/O: 'A00270 OK [COPYUID 4 13 453] COPY completed'

This successfully moves a message to Trash folder on connect 'A'. Meanwhile had been made connection 'B' (and 'C').

[imapx:B] I/O: 'B00271 SELECT Trash' [imapx:B] I/O: ' 3182 FETCH (FLAGS (\Seen nonjunk) UID 3647)' [imapx:B] I/O: ' 3183 EXISTS' [imapx:B] I/O: ' OK [UNSEEN 3] ' [imapx:B] I/O: ' OK [UNSEEN 0] ' [imapx:B] I/O: ' OK [UIDNEXT 454] Predicted next UID' [imapx:B] I/O: ' OK [UIDVALIDITY 4] UIDs valid' [imapx:B] I/O: ' FLAGS ()' [imapx:B] I/O: ' OK [PERMANENTFLAGS (\Seen \SEEN \Flagged \FLAGGED \Deleted \DELETED \Draft \DRAFT $junk junk nonjunk)] Flags permitted.' [imapx:B] I/O: ' 1 EXISTS' [imapx:B] I/O: ' 0 RECENT' [imapx:B] I/O: 'B00271 OK [READ-WRITE] SELECT completed'

The connection 'B' has selected the Trash folder now.

[imapx:B] I/O: 'B00278 UID STORE 453 -FLAGS.SILENT (\DELETED)' [imapx:B] I/O: 'B00278 OK UID STORE completed' [imapx:B] I/O: 'B00280 UID STORE 453 -FLAGS.SILENT (JUNK)' [imapx:B] I/O: 'B00280 NO mailbox 4 does not exist'

IMAPx wants to unset \Deleted and Junk flags on the just moved message, because it's in the Trash folder, where the \Deleted flag doesn't make much sense. It succeeded with the \Deleted flag, but failed with the Junk flag.

It seems to me that the server treats the Spam and Trash folders very specifically, the same as the Junk flag. Why it errors out this way might be a question for them.

With respect of the real versus virtual Trash/Junk folders, unless the server forces subscription of these folders you can check "Show only subscribed folders" option in the account Properties->Receiving Options tab (which is unchecked by default), and it'll show you only subscribed folders. Which they are can be seen in Folder->Subscriptions for that account.

I didn't get whether using virtual trash/junk behaves properly for the Trash/Spam sever-side folders or not. If it does weird things, then rather stay with the real folders. I'd definitely try to contact them and ask them about these errors. Feel free to use my text from above, which may or may not help them from the technical point of view.

Maybe there are IMAP clients not showing these errors, but I'd be afraid that such clients just hide these errors from the user, pretending everything worked fine. Of course, I cannot tell for sure unless having similar communication log. There's always a chance that the IMAPx provide does something wrong.

exander77 commented 4 years ago

@kortschak You cannot add or remove message to or from All Mail already now. All Mail cannot be changed, it's view over all messages of every mailbox.

@kortschak You cannot add or remove message (which is already in PM; migration from other provider is different situation) to or from All Mail already now. All Mail cannot be changed, it's view over all messages of every mailbox.

Oh, you tuned down your statement and made it completely nonsensical? Saying that you cannot add a mail into All Mail, because every mail is already there? Tanks for the info, Captain Obvious.

I am not sure how you define add an email. But it is APPEND for me and you can sure APPEND into the All Mail. (And delete as well.) For example, sending and receiving email adds a message to All Mail as well. I can be Captain Obvious as well.

Btw, if you want to go into semantics, the All Mail is not a view (in sense of a user, but may be in sense of all emails of all users), but it is the other way around. Inbox, Sent, Drafts, etc. are views over All Mail. Or where do you imagine emails which are just in the "All Mail view" reside?

bartbutler commented 4 years ago

Yes, fundamentally all labels and folders are views over All Mail. Or, as @horejsek described it, All Mail is a permanent label that cannot be removed from messages short of deletion of the messages. How to manage those restrictions with all clients is a real problem, and one that IMAP as a protocol is somewhat unsuited to, as returning errors has a tendency (historically) to screw up client state. That said, I agree that if we do return success, we also have to generate the appropriate background operations to assure consistency.

@horejsek what about translating a message that is in All Mail and Trash to having a \Deleted flag in All Mail? Then we can return success, generate the appropriate background events to restore the message to All Mail with the \Deleted flag, but still have an appropriate visual change for the client (depending on client support). We can ignore/defer indefinitely EXPUNGEs to non-Trash.

@exander77 I need to ask you to try to be constructive here. I understand your frustration, and we are trying to be accommodating to your behavior in light of that, but being verbally abusive to others in this thread (and that is exactly what you are doing) is not acceptable and will not be tolerated if it continues.

horejsek commented 4 years ago

@bartbutler Yes, I can confirm we are looking into how \Deleted flag can improve communication with IMAP clients. Unfortunately, it has its other issues, not much technical one like more how to behave in all possible situations in a way how most users would expect.

exander77 commented 4 years ago

@bartbutler In the context of my situation, I think I am being extremely calm.

I am a paying customer whose data were destroyed, corrupted, and mangled. I really tried to report issues with very detailed analysis, for most of the issues presented I did it more than 3 months ago, I have yet to be able to migrate all my emails, paid for Gmail additional 3 months, but lost hope, @kortschak seems to have filed his issues even earlier in March.

Mr. @horejsek presented a complete lack of knowledge of how the Bridge currently works, repeating untrue information to us. And instead of saying, "guys sorry, you are correct, we have to think of something better", he edited his post, making it nonsensical. The issue discussed here was, that you are proposing to disallow any operations on All Mail and the supportive argument was that it does not already allow adding or deleting messages there. Which is clearly not the case. I expected some kind of honesty from my business partners.

I don't think, that you are trying to be constructive. All I see is trying to do easy workarounds instead of real fixes.

I proposed solutions and presented issues with the solutions you have presented. I would like to know how is it going to be solved? And what is the problem with the following: **1. Deleting messages from All Mail should move them to Trash.

  1. Deleting messages from Trash should delete them permanently.
  2. Deleting messages from Spam should delete them permanently.
  3. All Mail should not contain Trash or Spam.
  4. The only time a message is deleted is when it is deleted from Trash or Spam. Moving a message should never delete it.**

As a software engineer myself, I have a very strong feeling that you know this is how it should be done (it is done that way on Gmail and Outlook), but you are unwilling to do it because there are some problems with the current architecture of the backend that would need some serious modification. Feel free to prove me wrong. And feel free to point why is it not the way how to do it. I went back throughout the thread and actually the only person who commented on it was @kortschak.

I am not sure who @horejsek is, but he repeatedly disputes our observations, feeding us false information without any apology whatsoever. Is it an employee of ProtonMail? Am I angry with this behavior? Sure, I am.

@kortschak > Given that I can delete messages from the box, I'd say that there is actually a bug. @horejsek > Not really, you cannot. Message is not deleted. Bridge ignores the request. @kortschak > This is not the behaviour I see.

@horejsek > You cannot add or remove message to or from All Mail already now. All Mail cannot be changed, it's view over all messages of every mailbox. @kortschak > This is fundamentally not true. I just went to my "All Mail" and deleted an orphaned message (one of those that I noted above) using my desktop client via the bridge. @exander77 > Bullshit. It works with copying messages to All Mail as well. I have done my migration like that and just retested, that I can import mail in All Mail. It shows an email without any folder icon in Web UI and is present in just All Mail.

The most striking thing here is, that both me and @kortschak have actually verified our statements by doing an experiment, not @horejsek.

I would really like if the reports were taken seriously and not disputed. I reported a lot of bugs in a lot of projects but was never argued about my observations.

I would also appreciate if you tune down the apologetics: @horejsek > We know that UX of what users could expect is not ideal. The problem is, everyone is used to something else, and each client is doing something else. There is no perfect solution, only good enough which will always have downsides.

It is not that important to argue about how the client behaves. Clients are not the issue. You can easily see what is in RFCs and how are other providers doing it. For example, I still can't fathom what is the motivation for Trash and Spam being in All Mail. I have never seen it. Can anybody tell me?

Also, what strikes me rather interesting on @horejsek is that he has his Gmail on his Github profile so: 1) Isn't he familiar with how Gmail works and if there are any issues with their approach? 2) Does he even use ProtonMail regularly, meaning daily use both Web UI and Bridge?

exander77 commented 4 years ago

@kortschak In the case, I am mentioning they will never actually move.

It seems that you cannot move to: Inbox -> Sent Sent -> Inbox Sent -> Draft Draft -> Sent Inbox -> Draft Draft -> Inbox Sent -> Starred (does not stare) Inbox -> Starred (does not stare)

You can move to: Sent -> Spam

When you move to: Spam -> Sent it ends up in Inbox... unless it was originally in Sent, then it is vice versa.

Everything is reported as a success... I am not sure what to make of it. It seems like everything succeeds in Web UI. And mails are either incoming or outgoing. Incoming can never be in outgoing and vice versa. But Spam and Trash are for both? And also drafts are separate.

Btw, all of my imported mail is "incoming".

I am sincerely disturbed by ProtonMail a lot. Hence I am reinforced into thinking everything is hostile... well because it is.

Just verified that this behavior is the same with beta.protonmail.com as well.

bartbutler commented 4 years ago

@horejsek is a Proton employee and a full-time bridge developer and as been a major contributor in improving the bridge. There were clearly some clarity issues in the discussion here but there was no intent of deception on his part. It's difficult to have constructive conversations about problems and solutions with the tone of this conversation, and fundamentally, it's not part of his job description to take verbal abuse from people on the Internet, so I'm asking you (politely) to tone down the attacks and try to be constructive.

On to the more substantive comments. You suggest:

  1. Deleting messages from All Mail should move them to Trash.
  2. Deleting messages from Trash should delete them permanently.
  3. Deleting messages from Spam should delete them permanently.
  4. All Mail should not contain Trash or Spam.
  5. The only time a message is deleted is when it is deleted from Trash or Spam. Moving a message should never delete it.

These would be reasonable things to do in a mail system. However, fundamentally, these are product decisions, and have consequences outside the Bridge, as labels/folders are shared with other native apps on other platforms. So while it's not impossible to redefine All Mail, please appreciate that there are reasons (and existing customer expectations) for things being set up the way they are, so yes, we are interested in exploring solutions that correct the buggy behavior that you are reporting, which we all agree is bad, without immediately resorting to fundamentally changing how certain folders and labels work in Proton.

Hence the discussion of possibly using the \Deleted flag to represent mail that is also in Trash, to provide a visual effect indicating deletion without changing the underlying system. The question then becomes, does it work, do the clients handle it well, etc, because, as @horejsek alluded to, IMAP handling among clients is considerably less uniform than one might expect and which is a big reason developing Bridge has been as difficult as it has been.

Regarding your discussion on Inbox/Drafts/Sent, the way this works at Proton generally (not Bridge specific) is that Inbox/Drafts/Sent are considered one folder, with the message ending up with the correct folder according to the internal flags of the message (RECEIVED and SENT). Neither flag -> draft, received -> inbox, sent -> sent, both -> both inbox and sent. If you move it to another exclusive label (folder) like trash or archive, these inbox/sent/draft labels get removed and replaced with trash/archive/etc. Meanwhile, you can also have non-exclusive labels. All Mail is one of these, applied to all messages. We also have AllSent and AllDrafts labels in fact, which also cannot be removed from the messages they apply to--in the web client for instance the user can toggle between which kind of behavior they would like. I don't believe this labels are used in Bridge. You can also have custom non-exclusive labels, where multiple can be added to messages.

Mapping this kind of system to IMAP, where all folders are considered exclusive and therefore messages in folders are considered entirely independent objects, is hard. Gmail does a reasonably good job of this with non-exclusive labels, and we try to as well, but it often involves mapping certain IMAP actions to other Proton actions behind the scenes and then generating the appropriate server side actions to ensure a consistent view and not lose data, and not doing some naive things that would be thought of as screwing up the native clients' inboxes (i.e., duplicating message content around).

So please--this is not a simple problem, there are not simple solutions, and do not take discussion of alternative solutions to yours as some attempt to dodge the problem, because the more complex truth is that Bridge and IMAP is not the only game in town here and when we are looking for a solution we are looking for the solution which works with the whole ProtonMail ecosystem. That may end up involving defining alternative labels for bridge to work with to better meet mail clients expectations, but I'm not ready to completely give up yet.

kortschak commented 4 years ago

Noting that this still affects the bridge at version 1.3.3.

horejsek commented 4 years ago

@kortschak Yes, mentioned issue GODT-409 fixing problems with Spam mailbox is not part of the 1.3.x release. It's in QA right now.

kortschak commented 4 years ago

@horejsek Is there an estimated time that the fix will be released, this issue has been known by ProtonMail since at least March. It still causes issues, sometimes requiring significant work to recover and at the very least requires that a user of the bridge also has open the web client to clean up after the mess that it leaves. Last week I had to spend about 20 minutes on one instance making it possible for the bridge to be able to see incoming email because of this kind of corruption, and there are still emails that exist in "All Mail" but nowhere else because of the sequelae of the mailbox corruption.

kortschak commented 4 years ago

As an additional data point. If a message is incorrectly marked as spam and so moved from the Spam folder to somewhere else in order to add the contact to the whitelist, the same error describing mailbox 4 not existing also comes up. In this case, it is not fixable with the previous approach because that fix required that the mail be deleted, obviously in this case, the mail is wanted and so should not be deleted.

In attempting to deal with this by deleting the messages from the folder they were in ("Archive") and putting them on another server, I find that removing items from the "Archive" folder when they are nowhere else does not put them in "Trash", but again leaves them orphaned, again requiring that messages be deleted from "All Mail".

:tumbleweeds:

andrzejsza commented 4 years ago

@kortschak thank you for all your reports so far. The next release is coming, but since there are still a few issues in QA we are not in a position to provide an estimate. We are also working on some additional improvements for handling labels/folders-related actions and will keep you posted on the new development, once live.

exander77 commented 3 years ago

https://www.reddit.com/r/ProtonMail/comments/jerc0c/all_mail_inbox_noise/

kortschak commented 3 years ago

Since upgrading to 1.4.4 I have not seen this issue present again, watching 1.4.5 now.

kortschak commented 3 years ago

Still good, so I am going to close this as fixed.

exander77 commented 3 years ago

Another complaint about All Mail containing Spam: https://www.reddit.com/r/ProtonMail/comments/l0nf8f/roadmap_for_filters_and_spam_or_just_a_roadmap/