From 51510c97f7b76a73a2032eb089552cf9a13e9274 Mon Sep 17 00:00:00 2001 From: Quentin Dufour Date: Wed, 10 Jan 2024 12:55:38 +0100 Subject: [PATCH] fix some logic error in the internals --- src/imap/command/authenticated.rs | 8 +++--- src/imap/index.rs | 2 +- src/imap/mailbox_view.rs | 6 ++--- src/mail/uidindex.rs | 41 ++++++++++++++++++++----------- 4 files changed, 34 insertions(+), 23 deletions(-) diff --git a/src/imap/command/authenticated.rs b/src/imap/command/authenticated.rs index 5af8e98..f083ac8 100644 --- a/src/imap/command/authenticated.rs +++ b/src/imap/command/authenticated.rs @@ -507,7 +507,7 @@ impl<'a> AuthenticatedContext<'a> { ) -> Result<(Response<'static>, flow::Transition)> { let append_tag = self.req.tag.clone(); match self.append_internal(mailbox, flags, date, message).await { - Ok((_mb, uidvalidity, uid)) => Ok(( + Ok((_mb, uidvalidity, uid, _modseq)) => Ok(( Response::build() .tag(append_tag) .message("APPEND completed") @@ -548,7 +548,7 @@ impl<'a> AuthenticatedContext<'a> { flags: &[Flag<'a>], date: &Option, message: &Literal<'a>, - ) -> Result<(Arc, ImapUidvalidity, ImapUidvalidity)> { + ) -> Result<(Arc, ImapUidvalidity, ImapUid, ModSeq)> { let name: &str = MailboxName(mailbox).try_into()?; let mb_opt = self.user.open_mailbox(&name).await?; @@ -566,9 +566,9 @@ impl<'a> AuthenticatedContext<'a> { let flags = flags.iter().map(|x| x.to_string()).collect::>(); // TODO: filter allowed flags? ping @Quentin - let (uidvalidity, uid) = mb.append(msg, None, &flags[..]).await?; + let (uidvalidity, uid, modseq) = mb.append(msg, None, &flags[..]).await?; - Ok((mb, uidvalidity, uid)) + Ok((mb, uidvalidity, uid, modseq)) } } diff --git a/src/imap/index.rs b/src/imap/index.rs index 4853374..95c16b3 100644 --- a/src/imap/index.rs +++ b/src/imap/index.rs @@ -21,7 +21,7 @@ impl<'a> Index<'a> { .table .get(&uuid) .ok_or(anyhow!("mail is missing from index"))? - .1 + .2 .as_ref(); let i_int: u32 = (i_enum + 1).try_into()?; let i: NonZeroU32 = i_int.try_into()?; diff --git a/src/imap/mailbox_view.rs b/src/imap/mailbox_view.rs index 046acfa..90cfc70 100644 --- a/src/imap/mailbox_view.rs +++ b/src/imap/mailbox_view.rs @@ -108,7 +108,7 @@ impl MailboxView { let old_mail = old_snapshot.table.get(uuid); let new_mail = new_snapshot.table.get(uuid); if old_mail.is_some() && old_mail != new_mail { - if let Some((uid, flags)) = new_mail { + if let Some((uid, _modseq, flags)) = new_mail { data.push(Body::Data(Data::Fetch { seq: NonZeroU32::try_from((i + 1) as u32).unwrap(), items: vec![ @@ -185,7 +185,7 @@ impl MailboxView { let msgs = state .table .iter() - .filter(|(_uuid, (_uid, flags))| flags.iter().any(|x| *x == deleted_flag)) + .filter(|(_uuid, (_uid, _modseq, flags))| flags.iter().any(|x| *x == deleted_flag)) .map(|(uuid, _)| *uuid); for msg in msgs { @@ -438,7 +438,7 @@ impl MailboxView { .table .values() .enumerate() - .find(|(_i, (_imap_uid, flags))| !flags.contains(&"\\Seen".to_string())) + .find(|(_i, (_imap_uid, _modseq, flags))| !flags.contains(&"\\Seen".to_string())) .map(|(i, _)| NonZeroU32::try_from(i as u32 + 1)) .transpose()?) } diff --git a/src/mail/uidindex.rs b/src/mail/uidindex.rs index f1c522c..e7023cf 100644 --- a/src/mail/uidindex.rs +++ b/src/mail/uidindex.rs @@ -136,7 +136,7 @@ impl BayouState for UidIndex { // Change UIDValidity if there is a UID conflict or a MODSEQ conflict // @FIXME Need to prove that summing work // The intuition: we increase the UIDValidity by the number of possible conflicts - if *uid < new.internalseq || *modseq < new.highestmodseq { + if *uid < new.internalseq || *modseq < new.internalmodseq { let bump_uid = new.internalseq.get() - uid.get(); let bump_modseq = new.internalmodseq.get() - modseq.get(); new.uidvalidity = @@ -148,7 +148,7 @@ impl BayouState for UidIndex { let new_uid = new.internalseq; // Assign the real modseq of the email and its new flags - let new_modseq = new.highestmodseq; + let new_modseq = new.internalmodseq; // Delete the previous entry if any. // Our proof has no assumption on `ident` uniqueness, @@ -175,11 +175,11 @@ impl BayouState for UidIndex { // We update the counter new.internalseq = NonZeroU32::new(new.internalseq.get() + 1).unwrap(); } - UidIndexOp::FlagAdd(ident, modseq, new_flags) => { - if let Some((uid, modseq, existing_flags)) = new.table.get_mut(ident) { + UidIndexOp::FlagAdd(ident, candidate_modseq, new_flags) => { + if let Some((uid, email_modseq, existing_flags)) = new.table.get_mut(ident) { // Bump UIDValidity if required - if *modseq < new.highestmodseq { - let bump_modseq = new.internalmodseq.get() - modseq.get(); + if *candidate_modseq < new.internalmodseq { + let bump_modseq = new.internalmodseq.get() - candidate_modseq.get(); new.uidvalidity = NonZeroU32::new(new.uidvalidity.get() + bump_modseq) .unwrap(); @@ -192,7 +192,8 @@ impl BayouState for UidIndex { .cloned() .collect(); new.idx_by_flag.insert(*uid, &to_add); - new.idx_by_modseq.insert(*modseq, *ident); + *email_modseq = new.internalmodseq; + new.idx_by_modseq.insert(new.internalmodseq, *ident); existing_flags.append(&mut to_add); // Update counters @@ -200,11 +201,11 @@ impl BayouState for UidIndex { new.internalmodseq = NonZeroU32::new(new.internalmodseq.get() + 1).unwrap(); } } - UidIndexOp::FlagDel(ident, modseq, rm_flags) => { - if let Some((uid, modseq, existing_flags)) = new.table.get_mut(ident) { + UidIndexOp::FlagDel(ident, candidate_modseq, rm_flags) => { + if let Some((uid, email_modseq, existing_flags)) = new.table.get_mut(ident) { // Bump UIDValidity if required - if *modseq < new.highestmodseq { - let bump_modseq = new.internalmodseq.get() - modseq.get(); + if *candidate_modseq < new.internalmodseq { + let bump_modseq = new.internalmodseq.get() - candidate_modseq.get(); new.uidvalidity = NonZeroU32::new(new.uidvalidity.get() + bump_modseq) .unwrap(); @@ -215,15 +216,24 @@ impl BayouState for UidIndex { new.idx_by_flag.remove(*uid, rm_flags); // Register that email has been modified - new.idx_by_modseq.insert(*modseq, *ident); + new.idx_by_modseq.insert(new.internalmodseq, *ident); + *email_modseq = new.internalmodseq; // Update counters new.highestmodseq = new.internalmodseq; new.internalmodseq = NonZeroU32::new(new.internalmodseq.get() + 1).unwrap(); } } - UidIndexOp::FlagSet(ident, modseq, new_flags) => { - if let Some((uid, modseq, existing_flags)) = new.table.get_mut(ident) { + UidIndexOp::FlagSet(ident, candidate_modseq, new_flags) => { + if let Some((uid, email_modseq, existing_flags)) = new.table.get_mut(ident) { + // Bump UIDValidity if required + if *candidate_modseq < new.internalmodseq { + let bump_modseq = new.internalmodseq.get() - candidate_modseq.get(); + new.uidvalidity = + NonZeroU32::new(new.uidvalidity.get() + bump_modseq) + .unwrap(); + } + // Remove flags from the source of trust and the cache let (keep_flags, rm_flags): (Vec, Vec) = existing_flags .iter() @@ -240,7 +250,8 @@ impl BayouState for UidIndex { new.idx_by_flag.insert(*uid, &to_add); // Register that email has been modified - new.idx_by_modseq.insert(*modseq, *ident); + new.idx_by_modseq.insert(new.internalmodseq, *ident); + *email_modseq = new.internalmodseq; // Update counters new.highestmodseq = new.internalmodseq;