Skip to content

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: protect the Date header #6877

wants to merge 1 commit into from

Conversation

link2xt
Copy link
Collaborator

@link2xt link2xt commented May 28, 2025 •
edited
Loading

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 move loop
described in the comments
we now only move the messages
from INBOX and Spam folders.

@link2xt
Copy link
Collaborator Author

link2xt commented May 28, 2025 •
edited
Loading

Legacy Python test tests/test_1_online.py::test_resend_message got broken by this.
Resent message is not recognized as duplicate anymore because outer timestamp of just received message is compared to the timestamp of the fetched message:

core/src/imap.rs

Lines 603 to 610 in adcc8a9

let ts_sent = headers
.get_header_value(HeaderDef::Date)
.and_then(|v| mailparse::dateparse(&v).ok())
.unwrap_or_default();
let is_dup = is_dup_msg(is_chat_msg, ts_sent, ts_sent_old);
if is_dup {
info!(context, "Deleting duplicate message {message_id}.");
}

@link2xt link2xt force-pushed the link2xt/protect-date-header branch 4 times, most recently from caf6eff to 29d1d87 Compare May 28, 2025 07:51
@link2xt
Copy link
Collaborator Author

link2xt commented May 28, 2025

This has now broken the test added in c14f45a

@link2xt link2xt marked this pull request as draft May 28, 2025 09:37
@link2xt link2xt self-assigned this May 28, 2025
@link2xt link2xt force-pushed the link2xt/protect-date-header branch from 29d1d87 to e533c2c Compare May 28, 2025 09:51
@link2xt link2xt changed the base branch from main to link2xt/scan-folders-lock May 28, 2025 11:46
@link2xt link2xt force-pushed the link2xt/protect-date-header branch from e533c2c to 5663fff Compare May 28, 2025 11:48
@link2xt link2xt marked this pull request as ready for review May 28, 2025 11:49
@link2xt link2xt force-pushed the link2xt/protect-date-header branch 5 times, most recently from 68a4d43 to 154272e Compare May 28, 2025 15:46
@link2xt link2xt force-pushed the link2xt/scan-folders-lock branch from 7a65541 to 89df953 Compare May 28, 2025 15:46
Base automatically changed from link2xt/scan-folders-lock to main May 28, 2025 16:05
@link2xt link2xt force-pushed the link2xt/protect-date-header branch from 154272e to 10f3e09 Compare May 30, 2025 11:17
@link2xt link2xt mentioned this pull request May 30, 2025
Open
@link2xt link2xt marked this pull request as draft May 30, 2025 19:17
@link2xt
Copy link
Collaborator Author

link2xt commented May 30, 2025

We will have to use something less suspicious than unix epoch, GMX is unhappy:
#6878 (comment)

@link2xt link2xt force-pushed the link2xt/protect-date-header branch 2 times, most recently from d06a61e to ebb7ce8 Compare June 22, 2025 14:50
@link2xt link2xt marked this pull request as ready for review June 22, 2025 14:50
@link2xt
Copy link
Collaborator Author

link2xt commented Jun 22, 2025

I have updated this PR to not send 1970 as the date. Should be good enough for review/merging now.

@link2xt link2xt requested review from Hocuri and iequidoo June 22, 2025 14:51
@link2xt
Copy link
Collaborator Author

link2xt commented Jun 22, 2025

So now the date looks like this Date: Sun, 15 Jun 2025 15:06:40 +0000.
You convert it and get:

date -d 'Sun, 15 Jun 2025 15:06:40 +0000' +%s
1750000000

@link2xt
feat: protect the Date header
54dce24 
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 move loop
described in the comments
we now only move the messages
from INBOX and Spam folders.
@link2xt link2xt force-pushed the link2xt/protect-date-header branch from ebb7ce8 to 54dce24 Compare June 22, 2025 17:24
iequidoo
iequidoo reviewed Jun 22, 2025
View reviewed changes
src/download.rs
@@ -162,25 +162,18 @@ pub(crate) async fn download_msg(
|row| {
let server_uid: u32 = row.get(0)?;
let server_folder: String = row.get(1)?;
let uidvalidity: u32 = row.get(2)?;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not looking at uidvalidity anymore? Moreover, it's still SELECTed above

src/imap.rs
@@ -581,52 +581,26 @@ impl Imap {

// Determine the target folder where the message should be moved to.
//
// If we have seen the message on the IMAP server before, do not move it.
// We only move the messages from the INBOX folder.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and Spam folders.

src/mimefactory.rs
//
// and the explanation page says
// "The time information deviates too much from the actual time".
let coarse_grained_timestamp = self.timestamp - (self.timestamp % 1000000);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just ~11.6 days, can't it be longer, say, several months? Secondly, this actually leaks the date when the remainder is 0 or close to it and the message reaches the server immediately or quickly. It'd be better to introduce a Config key with the currently used fake date and advance it randomly when it's too old. We have the config cache, so it will be cheap.

src/receive_imf.rs
@@ -253,27 +239,8 @@ pub(crate) async fn receive_imf_inner(
} else {
replace_msg_id = if rfc724_mid_orig == rfc724_mid {
None
} else if let Some((old_msg_id, old_ts_sent)) =
Copy link
Collaborator

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iequidoo iequidoo iequidoo left review comments

@Hocuri Hocuri Awaiting requested review from Hocuri

At least 1 approving review is required to merge this pull request.

Assignees

@link2xt link2xt

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Protect the Date header
2 participants
@link2xt @iequidoo