fix a condstore bug

src/imap/command/selected.rs

@@ -222,17 +222,25 @@ impl<'a> SelectedContext<'a> {
             .mailbox
             .store(sequence_set, kind, response, flags, unchanged_since, uid)
             .await?;
-        let modified_str = format!("MODIFIED {}", modified.into_iter().map(|x| x.to_string()).collect::<Vec<_>>().join(","));
+
+        let mut ok_resp = Response::build()
+                .to_req(self.req)
+                .message("STORE completed")
+                .set_body(data);
+
+
+        match modified[..] {
+            [] => (),
+            [_head, ..] => {
+                let modified_str = format!("MODIFIED {}", modified.into_iter().map(|x| x.to_string()).collect::<Vec<_>>().join(","));
+                ok_resp = ok_resp.code(Code::Other(CodeOther::unvalidated(modified_str.into_bytes())));
+            },
+        };
+
 
         self.client_capabilities.store_modifiers_enable(modifiers);
 
-        Ok((
+        Ok((ok_resp.ok()?,
-            Response::build()
-                .to_req(self.req)
-                .message("STORE completed")
-                .code(Code::Other(CodeOther::unvalidated(modified_str.into_bytes())))
-                .set_body(data)
-                .ok()?,
             flow::Transition::None,
         ))
     }

tests/behavior.rs

@@ -9,10 +9,12 @@ fn main() {
     rfc5161_imapext_enable();
     rfc6851_imapext_move();
     rfc7888_imapext_literal();
+    rfc4551_imapext_condstore();
+    println!("✅ SUCCESS 🌟🚀🥳🙏🥹");
 }
 
 fn rfc3501_imap4rev1_base() {
-    println!("rfc3501_imap4rev1_base");
+    println!("🧪 rfc3501_imap4rev1_base");
     common::aerogramme_provider_daemon_dev(|imap_socket, lmtp_socket| {
         connect(imap_socket).context("server says hello")?;
         capability(imap_socket, Extension::None).context("check server capabilities")?;
@@ -20,18 +22,19 @@ fn rfc3501_imap4rev1_base() {
         create_mailbox(imap_socket, Mailbox::Archive).context("created mailbox archive")?;
         // UNSUBSCRIBE IS NOT IMPLEMENTED YET
         //unsubscribe_mailbox(imap_socket).context("unsubscribe from archive")?;
-        select(imap_socket, Mailbox::Inbox, None).context("select inbox")?;
+        select(imap_socket, Mailbox::Inbox, SelectMod::None, None).context("select inbox")?;
         check(imap_socket).context("check must run")?;
         status_mailbox(imap_socket, Mailbox::Archive).context("status of archive from inbox")?;
         lmtp_handshake(lmtp_socket).context("handshake lmtp done")?;
         lmtp_deliver_email(lmtp_socket, Email::Multipart).context("mail delivered successfully")?;
-        noop_exists(imap_socket).context("noop loop must detect a new email")?;
+        noop_exists(imap_socket, 1).context("noop loop must detect a new email")?;
         fetch_rfc822(imap_socket, Selection::FirstId, Email::Multipart)
             .context("fetch rfc822 message, should be our first message")?;
         copy(imap_socket, Selection::FirstId, Mailbox::Archive)
             .context("copy message to the archive mailbox")?;
         append_email(imap_socket, Email::Basic).context("insert email in INBOX")?;
-        // SEARCH IS NOT IMPLEMENTED YET
+        noop_exists(imap_socket, 2).context("noop loop must detect a new email")?;
+        // SEARCH IS NOT TESTED YET
         //search(imap_socket).expect("search should return something");
         add_flags_email(imap_socket, Selection::FirstId, Flag::Deleted)
             .context("should add delete flag to the email")?;
@@ -45,7 +48,7 @@ fn rfc3501_imap4rev1_base() {
 }
 
 fn rfc3691_imapext_unselect() {
-    println!("rfc3691_imapext_unselect");
+    println!("🧪 rfc3691_imapext_unselect");
     common::aerogramme_provider_daemon_dev(|imap_socket, lmtp_socket| {
         connect(imap_socket).context("server says hello")?;
 
@@ -54,17 +57,17 @@ fn rfc3691_imapext_unselect() {
 
         capability(imap_socket, Extension::Unselect).context("check server capabilities")?;
         login(imap_socket, Account::Alice).context("login test")?;
-        select(imap_socket, Mailbox::Inbox, None).context("select inbox")?;
+        select(imap_socket, Mailbox::Inbox, SelectMod::None, None).context("select inbox")?;
-        noop_exists(imap_socket).context("noop loop must detect a new email")?;
+        noop_exists(imap_socket, 1).context("noop loop must detect a new email")?;
         add_flags_email(imap_socket, Selection::FirstId, Flag::Deleted)
             .context("add delete flags to the email")?;
         unselect(imap_socket)
             .context("unselect inbox while preserving email with the \\Delete flag")?;
-        select(imap_socket, Mailbox::Inbox, Some(1)).context("select inbox again")?;
+        select(imap_socket, Mailbox::Inbox, SelectMod::None, Some(1)).context("select inbox again")?;
         fetch_rfc822(imap_socket, Selection::FirstId, Email::Basic)
             .context("message is still present")?;
         close(imap_socket).context("close inbox and expunge message")?;
-        select(imap_socket, Mailbox::Inbox, Some(0))
+        select(imap_socket, Mailbox::Inbox, SelectMod::None, Some(0))
             .context("select inbox again and check it's empty")?;
 
         Ok(())
@@ -73,7 +76,7 @@ fn rfc3691_imapext_unselect() {
 }
 
 fn rfc5161_imapext_enable() {
-    println!("rfc5161_imapext_enable");
+    println!("🧪 rfc5161_imapext_enable");
     common::aerogramme_provider_daemon_dev(|imap_socket, _lmtp_socket| {
         connect(imap_socket).context("server says hello")?;
         login(imap_socket, Account::Alice).context("login test")?;
@@ -87,25 +90,25 @@ fn rfc5161_imapext_enable() {
 }
 
 fn rfc6851_imapext_move() {
-    println!("rfc6851_imapext_move");
+    println!("🧪 rfc6851_imapext_move");
     common::aerogramme_provider_daemon_dev(|imap_socket, lmtp_socket| {
         connect(imap_socket).context("server says hello")?;
 
         capability(imap_socket, Extension::Move).context("check server capabilities")?;
         login(imap_socket, Account::Alice).context("login test")?;
         create_mailbox(imap_socket, Mailbox::Archive).context("created mailbox archive")?;
-        select(imap_socket, Mailbox::Inbox, None).context("select inbox")?;
+        select(imap_socket, Mailbox::Inbox, SelectMod::None, None).context("select inbox")?;
 
         lmtp_handshake(lmtp_socket).context("handshake lmtp done")?;
         lmtp_deliver_email(lmtp_socket, Email::Basic).context("mail delivered successfully")?;
 
-        noop_exists(imap_socket).context("noop loop must detect a new email")?;
+        noop_exists(imap_socket, 1).context("noop loop must detect a new email")?;
         r#move(imap_socket, Selection::FirstId, Mailbox::Archive)
             .context("message from inbox moved to archive")?;
 
         unselect(imap_socket)
             .context("unselect inbox while preserving email with the \\Delete flag")?;
-        select(imap_socket, Mailbox::Archive, Some(1)).context("select archive")?;
+        select(imap_socket, Mailbox::Archive, SelectMod::None, Some(1)).context("select archive")?;
         fetch_rfc822(imap_socket, Selection::FirstId, Email::Basic).context("check mail exists")?;
         logout(imap_socket).context("must quit")?;
 
@@ -115,7 +118,7 @@ fn rfc6851_imapext_move() {
 }
 
 fn rfc7888_imapext_literal() {
-    println!("rfc7888_imapext_literal");
+    println!("🧪 rfc7888_imapext_literal");
     common::aerogramme_provider_daemon_dev(|imap_socket, _lmtp_socket| {
         connect(imap_socket).context("server says hello")?;
 
@@ -126,3 +129,21 @@ fn rfc7888_imapext_literal() {
     })
     .expect("test fully run");
 }
+
+fn rfc4551_imapext_condstore() {
+    println!("🧪 rfc4551_imapext_condstore");
+    common::aerogramme_provider_daemon_dev(|imap_socket, lmtp_socket| {
+        lmtp_handshake(lmtp_socket).context("handshake lmtp done")?;
+        lmtp_deliver_email(lmtp_socket, Email::Basic).context("mail delivered successfully")?;
+        lmtp_deliver_email(lmtp_socket, Email::Multipart).context("mail delivered successfully")?;
+
+        connect(imap_socket).context("server says hello")?;
+        capability(imap_socket, Extension::Condstore).context("check server capabilities")?;
+        login(imap_socket, Account::Alice).context("login test")?;
+        select(imap_socket, Mailbox::Inbox, SelectMod::Condstore, None).context("select inbox")?;
+        noop_exists(imap_socket, 2).context("noop loop must detect a new email")?;
+
+        Ok(())
+    })
+    .expect("test fully run");
+}

tests/common/fragments.rs

@@ -34,7 +34,7 @@ pub enum Extension {
     None,
     Unselect,
     Move,
-    CondStore,
+    Condstore,
     LiteralPlus,
 }
 
@@ -65,6 +65,11 @@ pub enum Selection {
     SecondId,
 }
 
+pub enum SelectMod {
+    None,
+    Condstore,
+}
+
 pub fn capability(imap: &mut TcpStream, ext: Extension) -> Result<()> {
     imap.write(&b"5 capability\r\n"[..])?;
 
@@ -72,7 +77,7 @@ pub fn capability(imap: &mut TcpStream, ext: Extension) -> Result<()> {
         Extension::None => None,
         Extension::Unselect => Some("UNSELECT"),
         Extension::Move => Some("MOVE"),
-        Extension::CondStore => Some("CONDSTORE"),
+        Extension::Condstore => Some("CONDSTORE"),
         Extension::LiteralPlus => Some("LITERAL+"),
     };
 
@@ -125,7 +130,7 @@ pub fn create_mailbox(imap: &mut TcpStream, mbx: Mailbox) -> Result<()> {
     Ok(())
 }
 
-pub fn select(imap: &mut TcpStream, mbx: Mailbox, maybe_exists: Option<u64>) -> Result<()> {
+pub fn select(imap: &mut TcpStream, mbx: Mailbox, modifier: SelectMod, maybe_exists: Option<u64>) -> Result<()> {
     let mut buffer: [u8; 6000] = [0; 6000];
 
     let mbx_str = match mbx {
@@ -133,7 +138,13 @@ pub fn select(imap: &mut TcpStream, mbx: Mailbox, maybe_exists: Option<u64>) ->
         Mailbox::Archive => "Archive",
         Mailbox::Drafts => "Drafts",
     };
-    imap.write(format!("20 select {}\r\n", mbx_str).as_bytes())?;
+
+    let mod_str = match modifier {
+        SelectMod::Condstore => " (CONDSTORE)",
+        SelectMod::None => "",
+    };
+
+    imap.write(format!("20 select {}{}\r\n", mbx_str, mod_str).as_bytes())?;
 
     let read = read_lines(imap, &mut buffer, Some(&b"20 OK"[..]))?;
     let srv_msg = std::str::from_utf8(read)?;
@@ -141,6 +152,12 @@ pub fn select(imap: &mut TcpStream, mbx: Mailbox, maybe_exists: Option<u64>) ->
         let expected = format!("* {} EXISTS", exists);
         assert!(srv_msg.contains(&expected));
     }
+    match modifier {
+        SelectMod::Condstore => {
+            assert!(srv_msg.contains("[HIGHESTMODSEQ"));
+        }
+        _ => (),
+    }
 
     Ok(())
 }
@@ -206,7 +223,7 @@ pub fn lmtp_deliver_email(lmtp: &mut TcpStream, email_type: Email) -> Result<()>
     Ok(())
 }
 
-pub fn noop_exists(imap: &mut TcpStream) -> Result<()> {
+pub fn noop_exists(imap: &mut TcpStream, must_exists: u32) -> Result<()> {
     let mut buffer: [u8; 6000] = [0; 6000];
 
     let mut max_retry = 20;
@@ -216,16 +233,23 @@ pub fn noop_exists(imap: &mut TcpStream) -> Result<()> {
         let read = read_lines(imap, &mut buffer, Some(&b"30 OK"[..]))?;
         let srv_msg = std::str::from_utf8(read)?;
 
-        match (max_retry, srv_msg.lines().count()) {
+        for line in srv_msg.lines() {
-            (_, cnt) if cnt > 1 => break,
+            if line.contains("EXISTS") {
-            (0, _) => bail!("no more retry"),
+                let got = read_first_u32(line)?;
-            _ => (),
+                if got == must_exists {
+                    // Done
+                    return Ok(());
+                }
+            }
+        }
+
+        if max_retry <= 0 {
+            // Failed
+            bail!("no more retry");
         }
 
         thread::sleep(SMALL_DELAY);
     }
-
-    Ok(())
 }
 
 pub fn fetch_rfc822(imap: &mut TcpStream, selection: Selection, r#ref: Email) -> Result<()> {
@@ -281,9 +305,6 @@ pub fn append_email(imap: &mut TcpStream, content: Email) -> Result<()> {
     let read = read_lines(imap, &mut buffer, None)?;
     assert_eq!(&read[..5], &b"47 OK"[..]);
 
-    // we check that noop detects the change
-    noop_exists(imap)?;
-
     Ok(())
 }
 
@@ -292,7 +313,7 @@ pub fn add_flags_email(imap: &mut TcpStream, selection: Selection, flag: Flag) -
     assert!(matches!(selection, Selection::FirstId));
     assert!(matches!(flag, Flag::Deleted));
     imap.write(&b"50 store 1 +FLAGS (\\Deleted)\r\n"[..])?;
-    let _read = read_lines(imap, &mut buffer, Some(&b"50 OK STORE"[..]))?;
+    let _read = read_lines(imap, &mut buffer, Some(&b"50 OK"[..]))?;
 
     Ok(())
 }
@@ -302,7 +323,7 @@ pub fn add_flags_email(imap: &mut TcpStream, selection: Selection, flag: Flag) -
 pub fn search(imap: &mut TcpStream) -> Result<()> {
     imap.write(&b"55 search text \"OoOoO\"\r\n"[..])?;
     let mut buffer: [u8; 1500] = [0; 1500];
-    let _read = read_lines(imap, &mut buffer, Some(&b"55 OK SEARCH"[..]))?;
+    let _read = read_lines(imap, &mut buffer, Some(&b"55 OK"[..]))?;
     Ok(())
 }
 

tests/common/mod.rs

@@ -88,3 +88,12 @@ pub fn read_lines<'a, F: Read>(
     println!("read: {}", std::str::from_utf8(&buffer[..nbytes])?);
     Ok(&buffer[..nbytes])
 }
+
+pub fn read_first_u32(inp: &str) -> Result<u32> {
+    Ok(inp
+        .chars()
+        .skip_while(|c| !c.is_digit(10))
+        .take_while(|c| c.is_digit(10))
+        .collect::<String>()
+        .parse::<u32>()?)
+}