From 55702e4985332d890bf5da32be9b9816d188c908 Mon Sep 17 00:00:00 2001 From: l Date: Wed, 30 Oct 2024 02:38:15 +0000 Subject: [PATCH] fix: skip IDLE if we got unsolicited FETCH (#6130) This may indicate that there was a new \Seen flag that we don't want to skip. Also don't drain unsolicited responses while scanning folders. Now we only drain unsolicited responses right before IDLE and always redo the whole fetch cycle if there have been some. Some message in the scanned folder may not be fetched that would be previously fetched otherwise, but it will be picked up on the next folder scan. --- src/imap.rs | 62 ++++++++++++++++++++++++++++------------ src/imap/idle.rs | 2 +- src/imap/scan_folders.rs | 20 ++++--------- 3 files changed, 50 insertions(+), 34 deletions(-) diff --git a/src/imap.rs b/src/imap.rs index 297af1a2a..f23045791 100644 --- a/src/imap.rs +++ b/src/imap.rs @@ -1200,6 +1200,8 @@ impl Session { .await .context("failed to fetch flags")?; + let mut got_unsolicited_fetch = false; + while let Some(fetch) = list .try_next() .await @@ -1209,6 +1211,7 @@ impl Session { uid } else { info!(context, "FETCH result contains no UID, skipping"); + got_unsolicited_fetch = true; continue; }; let is_seen = fetch.flags().any(|flag| flag == Flag::Seen); @@ -1231,6 +1234,15 @@ impl Session { warn!(context, "FETCH result contains no MODSEQ"); } } + drop(list); + + if got_unsolicited_fetch { + // We got unsolicited FETCH, which means some flags + // have been modified while our request was in progress. + // We may or may not have these new flags as a part of the response, + // so better skip next IDLE and do another round of flag synchronization. + self.new_mail = true; + } set_modseq(context, folder, highest_modseq) .await @@ -1716,17 +1728,21 @@ impl Imap { } impl Session { - /// Return whether the server sent an unsolicited EXISTS response. + /// Return whether the server sent an unsolicited EXISTS or FETCH response. + /// /// Drains all responses from `session.unsolicited_responses` in the process. - /// If this returns `true`, this means that new emails arrived and you should - /// fetch again, even if you just fetched. - fn server_sent_unsolicited_exists(&self, context: &Context) -> Result { + /// + /// If this returns `true`, this means that new emails arrived + /// or flags have been changed. + /// In this case we may want to skip next IDLE and do a round + /// of fetching new messages and synchronizing seen flags. + fn drain_unsolicited_responses(&self, context: &Context) -> Result { use async_imap::imap_proto::Response; use async_imap::imap_proto::ResponseCode; use UnsolicitedResponse::*; let folder = self.selected_folder.as_deref().unwrap_or_default(); - let mut unsolicited_exists = false; + let mut should_refetch = false; while let Ok(response) = self.unsolicited_responses.try_recv() { match response { Exists(_) => { @@ -1734,28 +1750,38 @@ impl Session { context, "Need to refetch {folder:?}, got unsolicited EXISTS {response:?}" ); - unsolicited_exists = true; + should_refetch = true; } - // We are not interested in the following responses and they are are - // sent quite frequently, so, we ignore them without logging them Expunge(_) | Recent(_) => {} - Other(response_data) - if matches!( - response_data.parsed(), - Response::Fetch { .. } - | Response::Done { - code: Some(ResponseCode::CopyUid(_, _, _)), - .. - } - ) => {} + Other(ref response_data) => { + match response_data.parsed() { + Response::Fetch { .. } => { + info!( + context, + "Need to refetch {folder:?}, got unsolicited FETCH {response:?}" + ); + should_refetch = true; + } + // We are not interested in the following responses and they are are + // sent quite frequently, so, we ignore them without logging them. + Response::Done { + code: Some(ResponseCode::CopyUid(_, _, _)), + .. + } => {} + + _ => { + info!(context, "{folder:?}: got unsolicited response {response:?}") + } + } + } _ => { info!(context, "{folder:?}: got unsolicited response {response:?}") } } } - Ok(unsolicited_exists) + Ok(should_refetch) } } diff --git a/src/imap/idle.rs b/src/imap/idle.rs index 892e8757b..f53a1d1cc 100644 --- a/src/imap/idle.rs +++ b/src/imap/idle.rs @@ -32,7 +32,7 @@ impl Session { self.select_with_uidvalidity(context, folder).await?; - if self.server_sent_unsolicited_exists(context)? { + if self.drain_unsolicited_responses(context)? { self.new_mail = true; } diff --git a/src/imap/scan_folders.rs b/src/imap/scan_folders.rs index a4e17d04d..d102684d5 100644 --- a/src/imap/scan_folders.rs +++ b/src/imap/scan_folders.rs @@ -66,21 +66,11 @@ impl Imap { && folder_meaning != FolderMeaning::Drafts && folder_meaning != FolderMeaning::Trash { - // Drain leftover unsolicited EXISTS messages - session.server_sent_unsolicited_exists(context)?; - - loop { - self.fetch_move_delete(context, session, folder.name(), folder_meaning) - .await - .context("Can't fetch new msgs in scanned folder") - .log_err(context) - .ok(); - - // If the server sent an unsocicited EXISTS during the fetch, we need to fetch again - if !session.server_sent_unsolicited_exists(context)? { - break; - } - } + self.fetch_move_delete(context, session, folder.name(), folder_meaning) + .await + .context("Can't fetch new msgs in scanned folder") + .log_err(context) + .ok(); } }