-
-
Notifications
You must be signed in to change notification settings - Fork 109
feat: protect the Date header #6877
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Legacy Python test Lines 603 to 610 in adcc8a9
|
caf6eff to
29d1d87
Compare
|
This has now broken the test added in c14f45a |
29d1d87 to
e533c2c
Compare
e533c2c to
5663fff
Compare
68a4d43 to
154272e
Compare
7a65541 to
89df953
Compare
154272e to
10f3e09
Compare
|
We will have to use something less suspicious than unix epoch, GMX is unhappy: |
d06a61e to
ebb7ce8
Compare
|
I have updated this PR to not send 1970 as the date. Should be good enough for review/merging now. |
|
So now the date looks like this |
ebb7ce8 to
54dce24
Compare
| } else { | ||
| replace_msg_id = if rfc724_mid_orig == rfc724_mid { | ||
| None | ||
| } else if let Some((old_msg_id, old_ts_sent)) = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part looks unrelated because looking at encrypted timestamp_sent is fine, and does this mean that duplicates will be never deleted, even if the message is deleted by the user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When user manually deletes a message, all messages with the same rfc724_mid are deleted. See message::delete_msg_ex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code was deleting duplicates on IMAP, w/o it duplicates will only be deleted locally
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently this code removal corresponds to this part of the commit message:
It may not be a good idea to delete
the duplicate in multi-device setups anyway,
because the device which has a message
may delete the duplicate of a message
the other device missed.
But even if don't delete the duplicate, there's no guarantee the other device didn't miss both messages. Leaving the duplicate is useful though if the user detected a missed message on the other device and asked to resend it. Anyway, if we don't want to IMAP-delete duplicates anymore, i'd add a comment here explaining the motivation / scenario in which keeping the duplicate is useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could add a comment, but also, it's fine as-is, because the code IMAP-deleting duplicates is removed now, and it's more important to explain why we do things than explain why we don't do things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code in imap is being removed, but it was just an optimization removing duplicates before decryption and parsing. A short comment won't make harm.
But i think that it's better to IMAP-delete not the duplicate, but the existing message instead, this way we don't affect receiving re-sent messages by other devices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But i think that it's better to IMAP-delete not the duplicate, but the existing message instead, this way we don't affect receiving re-sent messages by other devices.
If we change from previous behavior (removing last message) to removing the first message, then in multi-device setup with two different versions it's possible to end up with both copies deleted on the server.
54dce24 to
118c10d
Compare
118c10d to
ce0a681
Compare
ce0a681 to
5a694aa
Compare
5a694aa to
9b675b5
Compare
c49a1b1 to
511f093
Compare
69720bc to
3fcbe1d
Compare
3fcbe1d to
a4dd140
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
We do not try to delete resent messages anymore. Previously resent messages were distinguised by having duplicate Message-ID, but future Date, but now we need to download the message before we even see the Date. We now move the message to the destination folder but do not fetch it. It may not be a good idea to delete the duplicate in multi-device setups anyway, because the device which has a message may delete the duplicate of a message the other device missed. To avoid triggering IMAP busy move loop described in the comments we now only move the messages from INBOX and Spam folders.
8d4632a to
6bb9d49
Compare
|
@link2xt can we merge this, or is there a specific reason why you didn't merge it yet? |
Closes #6878
Based on #6867
This depends an a fix #6669, so we may want to postpone merging this until more users have it.
We do not try to delete resent messages
anymore. Previously resent messages
were distinguised by having duplicate Message-ID,
but future Date, but now we need to download
the message before we even see the Date.
We now move the message to the destination folder
but do not fetch it.
It may not be a good idea to delete
the duplicate in multi-device setups anyway,
because the device which has a message
may delete the duplicate of a message
the other device missed.
To avoid triggering IMAP busy move loop
described in the comments
we now only move the messages
from INBOX and Spam folders.