diff --git a/crates/nuncio-core/src/model.rs b/crates/nuncio-core/src/model.rs index 0df4a26..1b76353 100644 --- a/crates/nuncio-core/src/model.rs +++ b/crates/nuncio-core/src/model.rs @@ -350,17 +350,30 @@ impl Email { /// The surrogate tier is folder-scoped and therefore *not* stable across a /// move; it is the honest answer when the server offered nothing better. /// - /// # Keys do not yet converge across engine types + /// # What convergence across engine types does and does not follow from /// /// Precedence makes a key reproducible from the same inputs, not from the - /// same *message*. The IMAP engine does not currently request `EMAILID` or - /// `X-GM-MSGID`, so it can only ever reach the `Message-ID`+content tier or - /// the surrogate, while JMAP supplies a server id and enters at the top - /// tier. Two engines syncing one account therefore produce disjoint key - /// spaces: the same message is two identities, one per engine. This is a - /// known gap in the convergent multi-engine model, not a property to rely - /// on -- do not assume a key derived by one engine addresses the row - /// another engine wrote. + /// same *message*. Two engines converge exactly when they enter at the same + /// tier with the same value. + /// + /// They do when the server publishes a stable object id and both engines + /// ask for it. The IMAP engine requests RFC 8474 `EMAILID` from an + /// `OBJECTID` server and `X-GM-MSGID` from an `X-GM-EXT-1` one, and JMAP's + /// Email object id is the same identifier RFC 8474 exposes to IMAP -- so + /// one message synced through both protocols yields one key. That is the + /// whole point of the OBJECTID extension. + /// + /// They do not when the server offers neither. IMAP then falls to the + /// `Message-ID`+content tier or the surrogate, both derived from what the + /// message or the mailbox happened to contain, while a JMAP engine against + /// the same account still enters at the top tier from a server-assigned id. + /// Those key spaces are disjoint, and no comparison across them means + /// anything. The surrogate additionally is not stable across a *move*, even + /// within one engine. + /// + /// Object ids are also only comparable within one account on one server: + /// RFC 8474 section 4 scopes uniqueness that way, which is why every tier + /// here is account-scoped. pub fn derive_message_key( account_id: &str, remote: RemoteIdentity<'_>, diff --git a/crates/nuncio-mail/src/imap.rs b/crates/nuncio-mail/src/imap.rs index 57c5922..d7c9130 100644 --- a/crates/nuncio-mail/src/imap.rs +++ b/crates/nuncio-mail/src/imap.rs @@ -175,6 +175,13 @@ pub(crate) struct MailboxSyncCapabilities { /// `NOMODSEQ` has no mod-sequences to compare against no matter what the /// server advertises. pub mailbox_has_modseq: bool, + /// Server advertises `OBJECTID` (RFC 8474), so `EMAILID` may be asked for. + /// Asking a server that did not advertise it is not merely rude: an + /// unrecognised FETCH item is answered `BAD`, costing a round trip per + /// batch to learn nothing. + pub objectid: bool, + /// Server advertises `X-GM-EXT-1`, so `X-GM-MSGID` may be asked for. + pub gmail_extensions: bool, } /// One folder-sync pass, resolved before any body is fetched. @@ -182,6 +189,11 @@ pub(crate) struct MailboxSyncCapabilities { pub(crate) struct FolderSyncPlan { /// The mechanism this pass negotiated. pub rung: SyncRung, + /// What the negotiation found the server able to do. Carried past the + /// planning stage because the identity extensions decide what the *fetch* + /// asks for, and a fetch that asks for an unadvertised item is answered + /// `BAD`. + pub caps: MailboxSyncCapabilities, pub uid_validity: Option, pub uid_next: Option, pub highest_modseq: Option, @@ -326,6 +338,24 @@ pub fn build_fetch_command_query() -> &'static str { "(FLAGS INTERNALDATE RFC822.SIZE ENVELOPE BODY.PEEK[])" } +/// Add `X-GM-MSGID` to a FETCH item list when, and only when, the server +/// advertised `X-GM-EXT-1`. +/// +/// Gmail's message id is the second identity tier, and unlike RFC 8474's +/// `EMAILID` it can be asked for through the typed client: `imap-proto` parses +/// the attribute and `Fetch::gmail_msg_id` reads it back. So it rides along on +/// the fetch the sync already issues and costs no extra round trip. +/// +/// A server that advertises nothing gets a query byte-identical to the one it +/// got before this existed. That is the point: capability negotiation must not +/// change what an unextended server sees. +fn with_gmail_msgid(items: &str, advertised: bool) -> String { + match items.strip_suffix(')').filter(|_| advertised) { + Some(head) => format!("{head} X-GM-MSGID)"), + None => items.to_string(), + } +} + /// The `UID FETCH` query for the cheap enumeration pass: UID plus `RFC822.SIZE` /// only, deliberately WITHOUT a `BODY.PEEK[]`. This lets the sync discover every /// message's UID and size (to decide per-message whether the body is safe to @@ -906,15 +936,24 @@ impl ImapEngine { drop(enumeration); let mut emails = Vec::new(); + let body_query = with_gmail_msgid(build_fetch_command_query(), plan.caps.gmail_extensions); + let header_query = with_gmail_msgid( + build_headers_only_command_query(), + plan.caps.gmail_extensions, + ); // Phase 2a: normal messages -- full bodies, in bounded UID-set batches. for chunk in body_uids.chunks(batch_size) { let set = join_uid_batch(chunk); + // Read the batch's object ids first. It has to be its own command: + // `EMAILID` cannot ride on the body fetch, because that response + // goes through a parser which treats the item as a fatal error. + // Issuing it here is also what keeps the raw layer's precondition + // true -- the last thing on this connection was a tagged + // completion, so the client holds no buffered bytes. + let object_ids = self.fetch_object_ids(session, plan.caps, &set).await?; let items = { - let mut fetch_stream = session - .uid_fetch(&set, build_fetch_command_query()) - .await - .map_err(|e| { + let mut fetch_stream = session.uid_fetch(&set, &body_query).await.map_err(|e| { MailError::ImapError(format!( "UID FETCH failed for folder '{}': {}", folder_id, e @@ -927,11 +966,16 @@ impl ImapEngine { // `UID FETCH` must carry one (RFC 3501 section 6.4.8), so this // only fires for a non-conforming server. Recording it anyway // would write a placement at the meaningless UID 0. - if item.uid.filter(|u| *u >= 1).is_none() { + let Some(uid) = item.uid.filter(|u| *u >= 1) else { skipped_no_uid += 1; continue; - } - emails.push(self.build_email_from_fetch(folder_id, server_uid_validity, item)?); + }; + emails.push(self.build_email_from_fetch( + folder_id, + server_uid_validity, + item, + object_ids.get(&uid).map(String::as_str), + )?); } } @@ -940,11 +984,13 @@ impl ImapEngine { // buffer an unbounded number of header fetches at once. for chunk in oversized_uids.chunks(batch_size) { let set = join_uid_batch(chunk); + // An oversized message needs the object id most: its body is never + // fetched, so there is no content hash and the alternative is the + // folder-scoped surrogate. + let object_ids = self.fetch_object_ids(session, plan.caps, &set).await?; let items = { - let mut fetch_stream = session - .uid_fetch(&set, build_headers_only_command_query()) - .await - .map_err(|e| { + let mut fetch_stream = + session.uid_fetch(&set, &header_query).await.map_err(|e| { MailError::ImapError(format!( "UID FETCH (headers) failed for folder '{}': {}", folder_id, e @@ -955,11 +1001,16 @@ impl ImapEngine { for item in &items { // Same guard as the body pass: an item with no UID cannot be // placed, and UID 0 is not an address. - if item.uid.filter(|u| *u >= 1).is_none() { + let Some(uid) = item.uid.filter(|u| *u >= 1) else { skipped_no_uid += 1; continue; - } - emails.push(self.build_headers_only_email(folder_id, server_uid_validity, item)); + }; + emails.push(self.build_headers_only_email( + folder_id, + server_uid_validity, + item, + object_ids.get(&uid).map(String::as_str), + )); } } @@ -1067,12 +1118,19 @@ impl ImapEngine { where S: AsyncRead + AsyncWrite + Unpin + Send + Debug + 'static, { - let (has_qresync, has_condstore) = { + let (has_qresync, has_condstore, has_objectid, has_gmail) = { let caps = session .capabilities() .await .map_err(|e| MailError::ImapError(format!("CAPABILITY query failed: {e}")))?; - (caps.has_str("QRESYNC"), caps.has_str("CONDSTORE")) + ( + caps.has_str("QRESYNC"), + caps.has_str("CONDSTORE"), + // RFC 8474 names the capability OBJECTID; EMAILID is the FETCH + // item it brings, and asking for it otherwise earns a BAD. + caps.has_str("OBJECTID"), + caps.has_str("X-GM-EXT-1"), + ) }; let checkpoint = since_state.and_then(parse_checkpoint); @@ -1127,6 +1185,8 @@ impl ImapEngine { qresync_enabled, condstore: has_condstore, mailbox_has_modseq: selected.highest_modseq.is_some(), + objectid: has_objectid, + gmail_extensions: has_gmail, }; let mut rung = choose_rung(checkpoint, selected.uid_validity, caps); // The QRESYNC SELECT was issued optimistically; if the server answered @@ -1144,6 +1204,7 @@ impl ImapEngine { let mut plan = FolderSyncPlan { rung, + caps, uid_validity: selected.uid_validity, uid_next: selected.uid_next, highest_modseq: selected.highest_modseq, @@ -1202,6 +1263,29 @@ impl ImapEngine { Ok(plan) } + /// The batch's `uid -> EMAILID` map, or an empty one when the server never + /// advertised RFC 8474 `OBJECTID`. + /// + /// The capability gate is the whole reason this is a separate method: a + /// server without `OBJECTID` must see exactly the traffic it saw before + /// object ids existed -- no extra command, no extra round trip -- and a + /// server that has it must be asked through the raw layer rather than the + /// typed client, which cannot parse the reply without dying. + async fn fetch_object_ids( + &self, + session: &mut async_imap::Session, + caps: MailboxSyncCapabilities, + uid_set: &str, + ) -> Result, MailError> + where + S: AsyncRead + AsyncWrite + Unpin + Send + Debug + 'static, + { + if !caps.objectid { + return Ok(std::collections::BTreeMap::new()); + } + crate::imap_raw::uid_fetch_email_ids(session, uid_set).await + } + /// List the folder's UIDs, optionally narrowed to those changed since /// `changed_since`. Requests no bodies and no flags, so the cost is one /// short line per message rather than a download. @@ -1267,12 +1351,15 @@ impl ImapEngine { /// and spreading them out buys no clarity while pushing this function past /// the argument-count lint. /// - /// In practice every IMAP call site passes `email_id: None` and - /// `gm_msgid: None` today, because the FETCH never asks for them, so this - /// engine reaches only the `Message-ID`+content tier or the surrogate. The - /// JMAP engine enters at the top tier, which means an IMAP-derived key and - /// a JMAP-derived key for the *same* message will not match. Syncing one - /// account over both engines does not converge on a shared identity yet. + /// Both server-assigned tiers are reachable from here. `EMAILID` comes from + /// a raw `UID FETCH` issued only against an `OBJECTID` server + /// ([`crate::imap_raw::uid_fetch_email_ids`]) and `X-GM-MSGID` from the + /// typed fetch against an `X-GM-EXT-1` server, so an IMAP-derived key and a + /// JMAP-derived key for the same message on a server that publishes object + /// ids through both protocols are the same key -- which is what RFC 8474 + /// exists to make possible. Against a server that advertises neither, this + /// engine still lands on the `Message-ID`+content tier or the surrogate, + /// and those do not converge with a JMAP engine's server-assigned ids. fn place( &self, folder_id: &str, @@ -1298,11 +1385,16 @@ impl ImapEngine { /// when it does not (a server that returned only the ENVELOPE), a minimal /// record is synthesised from the envelope so the message is still surfaced /// honestly. + /// + /// `email_id` is this UID's RFC 8474 object id when the folder pass read + /// one; `X-GM-MSGID` is read from the item itself, since the typed parser + /// understands that attribute and the raw layer never needs to see it. fn build_email_from_fetch( &self, folder_id: &str, server_uid_validity: Option, fetch_data: &async_imap::types::Fetch, + email_id: Option<&str>, ) -> Result { let uid_num = fetch_data.uid.unwrap_or(0); // The message is addressed on the wire by its UID within the folder's @@ -1318,19 +1410,11 @@ impl ImapEngine { .flags() .any(|flag| matches!(flag, async_imap::types::Flag::Seen)); - // EMAILID (RFC 8474) and X-GM-MSGID are not requested by the current - // FETCH, so identity rests on the Message-ID/content pair where the - // message carried a Message-ID and on the folder-scoped surrogate - // otherwise. - // - // Adding those FETCH items is not a local change. Every already-stored - // message would derive a DIFFERENT key on its next resync, so each - // re-reported placement repoints to the new key -- and the old message - // row, now referenced by nothing, is never reaped. Its decrypted body - // stays in the plaintext FTS index forever, which is exactly the leak - // the reap on placement removal exists to prevent. The FETCH items can - // only be requested once the repoint path reaps the message it - // abandons; until then, requesting them leaks plaintext. + // Gmail's id is a 64-bit integer on the wire; it is rendered decimal + // here because identity inputs are hashed as strings and the rendering + // has to be the one every engine would produce from the same number. + let gm_msgid = fetch_data.gmail_msg_id().map(u64::to_string); + if let Some(raw_bytes) = fetch_data.body() { let mut email = MimeParserAdapter::parse_mime(&self.account_id, raw_bytes)?; let (identity, placement) = self.place( @@ -1338,8 +1422,8 @@ impl ImapEngine { &uid_validity, &remote_id, RemoteIdentity { - email_id: None, - gm_msgid: None, + email_id, + gm_msgid: gm_msgid.as_deref(), message_id: email.message_id.as_deref(), content_hash: email.content_hash.as_deref(), }, @@ -1383,16 +1467,16 @@ impl ImapEngine { .and_then(|raw| Email::normalize_message_id(&String::from_utf8_lossy(raw))); // With no octets there is no content hash, and the `Message-ID` alone - // is never enough to key on, so this path lands on the surrogate tier. - // That is the honest answer: nothing was fetched that could prove two - // folders hold the same message. + // is never enough to key on, so absent a server-assigned id this path + // lands on the surrogate tier. That is the honest answer: nothing was + // fetched that could prove two folders hold the same message. let (identity, placement) = self.place( folder_id, &uid_validity, &remote_id, RemoteIdentity { - email_id: None, - gm_msgid: None, + email_id, + gm_msgid: gm_msgid.as_deref(), message_id: message_id.as_deref(), content_hash: None, }, @@ -1429,6 +1513,7 @@ impl ImapEngine { folder_id: &str, server_uid_validity: Option, fetch_data: &async_imap::types::Fetch, + email_id: Option<&str>, ) -> PlacedMessage { let uid_num = fetch_data.uid.unwrap_or(0); let remote_id = uid_num.to_string(); @@ -1438,6 +1523,7 @@ impl ImapEngine { let is_read = fetch_data .flags() .any(|flag| matches!(flag, async_imap::types::Flag::Seen)); + let gm_msgid = fetch_data.gmail_msg_id().map(u64::to_string); if let Some(header_bytes) = fetch_data.header() { if let Ok(mut email) = MimeParserAdapter::parse_mime(&self.account_id, header_bytes) { @@ -1452,16 +1538,17 @@ impl ImapEngine { // something else, so drop it; the `Message-ID` it recovered // from those headers is still correct and is kept. Dropping it // also costs this message the content tier -- a `Message-ID` - // never keys on its own -- so an oversized message identifies - // by its folder-scoped surrogate until the octets are fetched. + // never keys on its own -- so absent a server-assigned id an + // oversized message identifies by its folder-scoped surrogate + // until the octets are fetched. email.content_hash = None; let (identity, placement) = self.place( folder_id, &uid_validity, &remote_id, RemoteIdentity { - email_id: None, - gm_msgid: None, + email_id, + gm_msgid: gm_msgid.as_deref(), message_id: email.message_id.as_deref(), content_hash: None, }, @@ -1479,13 +1566,17 @@ impl ImapEngine { // No usable header section: fall back to whatever the envelope offers. // `build_email_from_fetch` cannot error on this path (no body to parse), // but if it ever did, surface a minimal honest record rather than panic. - self.build_email_from_fetch(folder_id, server_uid_validity, fetch_data) + self.build_email_from_fetch(folder_id, server_uid_validity, fetch_data, email_id) .unwrap_or_else(|_| { let (identity, placement) = self.place( folder_id, &uid_validity, &remote_id, - RemoteIdentity::default(), + RemoteIdentity { + email_id, + gm_msgid: gm_msgid.as_deref(), + ..RemoteIdentity::default() + }, is_read, ); PlacedMessage { @@ -2141,6 +2232,10 @@ mod tests { qresync_enabled: true, condstore: true, mailbox_has_modseq: true, + // The identity extensions decide what a FETCH asks for, never + // which rung a pass runs on; they are set for completeness. + objectid: true, + gmail_extensions: true, } } @@ -2217,7 +2312,8 @@ mod tests { MailboxSyncCapabilities { qresync_enabled: false, condstore: true, - mailbox_has_modseq: true + mailbox_has_modseq: true, + ..MailboxSyncCapabilities::default() } ), SyncRung::Condstore @@ -2231,7 +2327,8 @@ mod tests { MailboxSyncCapabilities { qresync_enabled: false, condstore: false, - mailbox_has_modseq: false + mailbox_has_modseq: false, + ..MailboxSyncCapabilities::default() } ), SyncRung::UidSetDiff @@ -2247,7 +2344,8 @@ mod tests { MailboxSyncCapabilities { qresync_enabled: true, condstore: true, - mailbox_has_modseq: false + mailbox_has_modseq: false, + ..MailboxSyncCapabilities::default() } ), SyncRung::Full diff --git a/crates/nuncio-mail/src/imap_raw.rs b/crates/nuncio-mail/src/imap_raw.rs index cee5f64..b4eb29b 100644 --- a/crates/nuncio-mail/src/imap_raw.rs +++ b/crates/nuncio-mail/src/imap_raw.rs @@ -1,8 +1,8 @@ //! Raw IMAP command layer: the responses `async-imap`'s typed API discards. //! -//! The engine needs four things the typed client cannot express, and all four -//! decide whether a sync saw everything or whether a mutation actually -//! happened: +//! The engine needs five things the typed client cannot express, and they all +//! decide whether a sync saw everything, whether a mutation actually happened, +//! or whether two engines can agree on what a message *is*: //! //! | Needed | Why the typed API cannot give it | //! | --- | --- | @@ -10,6 +10,7 @@ //! | `SELECT (QRESYNC …)` | no such method, and `select()`'s parser drops `VANISHED` | //! | `COPYUID` | `uid_copy`/`uid_mv` return `Result<()>`; the tagged response *code* is discarded | //! | `[MODIFIED …]` | same discard, and `imap-proto` has no `ResponseCode` variant for it at all | +//! | `EMAILID` | `imap-proto` cannot *parse* it, and failing to parse kills the connection -- see [`uid_fetch_email_ids`] | //! //! ## Why this is hand-rolled rather than a fork //! @@ -32,13 +33,15 @@ //! against a server that emits the real syntax, and the alternative is having //! no conflict detection at all. +use std::collections::BTreeMap; use std::fmt::Debug; +use std::sync::atomic::{AtomicU64, Ordering}; use async_imap::Session; // Reached through `async-imap`'s re-export rather than a direct dependency, // so the parser types can never version-skew against the client using them. use async_imap::imap_proto::{Response, ResponseCode, Status, UidSetMember}; -use tokio::io::{AsyncRead, AsyncWrite}; +use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; use crate::parser::MailError; @@ -386,10 +389,326 @@ where Ok(exchange) } +/// Upper bound on the bytes one raw exchange will accumulate before giving up. +/// +/// The raw reader has no framing beyond "a line beginning with our tag", so a +/// server that answers with an endless stream and never completes the command +/// would otherwise grow this buffer without limit. One `EMAILID` line is a few +/// dozen bytes and a batch is capped at a few hundred messages, so this is +/// orders of magnitude above any legitimate reply. +const MAX_RAW_RESPONSE_BYTES: usize = 4 * 1024 * 1024; + +/// Tag prefix for commands this module writes straight to the transport. +/// +/// `async-imap` allocates its own tags from an `IdGenerator` that emits +/// `format!("A{:04}", n % 10_000)` -- always the letter `A` followed by four +/// decimal digits. A tag starting with `NX` therefore cannot collide with one +/// the typed client will ever issue, no matter how many commands a session +/// runs or how often the generator wraps. Colliding would be worse than +/// untidy: two commands sharing a tag makes each one's completion look like +/// the other's, and a reader would stop at the wrong response. +const RAW_TAG_PREFIX: &str = "NX"; + +/// Sequence behind [`RAW_TAG_PREFIX`]. Process-wide rather than per-session +/// because it costs nothing and makes a tag unique in a packet capture across +/// every connection, which is the only thing anyone reads these tags for. +static RAW_TAG_SEQUENCE: AtomicU64 = AtomicU64::new(0); + +fn next_raw_tag() -> String { + let n = RAW_TAG_SEQUENCE.fetch_add(1, Ordering::Relaxed); + format!("{RAW_TAG_PREFIX}{n}") +} + +/// `UID FETCH (UID EMAILID)` (RFC 8474 section 5.2), read straight off +/// the transport, returning `uid -> EMAILID`. +/// +/// # Why this bypasses the client entirely +/// +/// `imap-proto` 0.16.7 cannot parse `EMAILID`: its `msg_att` parser is a closed +/// `alt(...)` of known attributes with no catch-all, so an `EMAILID` item is a +/// hard nom error rather than an unknown-but-tolerated one. `async-imap`'s +/// decoder turns any non-`Incomplete` parse error into a fatal `io::Error` and +/// then latches `read_closed`, after which the stream yields `None` forever -- +/// the session is dead, not degraded. So `EMAILID` must never reach +/// `parse_response`, which rules out `Session::uid_fetch` **and** +/// [`run_collected`], since the latter reads through `Session::read_response` +/// and therefore through the same decoder. +/// +/// What is left is the transport underneath: `Session` exposes `&mut T` +/// (`AsMut`, and `Connection::get_mut` through `Deref`), so the command is +/// written and its reply scanned here, by hand, and no byte of it is offered +/// to nom. +/// +/// # The two conditions this is only correct under +/// +/// 1. **The client's read buffer must be empty.** `ImapStream` owns a buffer +/// that can hold bytes already pulled off the socket. Raw reads bypass it, +/// so anything sitting there is invisible to this scanner. Call this only +/// at a quiescent point -- immediately after a previous command's *tagged* +/// completion has been read -- and never interleaved with an in-flight +/// typed command or a live response stream. +/// 2. **The tag must not collide** with the client's own. See +/// [`RAW_TAG_PREFIX`]. +/// +/// # Degrading rather than failing +/// +/// A tagged `NO`/`BAD` yields an empty map, not an error. The object id is an +/// identity *upgrade*: without it a message still syncs under the +/// `Message-ID`+content tier or the surrogate, so a server that advertises +/// `OBJECTID` and then refuses the fetch must cost the sync precision, never +/// the mail. A stalled read is different and does surface, because a server +/// that stopped answering mid-command has left the session unusable. +pub async fn uid_fetch_email_ids( + session: &mut Session, + uid_set: &str, +) -> Result, MailError> +where + S: AsyncRead + AsyncWrite + Unpin + Send + Debug + 'static, +{ + if uid_set.is_empty() { + return Ok(BTreeMap::new()); + } + + let tag = next_raw_tag(); + let command = format!("{tag} UID FETCH {uid_set} (UID EMAILID)\r\n"); + + let stream: &mut S = session.as_mut(); + stream + .write_all(command.as_bytes()) + .await + .map_err(|e| MailError::ImapError(format!("failed to issue '{command:?}': {e}")))?; + stream + .flush() + .await + .map_err(|e| MailError::ImapError(format!("failed to flush '{command:?}': {e}")))?; + + let (lines, tagged) = read_until_tagged(stream, &tag).await?; + + if !tagged_is_ok(&tagged) { + tracing::warn!( + response = %tagged, + "server refused UID FETCH (UID EMAILID) despite advertising OBJECTID; \ + this pass identifies its messages by a weaker tier" + ); + return Ok(BTreeMap::new()); + } + + let mut ids = BTreeMap::new(); + for line in &lines { + if let Some((uid, email_id)) = parse_email_id_line(line) { + ids.insert(uid, email_id); + } + } + Ok(ids) +} + +/// Read complete CRLF-terminated lines until one begins with `tag`, returning +/// the lines before it and the tagged line itself. +/// +/// Line-oriented scanning is sound *for this command only*: an `EMAILID` +/// response carries no literal (`{n}`) section, so no line body can contain an +/// embedded CRLF. It must not be reused for a command whose reply can carry +/// literals -- a body fetch -- without adding literal handling. +async fn read_until_tagged(stream: &mut S, tag: &str) -> Result<(Vec, String), MailError> +where + S: AsyncRead + AsyncWrite + Unpin, +{ + let mut buffer: Vec = Vec::new(); + let mut chunk = [0u8; 4096]; + let mut scanned = 0usize; + let mut lines: Vec = Vec::new(); + let tag_prefix = format!("{tag} "); + + loop { + let read = tokio::time::timeout(RESPONSE_READ_TIMEOUT, stream.read(&mut chunk)) + .await + .map_err(|_| { + MailError::FetchStalled(format!( + "no response to raw '{tag}' within {RESPONSE_READ_TIMEOUT:?}" + )) + })? + .map_err(|e| MailError::ImapError(format!("reading response to raw '{tag}': {e}")))?; + if read == 0 { + return Err(MailError::ImapError(format!( + "connection closed before raw '{tag}' completed" + ))); + } + buffer.extend_from_slice(&chunk[..read]); + if buffer.len() > MAX_RAW_RESPONSE_BYTES { + return Err(MailError::ImapError(format!( + "raw '{tag}' response exceeded {MAX_RAW_RESPONSE_BYTES} bytes without completing" + ))); + } + + // Consume every line that is now complete. `scanned` marks how far the + // buffer has already been split, so a reply arriving in many chunks is + // not re-scanned from the start each time. + while let Some(offset) = buffer[scanned..].iter().position(|b| *b == b'\n') { + let end = scanned + offset; + let line = String::from_utf8_lossy(&buffer[scanned..end]) + .trim_end_matches('\r') + .to_string(); + scanned = end + 1; + if line.starts_with(&tag_prefix) { + return Ok((lines, line)); + } + lines.push(line); + } + } +} + +/// Whether a tagged completion line reports `OK`. +fn tagged_is_ok(tagged: &str) -> bool { + tagged + .split_whitespace() + .nth(1) + .is_some_and(|status| status.eq_ignore_ascii_case("OK")) +} + +/// Pull `(uid, EMAILID)` out of one untagged `FETCH` line. +/// +/// Both wire spellings are accepted. RFC 8474 section 5.2 defines the response +/// as `"EMAILID" SP "(" objectid ")"`, but the bare `EMAILID ` form +/// is what a hand-rolled scanner meets often enough that rejecting it would +/// mean silently dropping to a weaker identity tier against a server that +/// answered the question correctly enough. +/// +/// Returns `None` for anything that is not an untagged `FETCH` carrying both a +/// UID and a non-empty object id -- there is nothing to key on without both. +fn parse_email_id_line(line: &str) -> Option<(u32, String)> { + let upper = line.to_ascii_uppercase(); + if !upper.starts_with("* ") || !upper.contains(" FETCH ") { + return None; + } + // `to_ascii_uppercase` is byte-length preserving, so offsets found in the + // uppercased copy index the original exactly. Item *names* are + // case-insensitive (RFC 3501 section 9); an object id is not, so the value + // is always taken from the original line. + let uid = find_item_value(&upper, line, "UID ")?.parse::().ok()?; + let email_id = find_item_value(&upper, line, "EMAILID ")?; + let email_id = email_id + .trim_start_matches('(') + .trim_end_matches(')') + .trim_matches('"') + .to_string(); + if email_id.is_empty() { + return None; + } + Some((uid, email_id)) +} + +/// The token following FETCH item `name` in `line`, located via its uppercased +/// twin `upper`. +/// +/// The name must sit on a token boundary, so `UID ` never matches the tail of +/// another attribute name. A parenthesised value is returned whole, brackets +/// included, for the caller to strip. +fn find_item_value(upper: &str, line: &str, name: &str) -> Option { + let mut from = 0usize; + loop { + let at = from + upper[from..].find(name)?; + let boundary = at == 0 + || upper[..at] + .chars() + .next_back() + .is_some_and(|c| c == '(' || c == ' '); + if boundary { + let value_start = at + name.len(); + let rest = &line[value_start..]; + let end = if rest.starts_with('(') { + rest.find(')').map_or(rest.len(), |i| i + 1) + } else { + rest.find([' ', ')']).unwrap_or(rest.len()) + }; + let value = rest[..end].trim(); + if !value.is_empty() { + return Some(value.to_string()); + } + } + from = at + name.len(); + } +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn an_object_id_is_read_from_both_wire_spellings() { + // RFC 8474 5.2 parenthesises the id; the bare form appears in the wild. + // Both must key the same message, or which spelling a server chose + // would decide whether identity converges. + assert_eq!( + parse_email_id_line("* 1 FETCH (UID 7 EMAILID (M00000001))"), + Some((7, "M00000001".to_string())) + ); + assert_eq!( + parse_email_id_line("* 1 FETCH (UID 7 EMAILID M00000001)"), + Some((7, "M00000001".to_string())) + ); + // Item names are case-insensitive; the id itself is not. + assert_eq!( + parse_email_id_line("* 1 fetch (uid 7 emailid (Mixed-Case_01))"), + Some((7, "Mixed-Case_01".to_string())) + ); + // Order is not fixed, and other attributes may sit between them. + assert_eq!( + parse_email_id_line("* 3 FETCH (EMAILID (M9) FLAGS (\\Seen) UID 12)"), + Some((12, "M9".to_string())) + ); + } + + #[test] + fn a_line_missing_either_half_yields_nothing() { + // Half an answer is not a weaker identity, it is no identity: without + // the UID there is nothing to attach the id to, and without the id + // there is nothing to attach. + assert_eq!( + parse_email_id_line("* 1 FETCH (UID 7 FLAGS (\\Seen))"), + None + ); + assert_eq!(parse_email_id_line("* 1 FETCH (EMAILID (M1))"), None); + assert_eq!(parse_email_id_line("* 1 FETCH (UID 7 EMAILID ())"), None); + assert_eq!(parse_email_id_line("NX0 OK UID FETCH completed"), None); + assert_eq!(parse_email_id_line("* 4 EXPUNGE"), None); + } + + #[test] + fn an_item_name_is_only_matched_on_a_token_boundary() { + // RFC 8474 defines THREADID alongside EMAILID, so a line carrying both + // is ordinary. Every occurrence of `ID ` in it is the tail of a longer + // attribute name; without the boundary rule a search would read + // THREADID's value as the item's own and key the message on it. + let line = "* 1 FETCH (THREADID (T1) EMAILID (M1) UID 7)"; + let upper = line.to_ascii_uppercase(); + assert_eq!(find_item_value(&upper, line, "ID "), None); + assert_eq!( + find_item_value(&upper, line, "EMAILID "), + Some("(M1)".to_string()) + ); + assert_eq!(find_item_value(&upper, line, "UID "), Some("7".to_string())); + assert_eq!(parse_email_id_line(line), Some((7, "M1".to_string()))); + } + + #[test] + fn only_a_tagged_ok_counts_as_a_usable_answer() { + assert!(tagged_is_ok("NX7 OK UID FETCH completed")); + assert!(tagged_is_ok("NX7 ok uid fetch completed")); + assert!(!tagged_is_ok("NX7 NO server does not do that")); + assert!(!tagged_is_ok("NX7 BAD unknown item")); + assert!(!tagged_is_ok("NX7")); + } + + #[test] + fn raw_tags_can_never_collide_with_the_clients_own() { + // async-imap's IdGenerator emits `A` + four digits and wraps at 10_000, + // so a shared tag is only impossible if the prefix differs. + let tag = next_raw_tag(); + assert!(tag.starts_with(RAW_TAG_PREFIX)); + assert_ne!(&tag[..1], "A"); + assert_ne!(next_raw_tag(), tag, "each raw command gets its own tag"); + } + #[test] fn modified_response_code_is_recovered_from_the_tagged_text() { // imap-proto has no ResponseCode::Modified, and its opt(resp_text_code) diff --git a/crates/nuncio-mail/src/test_server.rs b/crates/nuncio-mail/src/test_server.rs index 9eaffea..357517d 100644 --- a/crates/nuncio-mail/src/test_server.rs +++ b/crates/nuncio-mail/src/test_server.rs @@ -48,21 +48,6 @@ pub enum ServerProfile { } impl ServerProfile { - /// The `CAPABILITY` atom list for this profile. `UIDPLUS` and `MOVE` are - /// advertised by all three because every profile we model supports them. - fn capability_line(self) -> String { - let mut caps = vec!["IMAP4rev1", "UIDPLUS", "MOVE", "ENABLE", "LIST-STATUS"]; - match self { - ServerProfile::Basic => {} - ServerProfile::Condstore => caps.push("CONDSTORE"), - ServerProfile::Qresync => { - caps.push("CONDSTORE"); - caps.push("QRESYNC"); - } - } - caps.join(" ") - } - fn supports_condstore(self) -> bool { matches!(self, ServerProfile::Condstore | ServerProfile::Qresync) } @@ -72,6 +57,57 @@ impl ServerProfile { } } +/// Which server-assigned message-id extensions this server publishes. +/// +/// Independent of [`ServerProfile`] because the two axes genuinely are +/// independent in the field: Gmail advertises `X-GM-EXT-1` with CONDSTORE and +/// no QRESYNC, Fastmail advertises `OBJECTID` with QRESYNC, and Exchange +/// advertises neither. Selectable per instance so a test can pin the +/// behaviour of a server that offers *nothing*, which is the case that must +/// stay byte-identical to how the engine behaved before object ids existed. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct IdentityExtensions { + /// RFC 8474 `OBJECTID`, so `EMAILID` is served when asked for. + pub objectid: bool, + /// Gmail's `X-GM-EXT-1`, so `X-GM-MSGID` is served when asked for. + pub gmail: bool, +} + +impl IdentityExtensions { + /// A server that publishes no stable message id at all. + pub fn none() -> Self { + Self::default() + } + + /// RFC 8474 only. + pub fn objectid() -> Self { + Self { + objectid: true, + gmail: false, + } + } + + /// Gmail's vendor extension only, which is what Gmail itself does. + pub fn gmail() -> Self { + Self { + objectid: false, + gmail: true, + } + } + + /// The `CAPABILITY` atoms these extensions contribute. + fn atoms(self) -> Vec<&'static str> { + let mut atoms = Vec::new(); + if self.objectid { + atoms.push("OBJECTID"); + } + if self.gmail { + atoms.push("X-GM-EXT-1"); + } + atoms + } +} + /// One message as the server holds it. #[derive(Debug, Clone)] pub struct MockMessage { @@ -80,6 +116,23 @@ pub struct MockMessage { pub flags: BTreeSet, /// Full RFC822 octets served for `BODY[]`. pub body: String, + /// The server-assigned ids this message keeps wherever it sits. + pub object_ids: ObjectIds, +} + +/// The message-level ids a server assigns once and reports identically from +/// every mailbox holding the message. +/// +/// That invariant is the entire point of RFC 8474: a UID is per-mailbox, an +/// `EMAILID` is not, so a copy in a second folder reports the same one. A mock +/// that minted a fresh id per placement would model a server that does not +/// exist and would let a broken client pass. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ObjectIds { + /// RFC 8474 `EMAILID`. + pub email_id: String, + /// Gmail `X-GM-MSGID`, a 64-bit integer on the wire. + pub gm_msgid: u64, } /// A mailbox: its UID space, its mod-sequence, and the tombstones needed to @@ -113,7 +166,7 @@ impl MockMailbox { self.highest_modseq } - fn append(&mut self, body: &str, flags: &[&str]) -> u32 { + fn append(&mut self, body: &str, flags: &[&str], object_ids: ObjectIds) -> u32 { let uid = self.uid_next; self.uid_next += 1; let modseq = self.next_modseq(); @@ -122,6 +175,7 @@ impl MockMailbox { modseq, flags: flags.iter().map(|f| (*f).to_string()).collect(), body: body.to_string(), + object_ids, }); uid } @@ -150,21 +204,57 @@ impl MockMailbox { #[derive(Debug)] pub struct ServerState { pub profile: ServerProfile, + /// Which server-assigned id extensions this instance publishes. + pub extensions: IdentityExtensions, pub mailboxes: BTreeMap, /// Every command line received, across all connections, in arrival order. /// Tests assert on this to prove *which* protocol path ran -- e.g. that a /// second sync narrowed its FETCH rather than re-fetching everything. pub commands: Vec, next_uid_validity: u32, + next_object_id: u64, } impl ServerState { - fn new(profile: ServerProfile) -> Self { + fn new(profile: ServerProfile, extensions: IdentityExtensions) -> Self { Self { profile, + extensions, mailboxes: BTreeMap::new(), commands: Vec::new(), next_uid_validity: 1000, + next_object_id: 1, + } + } + + /// The `CAPABILITY` atom list. `UIDPLUS` and `MOVE` are advertised by every + /// profile because every server we model supports them; the sync-ladder and + /// identity atoms are what varies. + fn capability_line(&self) -> String { + let mut caps = vec!["IMAP4rev1", "UIDPLUS", "MOVE", "ENABLE", "LIST-STATUS"]; + match self.profile { + ServerProfile::Basic => {} + ServerProfile::Condstore => caps.push("CONDSTORE"), + ServerProfile::Qresync => { + caps.push("CONDSTORE"); + caps.push("QRESYNC"); + } + } + caps.extend(self.extensions.atoms()); + caps.join(" ") + } + + /// Mint the ids for a message the test did not name explicitly. + fn mint_object_ids(&mut self) -> ObjectIds { + let n = self.next_object_id; + self.next_object_id += 1; + ObjectIds { + // RFC 8474 section 4 restricts an objectid to 1*255 of + // ALPHA/DIGIT/`_`/`-`, so this shape is representative rather than + // decorative -- a scanner that mishandles it mishandles the real + // thing. + email_id: format!("M{n:08}"), + gm_msgid: 1_278_455_344_230_334_000 + n, } } } @@ -195,11 +285,20 @@ fn lock_state(state: &Arc>) -> MutexGuard<'_, ServerState> { } impl MockImapServer { - /// Bind an ephemeral loopback port and start accepting connections. + /// Bind an ephemeral loopback port and start accepting connections, with + /// no server-assigned message-id extensions. pub async fn start(profile: ServerProfile) -> std::io::Result { + Self::start_with_identity(profile, IdentityExtensions::none()).await + } + + /// As [`Self::start`], additionally publishing `extensions`. + pub async fn start_with_identity( + profile: ServerProfile, + extensions: IdentityExtensions, + ) -> std::io::Result { let listener = TcpListener::bind("127.0.0.1:0").await?; let addr = listener.local_addr()?; - let state = Arc::new(Mutex::new(ServerState::new(profile))); + let state = Arc::new(Mutex::new(ServerState::new(profile, extensions))); let accept_state = Arc::clone(&state); let accept_task = tokio::spawn(async move { @@ -241,8 +340,26 @@ impl MockImapServer { .insert(name.to_string(), MockMailbox::new(uid_validity)); } - /// Append a message, creating the mailbox if absent. Returns its UID. + /// Append a message under freshly minted object ids, creating the mailbox + /// if absent. Returns its UID. pub fn append_message(&self, mailbox: &str, body: &str, flags: &[&str]) -> u32 { + let ids = lock_state(&self.state).mint_object_ids(); + self.append_message_with_object_ids(mailbox, body, flags, ids) + } + + /// Append a message under *given* object ids. + /// + /// This is how a test puts the same message in two mailboxes: a real + /// OBJECTID server reports one `EMAILID` for both copies, and only that + /// makes the cross-folder identity claim testable. Passing distinct ids to + /// two appends models two genuinely different messages. + pub fn append_message_with_object_ids( + &self, + mailbox: &str, + body: &str, + flags: &[&str], + object_ids: ObjectIds, + ) -> u32 { let mut state = lock_state(&self.state); let uid_validity = state.next_uid_validity; let entry = state.mailboxes.entry(mailbox.to_string()); @@ -251,13 +368,24 @@ impl MockImapServer { MockMailbox::new(uid_validity) }); let fresh = mailbox.uid_next == 1 && mailbox.messages.is_empty(); - let uid = mailbox.append(body, flags); + let uid = mailbox.append(body, flags, object_ids); if fresh { state.next_uid_validity += 1; } uid } + /// The object ids the server assigned to `uid` in `mailbox`. + pub fn object_ids(&self, mailbox: &str, uid: u32) -> Option { + let state = lock_state(&self.state); + state.mailboxes.get(mailbox).and_then(|mb| { + mb.messages + .iter() + .find(|m| m.uid == uid) + .map(|m| m.object_ids.clone()) + }) + } + /// Simulate another client removing a message -- the case a forward-only /// high-water-mark sync can never observe. pub fn expunge(&self, mailbox: &str, uid: u32) -> bool { @@ -331,7 +459,7 @@ async fn handle_connection( let st = lock_state(&state); format!( "* OK [CAPABILITY {}] Nuncio mock IMAP ready\r\n", - st.profile.capability_line() + st.capability_line() ) }; socket.write_all(greeting.as_bytes()).await?; @@ -381,11 +509,11 @@ fn dispatch(line: &str, session: &mut Session, st: &mut ServerState) -> Reply { match command.as_str() { "CAPABILITY" => Reply::open(format!( "* CAPABILITY {}\r\n{tag} OK CAPABILITY completed\r\n", - st.profile.capability_line() + st.capability_line() )), "LOGIN" => Reply::open(format!( "{tag} OK [CAPABILITY {}] Logged in\r\n", - st.profile.capability_line() + st.capability_line() )), "ENABLE" => handle_enable(&tag, &rest, session, st), "NOOP" => Reply::open(format!("{tag} OK NOOP completed\r\n")), @@ -499,7 +627,7 @@ fn handle_select( out.push_str(&format!("* VANISHED (EARLIER) {}\r\n", gone.join(","))); } for msg in mb.messages.iter().filter(|m| m.modseq > since) { - out.push_str(&fetch_line(mb, msg, false)); + out.push_str(&fetch_line(mb, msg, FetchItems::none())); } } } @@ -576,7 +704,39 @@ fn format_flags(flags: &BTreeSet) -> String { flags.iter().cloned().collect::>().join(" ") } -fn fetch_line(mb: &MockMailbox, msg: &MockMessage, include_body: bool) -> String { +/// Which optional items a `FETCH` reply should carry. +/// +/// A server returns what was asked for. That is not pedantry here: `EMAILID` +/// is unparseable by the client's own parser, so serving it unasked would kill +/// every connection that ran a typed fetch -- exactly the failure the raw +/// layer exists to route around. +#[derive(Debug, Clone, Copy, Default)] +struct FetchItems { + body: bool, + email_id: bool, + gm_msgid: bool, +} + +impl FetchItems { + /// Read the request off the command line, honouring what this server + /// actually advertised: an item the client asked for without the + /// capability is simply not served. + fn requested(upper: &str, extensions: IdentityExtensions) -> Self { + Self { + body: upper.contains("BODY[]") || upper.contains("RFC822"), + email_id: extensions.objectid && upper.contains("EMAILID"), + gm_msgid: extensions.gmail && upper.contains("X-GM-MSGID"), + } + } + + /// The shape a non-FETCH path (a QRESYNC `SELECT`, a non-silent `STORE`) + /// volunteers: flags and mod-sequence only. + fn none() -> Self { + Self::default() + } +} + +fn fetch_line(mb: &MockMailbox, msg: &MockMessage, items_wanted: FetchItems) -> String { let seq = mb.seq_of(msg.uid); let mut items = format!( "UID {} FLAGS ({}) MODSEQ ({})", @@ -584,7 +744,14 @@ fn fetch_line(mb: &MockMailbox, msg: &MockMessage, include_body: bool) -> String format_flags(&msg.flags), msg.modseq ); - if include_body { + if items_wanted.email_id { + // RFC 8474 section 5.2 parenthesises the value. + items.push_str(&format!(" EMAILID ({})", msg.object_ids.email_id)); + } + if items_wanted.gm_msgid { + items.push_str(&format!(" X-GM-MSGID {}", msg.object_ids.gm_msgid)); + } + if items_wanted.body { items.push_str(&format!(" BODY[] {{{}}}\r\n{}", msg.body.len(), msg.body)); } format!("* {seq} FETCH ({items})\r\n") @@ -598,6 +765,7 @@ fn handle_uid_fetch( st: &mut ServerState, ) -> Reply { let condstore = st.profile.supports_condstore(); + let extensions = st.extensions; let Some(mb) = st.mailboxes.get(selected) else { return Reply::open(format!("{tag} NO mailbox vanished\r\n")); }; @@ -606,7 +774,14 @@ fn handle_uid_fetch( }; let upper = line.to_ascii_uppercase(); - let include_body = upper.contains("BODY[]") || upper.contains("RFC822"); + // An unrecognised FETCH item is a syntax error (RFC 3501 section 6.4.5), so + // a server that never advertised OBJECTID rejects the whole command rather + // than quietly answering without the item. Modelling that is what makes the + // client's capability gate load-bearing instead of decorative. + if upper.contains("EMAILID") && !extensions.objectid { + return Reply::open(format!("{tag} BAD unknown FETCH item EMAILID\r\n")); + } + let items_wanted = FetchItems::requested(&upper, extensions); let changed_since = if condstore { parse_changed_since(&upper) } else { @@ -623,7 +798,7 @@ fn handle_uid_fetch( continue; } } - out.push_str(&fetch_line(mb, msg, include_body)); + out.push_str(&fetch_line(mb, msg, items_wanted)); } // RFC 7162 3.2.6: with CHANGEDSINCE ... VANISHED the server reports @@ -724,7 +899,7 @@ fn handle_uid_store( if !silent { for uid in &touched { if let Some(msg) = mb.messages.iter().find(|m| m.uid == *uid) { - out.push_str(&fetch_line(mb, msg, false)); + out.push_str(&fetch_line(mb, msg, FetchItems::none())); } } } @@ -784,11 +959,14 @@ fn handle_uid_copy( }; let present: Vec = mb.messages.iter().map(|m| m.uid).collect(); let uids = expand_uid_set(spec, &present, mb.uid_next); - let bodies: Vec<(u32, String, BTreeSet)> = mb + // The copies keep the source's object ids. RFC 8474 section 4 makes + // EMAILID a property of the message, so a server that minted a new one + // per mailbox would be reporting two messages where there is one. + let bodies: Vec<(String, BTreeSet, ObjectIds)> = mb .messages .iter() .filter(|m| uids.contains(&m.uid)) - .map(|m| (m.uid, m.body.clone(), m.flags.clone())) + .map(|m| (m.body.clone(), m.flags.clone(), m.object_ids.clone())) .collect(); (uids, mb.uid_validity, bodies) }; @@ -806,9 +984,9 @@ fn handle_uid_copy( let Some(dst) = st.mailboxes.get_mut(&dest) else { return Reply::open(format!("{tag} NO destination vanished\r\n")); }; - for (_, body, flags) in &bodies { + for (body, flags, object_ids) in &bodies { let refs: Vec<&str> = flags.iter().map(String::as_str).collect(); - dest_uids.push(dst.append(body, &refs)); + dest_uids.push(dst.append(body, &refs, object_ids.clone())); } dst.uid_validity }; diff --git a/crates/nuncio-mail/tests/imap_identity_test.rs b/crates/nuncio-mail/tests/imap_identity_test.rs new file mode 100644 index 0000000..06a2f5a --- /dev/null +++ b/crates/nuncio-mail/tests/imap_identity_test.rs @@ -0,0 +1,231 @@ +//! Offline tests for server-assigned message identity over IMAP. +//! +//! The claim under test is the one ADR 0002 rests on: a message has one +//! account-scoped key, and which protocol observed it does not change that key. +//! RFC 8474 exists precisely so a server implementing both IMAP and JMAP +//! reports the same object id through both, so the last test here compares an +//! IMAP-derived key against a JMAP-derived one and requires them to be equal. +//! +//! Everything runs against [`MockImapServer`] over loopback and against a JMAP +//! response parsed from a literal; no test here reaches a network. + +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use nuncio_core::model::IdentitySource; +use nuncio_mail::backend::FolderChanges; +use nuncio_mail::test_server::{IdentityExtensions, MockImapServer, ObjectIds, ServerProfile}; +use nuncio_mail::{JmapEngine, MailBackend}; + +/// Sync one folder through the real engine against the mock. +async fn sync(server: &MockImapServer, account_id: &str, folder: &str) -> FolderChanges { + let engine = nuncio_mail::ImapEngine::with_credentials( + account_id, + server.host(), + server.port(), + nuncio_core::TlsMode::Plain, + "user", + "pass", + ); + engine + .sync_changes(folder, None) + .await + .expect("folder sync succeeds") +} + +#[tokio::test] +async fn an_objectid_server_keys_messages_by_their_emailid() { + // Tier 1. Reaching it at all is the difference between an identity the + // server vouches for and one derived from whatever the message happened to + // contain. + let server = + MockImapServer::start_with_identity(ServerProfile::Qresync, IdentityExtensions::objectid()) + .await + .expect("server starts"); + server.create_mailbox("INBOX"); + let uid = server.append_message("INBOX", "Subject: one\r\n\r\nbody", &[]); + + let changes = sync(&server, "acct-objectid", "INBOX").await; + + assert_eq!(changes.upserts.len(), 1); + assert_eq!( + changes.upserts[0].source, + IdentitySource::EmailId, + "an OBJECTID server must be asked for EMAILID and the answer used" + ); + assert_eq!(changes.upserts[0].placement.remote_id, uid.to_string()); + assert!( + server + .commands_matching("EMAILID") + .iter() + .any(|c| c.to_ascii_uppercase().contains("UID FETCH")), + "the object id must come from a real UID FETCH, got {:?}", + server.commands() + ); +} + +#[tokio::test] +async fn the_same_message_in_a_second_folder_keys_identically() { + // The property a folder-scoped surrogate can never have, and the reason + // tier 1 is worth a separate round trip: two placements, one message. + let server = + MockImapServer::start_with_identity(ServerProfile::Qresync, IdentityExtensions::objectid()) + .await + .expect("server starts"); + server.create_mailbox("INBOX"); + server.create_mailbox("Archive"); + + let shared = ObjectIds { + email_id: "M00000042".to_string(), + gm_msgid: 1_278_455_344_230_334_865, + }; + let body = "Subject: filed\r\n\r\nbody"; + server.append_message_with_object_ids("INBOX", body, &[], shared.clone()); + server.append_message_with_object_ids("Archive", body, &[], shared); + + let inbox = sync(&server, "acct-shared", "INBOX").await; + let archive = sync(&server, "acct-shared", "Archive").await; + + assert_eq!(inbox.upserts.len(), 1); + assert_eq!(archive.upserts.len(), 1); + assert_eq!( + inbox.upserts[0].email.id, archive.upserts[0].email.id, + "one message in two folders must be one identity" + ); + assert_eq!(inbox.upserts[0].source, IdentitySource::EmailId); + assert_eq!(archive.upserts[0].source, IdentitySource::EmailId); + assert_ne!( + inbox.upserts[0].placement, archive.upserts[0].placement, + "the two occupancies are still distinct; only the message is shared" + ); +} + +#[tokio::test] +async fn a_gmail_shaped_server_keys_messages_by_x_gm_msgid() { + // Gmail advertises X-GM-EXT-1 and not OBJECTID, so tier 2 is the best it + // offers -- and it is reachable through the typed client, with no extra + // round trip, because `imap-proto` can parse that attribute. + let server = + MockImapServer::start_with_identity(ServerProfile::Condstore, IdentityExtensions::gmail()) + .await + .expect("server starts"); + server.create_mailbox("INBOX"); + server.append_message("INBOX", "Subject: gmail\r\n\r\nbody", &[]); + + let changes = sync(&server, "acct-gmail", "INBOX").await; + + assert_eq!(changes.upserts.len(), 1); + assert_eq!( + changes.upserts[0].source, + IdentitySource::GmailMsgId, + "X-GM-EXT-1 must lift identity to the Gmail message id" + ); + assert!( + server.commands_matching("EMAILID").is_empty(), + "a server without OBJECTID must never be asked for EMAILID, got {:?}", + server.commands() + ); +} + +#[tokio::test] +async fn a_server_advertising_neither_extension_is_left_exactly_as_it_was() { + // The regression that matters most: capability negotiation must be + // invisible to a server that has neither extension. No new command, and + // the same tier it reached before any of this existed. + for profile in [ + ServerProfile::Qresync, + ServerProfile::Condstore, + ServerProfile::Basic, + ] { + let server = MockImapServer::start(profile).await.expect("server starts"); + server.create_mailbox("INBOX"); + server.append_message("INBOX", "Subject: plain\r\n\r\nbody", &[]); + + let changes = sync(&server, "acct-plain", "INBOX").await; + + assert_eq!(changes.upserts.len(), 1, "{profile:?}"); + assert_eq!( + changes.upserts[0].source, + IdentitySource::Surrogate, + "{profile:?}: with no server id and no octets, the surrogate is the honest answer" + ); + assert!( + server.commands_matching("EMAILID").is_empty(), + "{profile:?}: no EMAILID command may be issued, got {:?}", + server.commands() + ); + assert!( + server.commands_matching("X-GM-MSGID").is_empty(), + "{profile:?}: no X-GM-MSGID item may be requested, got {:?}", + server.commands() + ); + } +} + +#[tokio::test] +async fn an_imap_emailid_and_a_jmap_object_id_produce_the_same_key() { + // The convergence assertion. One account, one message, one server-assigned + // id -- reported as RFC 8474 EMAILID over IMAP and as the Email object id + // over JMAP. If either engine derived its key from a different tier, or + // hashed under a different domain label, these two strings would differ. + const ACCOUNT: &str = "acct-converge"; + const OBJECT_ID: &str = "M00000001"; + + let server = + MockImapServer::start_with_identity(ServerProfile::Qresync, IdentityExtensions::objectid()) + .await + .expect("server starts"); + server.create_mailbox("INBOX"); + server.append_message_with_object_ids( + "INBOX", + "Subject: both\r\n\r\nbody", + &[], + ObjectIds { + email_id: OBJECT_ID.to_string(), + gm_msgid: 1, + }, + ); + let imap = sync(&server, ACCOUNT, "INBOX").await; + + let jmap_response = format!( + r#"{{"methodResponses":[["Email/get",{{"accountId":"{ACCOUNT}", + "state":"s-1","list":[{{"id":"{OBJECT_ID}","subject":"both", + "from":[{{"email":"a@nuncio.mx"}}],"to":[{{"email":"b@nuncio.mx"}}], + "receivedAt":1700001000,"isUnread":true}}]}},"c1"]]}}"# + ); + let (jmap, _state) = JmapEngine::new(ACCOUNT) + .parse_email_get_response(&jmap_response) + .expect("the JMAP response parses"); + + assert_eq!(imap.upserts.len(), 1); + assert_eq!(jmap.len(), 1); + assert_eq!( + imap.upserts[0].source, + IdentitySource::EmailId, + "the IMAP side must have entered at the object-id tier" + ); + assert_eq!( + jmap[0].source, + IdentitySource::EmailId, + "the JMAP side must have entered at the object-id tier" + ); + assert_eq!( + imap.upserts[0].email.id, jmap[0].email.id, + "two engines syncing one account must agree on what the message is" + ); + + // Equality alone would survive both engines moving to a different tier or + // a different domain label together, since they share one derivation. The + // golden pins the actual value: SHA-256 over the length-prefixed + // (tier, account, object id) triple. It is a tripwire, not a spec -- if it + // moves, every stored message_key has moved with it, and + // `IDENTITY_SCHEMA_VERSION` must move in the same commit. + const EXPECTED_KEY: &str = "ba2eb228c6b973c548837e32b84e95cf06b73664b3e350f5dfa30614b2e38bd0"; + assert_eq!( + imap.upserts[0].email.id, EXPECTED_KEY, + "the object-id tier's derivation changed; bump IDENTITY_SCHEMA_VERSION with it" + ); + + // And the two engines still describe *where* it sits in their own terms: + // convergence is a claim about identity, never about placement. + assert_ne!(imap.upserts[0].placement, jmap[0].placement); +} diff --git a/crates/nuncio-mail/tests/imap_raw_wire_test.rs b/crates/nuncio-mail/tests/imap_raw_wire_test.rs index 97ed5f2..8638380 100644 --- a/crates/nuncio-mail/tests/imap_raw_wire_test.rs +++ b/crates/nuncio-mail/tests/imap_raw_wire_test.rs @@ -13,7 +13,7 @@ use nuncio_core::model::PlacementKey; use nuncio_mail::imap_raw; -use nuncio_mail::test_server::{MockImapServer, ServerProfile}; +use nuncio_mail::test_server::{IdentityExtensions, MockImapServer, ObjectIds, ServerProfile}; /// Connect the real client to the mock server and authenticate. async fn login( @@ -301,6 +301,89 @@ async fn a_qresync_select_without_enable_is_refused() { ); } +#[tokio::test] +async fn object_ids_are_read_off_the_transport_and_the_session_survives_it() { + // The whole reason this bypasses the client: `imap-proto` has no parser arm + // for EMAILID, and `async-imap` turns a parse error into a fatal I/O error + // that latches the stream closed. So the test that matters is not only that + // the ids come back, but that the *typed* client still works afterwards -- + // proving the raw exchange left the connection and the client's read buffer + // in a state the next command can use. + let server = + MockImapServer::start_with_identity(ServerProfile::Qresync, IdentityExtensions::objectid()) + .await + .expect("server starts"); + server.create_mailbox("INBOX"); + let first = server.append_message_with_object_ids( + "INBOX", + "Subject: one\r\n\r\nx", + &[], + ObjectIds { + email_id: "M-first".to_string(), + gm_msgid: 11, + }, + ); + let second = server.append_message_with_object_ids( + "INBOX", + "Subject: two\r\n\r\nx", + &[], + ObjectIds { + email_id: "M-second".to_string(), + gm_msgid: 12, + }, + ); + + let mut session = login(&server).await; + imap_raw::run_collected(&mut session, "SELECT INBOX") + .await + .expect("select"); + + let ids = imap_raw::uid_fetch_email_ids(&mut session, &format!("{first},{second}")) + .await + .expect("the raw EMAILID fetch succeeds"); + + assert_eq!(ids.get(&first).map(String::as_str), Some("M-first")); + assert_eq!(ids.get(&second).map(String::as_str), Some("M-second")); + + // The session is still alive: a typed command after the raw exchange must + // still see the mailbox. + let after = imap_raw::run_collected(&mut session, "UID FETCH 1:* (UID)") + .await + .expect("the session survives the raw exchange"); + assert_eq!(after.fetched_uids, vec![first, second]); +} + +#[tokio::test] +async fn a_refused_object_id_fetch_costs_precision_not_the_sync() { + // A server that rejects the item leaves the message identifiable by a + // weaker tier. Erroring here would fail an entire folder sync over an + // identity *upgrade*, which is the wrong trade in both directions. + let server = MockImapServer::start(ServerProfile::Qresync) + .await + .expect("server starts"); + server.create_mailbox("INBOX"); + let uid = server.append_message("INBOX", "Subject: one\r\n\r\nx", &[]); + + let mut session = login(&server).await; + imap_raw::run_collected(&mut session, "SELECT INBOX") + .await + .expect("select"); + + let ids = imap_raw::uid_fetch_email_ids(&mut session, &uid.to_string()) + .await + .expect("a BAD answer is not a transport failure"); + assert!( + ids.is_empty(), + "nothing was learned, and that is the answer" + ); + + // And the connection is still usable, which a `BAD` must not change. + let after = imap_raw::run_collected(&mut session, "UID FETCH 1:* (UID)") + .await + .expect("the session survives a refusal"); + assert_eq!(after.fetched_uids, vec![uid]); +} + /// Drive a real folder sync against the mock and return what it observed. async fn sync_folder( server: &MockImapServer, diff --git a/crates/nuncio-store/src/db.rs b/crates/nuncio-store/src/db.rs index 24ced58..6d8c5b0 100644 --- a/crates/nuncio-store/src/db.rs +++ b/crates/nuncio-store/src/db.rs @@ -628,7 +628,17 @@ impl DatabaseEngine { /// /// Bump this only for a change of that kind, and only together with the /// reset it implies (see [`DatabaseEngine::reconcile_schema_version`]). - pub const IDENTITY_SCHEMA_VERSION: i64 = 2; + /// + /// Generation 3 is where the IMAP engine began asking for RFC 8474 + /// `EMAILID` and `X-GM-MSGID`. Every message a capable server holds now + /// derives its key from a stronger identity tier, so no key stored under + /// generation 2 still names the message it named. Letting that happen a row + /// at a time -- as each resync repointed a placement -- would work, but the + /// repoint reap is a safety net against stranded plaintext, not a migration + /// mechanism, and it would leave the store in two identity models for as + /// long as any folder went unsynced. Rebuilding once is the honest form of + /// the same change. + pub const IDENTITY_SCHEMA_VERSION: i64 = 3; /// Reconcile the on-disk identity-schema generation with this binary's. /// diff --git a/docs/adr/0002-multi-engine-sync-model.md b/docs/adr/0002-multi-engine-sync-model.md index 2e45fa1..a874edd 100644 --- a/docs/adr/0002-multi-engine-sync-model.md +++ b/docs/adr/0002-multi-engine-sync-model.md @@ -175,6 +175,24 @@ sound without a sequence number and resumption. Precedence: `EMAILID` (RFC 8474 OBJECTID) → `X-GM-MSGID` → `(account_id, normalized Message-ID, content_hash)` → `(account, folder, uidvalidity, uid)`. +**Both engines negotiate the top two tiers, so keys converge exactly where the +server publishes an object id.** The IMAP engine requests `EMAILID` from a +server advertising `OBJECTID` and `X-GM-MSGID` from one advertising +`X-GM-EXT-1`; JMAP's Email object id is the same identifier RFC 8474 exposes to +IMAP. One message synced over both protocols therefore lands on one key, which +is what makes the multi-engine model's identity claim true across protocols and +not only within one. + +Be precise about where it stops. Against a server advertising neither +extension, IMAP reaches only the Message-ID+content tier or the folder-scoped +surrogate, while a JMAP engine on the same account still enters at the top tier +from a server-assigned id — those key spaces are disjoint and cross-engine +comparison means nothing there. Object ids are unique within one account on one +server (RFC 8474 §4), never globally, which is why every tier is +account-scoped. And `EMAILID` cannot be requested through the typed IMAP client +at all: `imap-proto` has no parser arm for the attribute and a parse failure is +fatal to the connection, so it is read through the raw command layer instead. + Bodies are keyed by a content hash of the full RFC822 octets, first-write-wins. **A Message-ID hash alone must not be the key**: Message-IDs are public, echoed in every `References:` header, and forgeable, so with `INSERT OR REPLACE` a @@ -238,7 +256,11 @@ work is chunked per folder so it yields to user actions. - **`async-imap` 0.11.3 cannot express any of this**: no `ENABLE`, its `SELECT` parser drops `VANISHED`, and `check_status_ok` discards the tagged response code where `COPYUID` and `[MODIFIED …]` live. A fork-versus-hand-rolled - decision gates the protocol work. + decision gates the protocol work. `EMAILID` is worse than discarded: + `imap-proto`'s `msg_att` is a closed alternation with no catch-all, and the + decoder turns the resulting parse error into a fatal I/O error that latches + the connection closed — so the object id must be read off the transport + underneath the client, never through it. ## Alternatives considered