From 381cedb088de32ed50a61cf1957eddc88947cd64 Mon Sep 17 00:00:00 2001 From: mdecimus Date: Fri, 30 Jun 2023 19:59:05 +0200 Subject: [PATCH] IMAP server passing tests. --- crates/imap-proto/src/lib.rs | 3 ++ crates/imap-proto/src/parser/create.rs | 2 +- crates/imap-proto/src/parser/search.rs | 30 +++++------- crates/imap-proto/src/protocol/mod.rs | 1 + crates/imap/src/core/mailbox.rs | 4 +- crates/imap/src/core/message.rs | 63 ++++++++++++------------- crates/imap/src/op/acl.rs | 4 +- crates/imap/src/op/copy_move.rs | 1 + crates/imap/src/op/create.rs | 11 +++-- crates/imap/src/op/delete.rs | 6 +-- crates/imap/src/op/fetch.rs | 2 +- crates/imap/src/op/rename.rs | 6 +-- crates/imap/src/op/status.rs | 48 ++++++++++--------- crates/imap/src/op/store.rs | 15 ++---- crates/jmap/src/email/copy.rs | 3 +- crates/jmap/src/mailbox/set.rs | 2 +- crates/jmap/src/services/housekeeper.rs | 29 +++++++++--- crates/managesieve/src/core/client.rs | 4 +- crates/managesieve/src/core/session.rs | 20 ++++++-- crates/managesieve/src/lib.rs | 6 +++ crates/managesieve/src/op/capability.rs | 2 +- crates/managesieve/src/op/getscript.rs | 3 +- crates/managesieve/src/op/putscript.rs | 3 +- crates/store/src/query/sort.rs | 5 +- tests/src/imap/acl.rs | 26 +++++----- tests/src/imap/condstore.rs | 17 ++++--- tests/src/imap/copy_move.rs | 8 ++++ tests/src/imap/fetch.rs | 6 +-- tests/src/imap/mailbox.rs | 9 +++- tests/src/imap/managesieve.rs | 18 +++++-- tests/src/imap/mod.rs | 12 ++++- tests/src/imap/search.rs | 2 +- tests/src/imap/store.rs | 14 +++++- 33 files changed, 233 insertions(+), 152 deletions(-) diff --git a/crates/imap-proto/src/lib.rs b/crates/imap-proto/src/lib.rs index 9e9ac71f..82d216c9 100644 --- a/crates/imap-proto/src/lib.rs +++ b/crates/imap-proto/src/lib.rs @@ -141,6 +141,9 @@ pub enum ResponseCode { MailboxId { mailbox_id: String, }, + + // USEATTR + UseAttr, } #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/crates/imap-proto/src/parser/create.rs b/crates/imap-proto/src/parser/create.rs index f75dde60..53430a52 100644 --- a/crates/imap-proto/src/parser/create.rs +++ b/crates/imap-proto/src/parser/create.rs @@ -98,7 +98,7 @@ impl Request { tag: self.tag, }) } else { - Err(self.into_error("Too many arguments.")) + Err(self.into_error("Missing arguments.")) } } } diff --git a/crates/imap-proto/src/parser/search.rs b/crates/imap-proto/src/parser/search.rs index 034cecee..3a9c662b 100644 --- a/crates/imap-proto/src/parser/search.rs +++ b/crates/imap-proto/src/parser/search.rs @@ -310,25 +310,19 @@ pub fn parse_filters( ))); } } else if value.eq_ignore_ascii_case(b"EMAILID") { - let argument = tokens - .next() - .ok_or_else(|| Cow::from("Expected an EMAILID value."))? - .unwrap_string()?; - if let Some((_, email_id)) = argument.split_once('-') { - filters.push(Filter::EmailId(email_id.to_string())); - } else { - return Err(Cow::from("Malformed EMAILID value.")); - } + filters.push(Filter::EmailId( + tokens + .next() + .ok_or_else(|| Cow::from("Expected an EMAILID value."))? + .unwrap_string()?, + )); } else if value.eq_ignore_ascii_case(b"THREADID") { - let argument = tokens - .next() - .ok_or_else(|| Cow::from("Expected an THREADID value."))? - .unwrap_string()?; - if let Some((_, thread_id)) = argument.split_once('-') { - filters.push(Filter::ThreadId(thread_id.to_string())); - } else { - return Err(Cow::from("Malformed THREADID value.")); - } + filters.push(Filter::ThreadId( + tokens + .next() + .ok_or_else(|| Cow::from("Expected an THREADID value."))? + .unwrap_string()?, + )); } else if value.eq_ignore_ascii_case(b"OR") { if filters_stack.len() > 10 { return Err(Cow::from("Too many nested filters")); diff --git a/crates/imap-proto/src/protocol/mod.rs b/crates/imap-proto/src/protocol/mod.rs index 47476557..fa09c0dd 100644 --- a/crates/imap-proto/src/protocol/mod.rs +++ b/crates/imap-proto/src/protocol/mod.rs @@ -383,6 +383,7 @@ impl ResponseCode { buf.extend_from_slice(modseq.to_string().as_bytes()); return; } + ResponseCode::UseAttr => b"USEATTR", }); } } diff --git a/crates/imap/src/core/mailbox.rs b/crates/imap/src/core/mailbox.rs index dec809e8..b577c6af 100644 --- a/crates/imap/src/core/mailbox.rs +++ b/crates/imap/src/core/mailbox.rs @@ -181,7 +181,7 @@ impl SessionData { } let has_children = mailboxes .iter() - .any(|(_, child_parent_id, _)| child_parent_id == mailbox_id); + .any(|(_, child_parent_id, _)| *child_parent_id == *mailbox_id + 1); account.mailbox_state.insert( *mailbox_id, @@ -232,7 +232,7 @@ impl SessionData { if has_children && iter_stack.len() < 100 { iter_stack.push((iter, parent_id, path)); - parent_id = *mailbox_id; + parent_id = *mailbox_id + 1; path = mailbox_path; iter = mailboxes.iter(); } diff --git a/crates/imap/src/core/message.rs b/crates/imap/src/core/message.rs index 87b3f50f..a2f24bd0 100644 --- a/crates/imap/src/core/message.rs +++ b/crates/imap/src/core/message.rs @@ -17,6 +17,7 @@ use crate::core::ImapId; use super::{MailboxId, MailboxState, SelectedMailbox, SessionData}; +#[derive(Debug)] struct UidMap { uid_next: u32, uid_validity: u32, @@ -24,6 +25,7 @@ struct UidMap { items: Vec, } +#[derive(Debug)] struct Uid { uid: u32, id: u32, @@ -165,8 +167,8 @@ impl SessionData { .with_account_id(mailbox.account_id) .with_collection(Collection::Mailbox) .update_document(mailbox.mailbox_id.unwrap_or(u32::MAX)) - .assert_value(Property::EmailId, &uid_map) - .value(Property::EmailId, &uid_map.inner, F_VALUE); + .assert_value(Property::EmailIds, &uid_map) + .value(Property::EmailIds, &uid_map.inner, F_VALUE); match self.jmap.store.write(batch.build()).await { Ok(_) => (), @@ -213,33 +215,24 @@ impl SessionData { last_state, }); } else { - let uid_next = id_list.len() as u32; + let uid_next = (id_list.len() + 1) as u32; let uid_validity = now() as u32 ^ mailbox.mailbox_id.unwrap_or(0); - let mut id_to_imap = AHashMap::with_capacity(uid_next as usize); - let mut uid_to_id = AHashMap::with_capacity(uid_next as usize); - let mut uids = Vec::with_capacity(uid_next as usize); + let mut id_to_imap = AHashMap::with_capacity(id_list.len()); + let mut uid_to_id = AHashMap::with_capacity(id_list.len()); + let mut uids = Vec::with_capacity(id_list.len()); let mut uid_map = UidMap { uid_next, uid_validity, hash: id_list_hash, - items: Vec::with_capacity(uid_next as usize), + items: Vec::with_capacity(id_list.len()), }; for (uid, (id, received)) in id_list.into_iter().enumerate() { - id_to_imap.insert( - id, - ImapId { - uid: uid as u32, - seqnum: (uid + 1) as u32, - }, - ); - uid_to_id.insert(uid as u32, id); - uids.push(uid as u32); - uid_map.items.push(Uid { - uid: uid as u32, - id, - received, - }); + let uid = (uid + 1) as u32; + id_to_imap.insert(id, ImapId { uid, seqnum: uid }); + uid_to_id.insert(uid, id); + uids.push(uid); + uid_map.items.push(Uid { uid, id, received }); } // Store uid map @@ -248,7 +241,7 @@ impl SessionData { .with_account_id(mailbox.account_id) .with_collection(Collection::Mailbox) .update_document(mailbox.mailbox_id.unwrap_or(u32::MAX)) - .value(Property::EmailId, &uid_map, F_VALUE); + .value(Property::EmailIds, &uid_map, F_VALUE); self.jmap.store.write(batch.build()).await.map_err(|err| { tracing::error!(event = "error", context = "store", @@ -291,14 +284,18 @@ impl SessionData { let mut buf = Vec::with_capacity(64); let (new_message_count, deletions) = mailbox.update_mailbox_state(new_state, true); if let Some(deletions) = deletions { - expunge::Response { - is_qresync, - ids: deletions - .into_iter() - .map(|id| if !is_uid { id.seqnum } else { id.uid }) - .collect(), - } - .serialize_to(&mut buf); + let mut ids = deletions + .into_iter() + .map(|id| { + if is_uid || is_qresync { + id.uid + } else { + id.seqnum + } + }) + .collect::>(); + ids.sort_unstable(); + expunge::Response { is_qresync, ids }.serialize_to(&mut buf); } if let Some(new_message_count) = new_message_count { Exists { @@ -489,9 +486,9 @@ impl Serialize for &UidMap { buf.extend_from_slice(self.uid_validity.to_le_bytes().as_ref()); buf.extend_from_slice(self.hash.to_le_bytes().as_ref()); - let mut last_uid = u32::MAX; + let mut last_uid = 0; for item in &self.items { - if last_uid.wrapping_add(1) != item.uid { + if last_uid + 1 != item.uid { buf.push(0); buf.push_leb128(item.uid); } @@ -525,7 +522,7 @@ impl UidMap { hash: u64::from_le_bytes(buf_u64), items: Vec::with_capacity(items_len), }; - let mut next_uid: u32 = 0; + let mut next_uid: u32 = 1; for _ in 0..items_len { let mut id: u32 = bytes.next_leb128()?; if id == 0 { diff --git a/crates/imap/src/op/acl.rs b/crates/imap/src/op/acl.rs index b25796c7..f3a09adc 100644 --- a/crates/imap/src/op/acl.rs +++ b/crates/imap/src/op/acl.rs @@ -329,7 +329,9 @@ impl Session { if op == ModRightsOp::Add { bitmap.union(&rights); } else { - bitmap.intersection(&rights); + for right in rights { + bitmap.remove(right); + } } if !bitmap.is_empty() { *current = bitmap.into(); diff --git a/crates/imap/src/op/copy_move.rs b/crates/imap/src/op/copy_move.rs index afa34690..3cfd4d8f 100644 --- a/crates/imap/src/op/copy_move.rs +++ b/crates/imap/src/op/copy_move.rs @@ -266,6 +266,7 @@ impl SessionData { Collection::Mailbox, src_mailbox.id.mailbox_id.unwrap(), ); + did_move = true; } copied_ids.push((imap_id, id)); } diff --git a/crates/imap/src/op/create.rs b/crates/imap/src/op/create.rs index a2a7f724..767275cb 100644 --- a/crates/imap/src/op/create.rs +++ b/crates/imap/src/op/create.rs @@ -31,7 +31,7 @@ use jmap_proto::{ object::{index::ObjectIndexBuilder, Object}, types::{ acl::Acl, collection::Collection, id::Id, property::Property, state::StateChange, - type_state::TypeState, + type_state::TypeState, value::Value, }, }; use store::{query::Filter, write::BatchBuilder}; @@ -110,7 +110,7 @@ impl SessionData { }; let mut mailbox = Object::with_capacity(3) .with_property(Property::Name, path_item) - .with_property(Property::ParentId, parent_id); + .with_property(Property::ParentId, Value::Id(Id::from(parent_id))); if pos == params.path.len() - 1 { if let Some(mailbox_role) = arguments.mailbox_role { mailbox.set(Property::Role, mailbox_role); @@ -345,7 +345,9 @@ impl SessionData { ) .with_code(ResponseCode::NoPerm)); } - } else { + } else if self.account_id != account_id + && !self.get_access_token().await?.is_member(account_id) + { return Err(StatusResponse::no( "You are not allowed to create root folders under shared folders.", ) @@ -374,7 +376,8 @@ impl SessionData { { return Err(StatusResponse::no(format!( "A mailbox with role '{mailbox_role}' already exists.", - ))); + )) + .with_code(ResponseCode::UseAttr)); } Attribute::try_from(mailbox_role).ok() } else { diff --git a/crates/imap/src/op/delete.rs b/crates/imap/src/op/delete.rs index 20a8e888..8d650857 100644 --- a/crates/imap/src/op/delete.rs +++ b/crates/imap/src/op/delete.rs @@ -90,7 +90,7 @@ impl SessionData { { Ok(Ok(did_remove_emails)) => did_remove_emails, Ok(Err(err)) => { - return StatusResponse::no(err.description.unwrap()) + return StatusResponse::no(err.description.unwrap_or("Delete failed".into())) .with_code(err.type_.into()) .with_tag(arguments.tag) } @@ -120,10 +120,6 @@ impl SessionData { // Update mailbox cache for account in self.mailboxes.lock().iter_mut() { if account.account_id == account_id { - account.state_mailbox = change_id.into(); - if did_remove_emails { - account.state_email = change_id.into(); - } account.mailbox_names.remove(&arguments.mailbox_name); account.mailbox_state.remove(&mailbox_id); break; diff --git a/crates/imap/src/op/fetch.rs b/crates/imap/src/op/fetch.rs index 87d19ce5..4deabeac 100644 --- a/crates/imap/src/op/fetch.rs +++ b/crates/imap/src/op/fetch.rs @@ -493,7 +493,7 @@ impl SessionData { .get_property::(account_id, Collection::Email, id, Property::Cid) .await { - items.push(DataItem::ModSeq { modseq }); + items.push(DataItem::ModSeq { modseq: modseq + 1 }); } } Attribute::EmailId => { diff --git a/crates/imap/src/op/rename.rs b/crates/imap/src/op/rename.rs index 40312639..f7942d3c 100644 --- a/crates/imap/src/op/rename.rs +++ b/crates/imap/src/op/rename.rs @@ -31,7 +31,7 @@ use jmap_proto::{ error::method::MethodError, object::{index::ObjectIndexBuilder, Object}, types::{ - acl::Acl, collection::Collection, property::Property, state::StateChange, + acl::Acl, collection::Collection, id::Id, property::Property, state::StateChange, type_state::TypeState, value::Value, }, }; @@ -167,7 +167,7 @@ impl SessionData { ObjectIndexBuilder::new(SCHEMA).with_changes( Object::with_capacity(2) .with_property(Property::Name, path_item) - .with_property(Property::ParentId, parent_id), + .with_property(Property::ParentId, Value::Id(Id::from(parent_id))), ), ); changes.log_insert(Collection::Mailbox, mailbox_id); @@ -180,7 +180,7 @@ impl SessionData { .with_changes( Object::with_capacity(2) .with_property(Property::Name, new_mailbox_name) - .with_property(Property::ParentId, parent_id), + .with_property(Property::ParentId, Value::Id(Id::from(parent_id))), ), ); changes.log_update(Collection::Mailbox, mailbox_id); diff --git a/crates/imap/src/op/status.rs b/crates/imap/src/op/status.rs index c4f0c27a..7504564d 100644 --- a/crates/imap/src/op/status.rs +++ b/crates/imap/src/op/status.rs @@ -23,8 +23,8 @@ use std::sync::Arc; -use ahash::AHashSet; use imap_proto::{ + parser::PushUnique, protocol::status::{Status, StatusItem, StatusItemType}, receiver::Request, Command, ResponseCode, StatusResponse, @@ -93,7 +93,7 @@ impl SessionData { }; // Make sure all requested fields are up to date - let mut items_update = AHashSet::with_capacity(items.len()); + let mut items_update = Vec::with_capacity(items.len()); let mut items_response = Vec::with_capacity(items.len()); let mut do_synchronize = false; @@ -101,7 +101,7 @@ impl SessionData { if account.account_id == mailbox.account_id { let mailbox_state = account .mailbox_state - .entry(mailbox.mailbox_id.as_ref().cloned().unwrap_or_default()) + .entry(mailbox.mailbox_id.as_ref().cloned().unwrap_or(u32::MAX)) .or_insert_with(Mailbox::default); for item in items { match item { @@ -109,14 +109,14 @@ impl SessionData { if let Some(value) = mailbox_state.total_messages { items_response.push((*item, StatusItemType::Number(value as u64))); } else { - items_update.insert(*item); + items_update.push_unique(*item); } } Status::UidNext => { if let Some(value) = mailbox_state.uid_next { items_response.push((*item, StatusItemType::Number(value as u64))); } else { - items_update.insert(*item); + items_update.push_unique(*item); do_synchronize = true; } } @@ -124,7 +124,7 @@ impl SessionData { if let Some(value) = mailbox_state.uid_validity { items_response.push((*item, StatusItemType::Number(value as u64))); } else { - items_update.insert(*item); + items_update.push_unique(*item); do_synchronize = true; } } @@ -132,21 +132,21 @@ impl SessionData { if let Some(value) = mailbox_state.total_unseen { items_response.push((*item, StatusItemType::Number(value as u64))); } else { - items_update.insert(*item); + items_update.push_unique(*item); } } Status::Deleted => { if let Some(value) = mailbox_state.total_deleted { items_response.push((*item, StatusItemType::Number(value as u64))); } else { - items_update.insert(*item); + items_update.push_unique(*item); } } Status::Size => { if let Some(value) = mailbox_state.size { items_response.push((*item, StatusItemType::Number(value as u64))); } else { - items_update.insert(*item); + items_update.push_unique(*item); } } Status::HighestModSeq => { @@ -203,25 +203,31 @@ impl SessionData { for item in items_update { let result = match item { - Status::Messages => message_ids.as_ref().map(|v| v.len()).unwrap_or(0), + Status::Messages => { + mailbox_message_ids.as_ref().map(|v| v.len()).unwrap_or(0) + } Status::UidNext => mailbox_state.as_ref().unwrap().uid_next as u64, Status::UidValidity => mailbox_state.as_ref().unwrap().uid_validity as u64, Status::Unseen => { - if let (Some(message_ids), Some(mailbox_message_ids), Some(mut seen)) = ( - &message_ids, - &mailbox_message_ids, - self.jmap + if let (Some(message_ids), Some(mailbox_message_ids)) = + (&message_ids, &mailbox_message_ids) + { + if let Some(mut seen) = self + .jmap .get_tag( mailbox.account_id, Collection::Email, Property::Keywords, Keyword::Seen, ) - .await?, - ) { - seen ^= message_ids; - seen &= mailbox_message_ids.as_ref(); - seen.len() + .await? + { + seen ^= message_ids; + seen &= mailbox_message_ids.as_ref(); + seen.len() + } else { + mailbox_message_ids.len() + } } else { 0 } @@ -285,7 +291,7 @@ impl SessionData { seen ^= message_ids.as_ref(); seen.len() }) - .unwrap_or(0), + .unwrap_or_else(|| message_ids.len()), Status::Deleted => self .jmap .get_tag( @@ -320,7 +326,7 @@ impl SessionData { if account.account_id == mailbox.account_id { let mailbox_state = account .mailbox_state - .entry(mailbox.mailbox_id.as_ref().cloned().unwrap_or_default()) + .entry(mailbox.mailbox_id.as_ref().cloned().unwrap_or(u32::MAX)) .or_insert_with(Mailbox::default); for (item, value) in values_update { diff --git a/crates/imap/src/op/store.rs b/crates/imap/src/op/store.rs index 61aa4954..7b830c7c 100644 --- a/crates/imap/src/op/store.rs +++ b/crates/imap/src/op/store.rs @@ -243,7 +243,7 @@ impl SessionData { } Operation::Clear => { for keyword in &set_keywords { - keywords.update(keyword.clone(), true); + keywords.update(keyword.clone(), false); } } } @@ -305,15 +305,14 @@ impl SessionData { changelog.log_update(Collection::Email, Id::from_parts(thread_id, id)); // Add item to response + let modseq = changelog.change_id + 1; if !arguments.is_silent { let mut data_items = vec![DataItem::Flags { flags }]; if is_uid { data_items.push(DataItem::Uid { uid: imap_id.uid }); } if is_condstore { - data_items.push(DataItem::ModSeq { - modseq: changelog.change_id, - }); + data_items.push(DataItem::ModSeq { modseq }); } items.items.push(FetchItem { id: imap_id.seqnum, @@ -324,15 +323,11 @@ impl SessionData { id: imap_id.seqnum, items: if is_uid { vec![ - DataItem::ModSeq { - modseq: changelog.change_id, - }, + DataItem::ModSeq { modseq }, DataItem::Uid { uid: imap_id.uid }, ] } else { - vec![DataItem::ModSeq { - modseq: changelog.change_id, - }] + vec![DataItem::ModSeq { modseq }] }, }); } diff --git a/crates/jmap/src/email/copy.rs b/crates/jmap/src/email/copy.rs index 7d2258f1..6c6eeac6 100644 --- a/crates/jmap/src/email/copy.rs +++ b/crates/jmap/src/email/copy.rs @@ -433,7 +433,8 @@ impl JMAP { .value(Property::Keywords, keywords, F_VALUE | F_BITMAP) .value(Property::Cid, changes.change_id, F_VALUE) .custom(EmailIndexBuilder::set(metadata)) - .custom(token_index); + .custom(token_index) + .custom(changes); self.store.write(batch.build()).await.map_err(|err| { tracing::error!( diff --git a/crates/jmap/src/mailbox/set.rs b/crates/jmap/src/mailbox/set.rs index 7c23fe01..e187b711 100644 --- a/crates/jmap/src/mailbox/set.rs +++ b/crates/jmap/src/mailbox/set.rs @@ -775,7 +775,7 @@ impl JMAP { Object::with_capacity(3) .with_property(Property::Name, name) .with_property(Property::Role, role) - .with_property(Property::ParentId, 0u32), + .with_property(Property::ParentId, Value::Id(0u64.into())), ), ); mailbox_ids.insert(mailbox_id); diff --git a/crates/jmap/src/services/housekeeper.rs b/crates/jmap/src/services/housekeeper.rs index 1b37dda2..43e6c445 100644 --- a/crates/jmap/src/services/housekeeper.rs +++ b/crates/jmap/src/services/housekeeper.rs @@ -23,7 +23,7 @@ use std::{sync::Arc, time::Duration}; -use chrono::{Datelike, TimeZone}; +use chrono::{Datelike, TimeZone, Timelike}; use store::write::now; use tokio::sync::mpsc; use utils::{config::Config, failed, map::ttl_dashmap::TtlMap, UnwrapFailure}; @@ -42,6 +42,7 @@ pub enum Event { enum SimpleCron { EveryDay { hour: u32, minute: u32 }, EveryWeek { day: u32, hour: u32, minute: u32 }, + EveryHour { minute: u32 }, } const TASK_PURGE_DB: usize = 0; @@ -155,20 +156,24 @@ impl SimpleCron { for (pos, value) in value.split(' ').enumerate() { if pos == 0 { - minute = value.parse::().failed("parse minute."); + minute = value.parse::().failed("parse cron minute"); if !(0..=59).contains(&minute) { failed(&format!("parse minute, invalid value: {}", minute)); } } else if pos == 1 { - hour = value.parse::().failed("parse hour."); - if !(0..=23).contains(&hour) { - failed(&format!("parse hour, invalid value: {}", hour)); + if value.as_bytes().first().failed("parse cron weekday") == &b'*' { + return SimpleCron::EveryHour { minute }; + } else { + hour = value.parse::().failed("parse cron hour"); + if !(0..=23).contains(&hour) { + failed(&format!("parse hour, invalid value: {}", hour)); + } } } else if pos == 2 { - if value.as_bytes().first().failed("parse weekday") == &b'*' { + if value.as_bytes().first().failed("parse cron weekday") == &b'*' { return SimpleCron::EveryDay { hour, minute }; } else { - let day = value.parse::().failed("parse weekday."); + let day = value.parse::().failed("parse cron weekday"); if !(1..=7).contains(&hour) { failed(&format!( "parse weekday, invalid value: {}, range is 1 (Monday) to 7 (Sunday).", @@ -209,6 +214,16 @@ impl SimpleCron { next } } + SimpleCron::EveryHour { minute } => { + let next = chrono::Local + .with_ymd_and_hms(now.year(), now.month(), now.day(), now.hour(), *minute, 0) + .unwrap(); + if next < now { + next + chrono::Duration::hours(1) + } else { + next + } + } }; (next - now).to_std().unwrap() diff --git a/crates/managesieve/src/core/client.rs b/crates/managesieve/src/core/client.rs index cfada9f6..4315f710 100644 --- a/crates/managesieve/src/core/client.rs +++ b/crates/managesieve/src/core/client.rs @@ -58,7 +58,7 @@ impl Session { Command::Authenticate => self.handle_authenticate(request).await, Command::StartTls => { self.write(b"OK Begin TLS negotiation now\r\n").await?; - return Ok(true); + return Ok(false); } Command::Logout => self.handle_logout().await, Command::Noop => self.handle_noop(request).await, @@ -82,7 +82,7 @@ impl Session { .await?; } - Ok(false) + Ok(true) } } diff --git a/crates/managesieve/src/core/session.rs b/crates/managesieve/src/core/session.rs index 164a2e39..69eefc84 100644 --- a/crates/managesieve/src/core/session.rs +++ b/crates/managesieve/src/core/session.rs @@ -7,12 +7,14 @@ use tokio::{ use tokio_rustls::server::TlsStream; use utils::listener::SessionManager; +use crate::SERVER_GREETING; + use super::{IsTls, ManageSieveSessionManager, Session, State}; impl SessionManager for ManageSieveSessionManager { fn spawn(&self, session: utils::listener::SessionData) { // Create session - let session = Session { + let mut session = Session { jmap: self.jmap.clone(), imap: self.imap.clone(), instance: session.instance, @@ -27,10 +29,20 @@ impl SessionManager for ManageSieveSessionManager { tokio::spawn(async move { if session.instance.is_tls_implicit { - if let Ok(session) = session.into_tls().await { - session.handle_conn().await; + if let Ok(mut session) = session.into_tls().await { + if session + .write(&session.handle_capability(SERVER_GREETING).await.unwrap()) + .await + .is_ok() + { + session.handle_conn().await; + } } - } else { + } else if session + .write(&session.handle_capability(SERVER_GREETING).await.unwrap()) + .await + .is_ok() + { session.handle_conn().await; } }); diff --git a/crates/managesieve/src/lib.rs b/crates/managesieve/src/lib.rs index 5f937f62..51a95938 100644 --- a/crates/managesieve/src/lib.rs +++ b/crates/managesieve/src/lib.rs @@ -1,6 +1,12 @@ pub mod core; pub mod op; +static SERVER_GREETING: &str = concat!( + "Stalwart ManageSieve v", + env!("CARGO_PKG_VERSION"), + " at your service." +); + #[cfg(test)] mod tests { use imap_proto::receiver::{Error, Receiver, Request, State, Token}; diff --git a/crates/managesieve/src/op/capability.rs b/crates/managesieve/src/op/capability.rs index d02b1f1b..33785e46 100644 --- a/crates/managesieve/src/op/capability.rs +++ b/crates/managesieve/src/op/capability.rs @@ -27,7 +27,7 @@ use tokio::io::{AsyncRead, AsyncWrite}; use crate::core::{IsTls, Session, StatusResponse}; impl Session { - pub async fn handle_capability(&mut self, message: &'static str) -> super::OpResult { + pub async fn handle_capability(&self, message: &'static str) -> super::OpResult { let mut response = Vec::with_capacity(128); response.extend_from_slice(b"\"IMPLEMENTATION\" \"Stalwart ManageSieve v"); response.extend_from_slice(env!("CARGO_PKG_VERSION").as_bytes()); diff --git a/crates/managesieve/src/op/getscript.rs b/crates/managesieve/src/op/getscript.rs index 3af05ca6..5d89bde8 100644 --- a/crates/managesieve/src/op/getscript.rs +++ b/crates/managesieve/src/op/getscript.rs @@ -53,7 +53,7 @@ impl Session { .ok_or_else(|| { StatusResponse::no("Script not found").with_code(ResponseCode::NonExistent) })? - .remove(&Property::BlobId) + .remove(&Property::Size) .try_unwrap_uint() .ok_or_else(|| { StatusResponse::no("Filed to retrieve blob size").with_code(ResponseCode::TryLater) @@ -72,6 +72,7 @@ impl Session { .ok_or_else(|| { StatusResponse::no("Script blob not found").with_code(ResponseCode::NonExistent) })?; + debug_assert_eq!(script.len() as u32, script_size); let mut response = Vec::with_capacity(script.len() + 30); response.push(b'{'); diff --git a/crates/managesieve/src/op/putscript.rs b/crates/managesieve/src/op/putscript.rs index dabde984..06258994 100644 --- a/crates/managesieve/src/op/putscript.rs +++ b/crates/managesieve/src/op/putscript.rs @@ -46,6 +46,7 @@ impl Session { .next() .ok_or_else(|| StatusResponse::no("Expected script as a parameter."))? .unwrap_bytes(); + let script_len = script.len() as u64; // Check quota let access_token = self.state.access_token(); @@ -113,7 +114,7 @@ impl Session { Object::with_capacity(3) .with_property(Property::Name, name) .with_property(Property::IsActive, Value::Bool(false)) - .with_property(Property::Size, Value::UnsignedInt(script.len() as u64)), + .with_property(Property::Size, Value::UnsignedInt(script_len)), ), ) .custom(changelog); diff --git a/crates/store/src/query/sort.rs b/crates/store/src/query/sort.rs index 12fa32e8..b27ad0c9 100644 --- a/crates/store/src/query/sort.rs +++ b/crates/store/src/query/sort.rs @@ -109,6 +109,7 @@ impl ReadTransaction<'_> { Ok(sorted_results) } else { + //TODO improve this algorithm, avoid re-sorting in memory. let mut sorted_ids = AHashMap::with_capacity(paginate.limit); for (pos, comparator) in comparators.into_iter().take(4).enumerate() { @@ -152,9 +153,7 @@ impl ReadTransaction<'_> { for document_id in results { sorted_ids.entry(document_id).or_insert([0u32; 4])[pos] = idx; } - } - - if !has_grouped_ids { + } else if !has_grouped_ids { // If we are sorting by multiple fields and we don't have grouped ids, we can // stop here break; diff --git a/tests/src/imap/acl.rs b/tests/src/imap/acl.rs index b5a10823..54013d15 100644 --- a/tests/src/imap/acl.rs +++ b/tests/src/imap/acl.rs @@ -67,8 +67,8 @@ pub async fn test(mut imap_john: &mut ImapConnection, _imap_check: &mut ImapConn .assert_read(Type::Tagged, ResponseType::Ok) .await .assert_equals("* LIST (\\NoSelect) \"/\" \"Shared Folders\"") - .assert_equals("* LIST (\\NoSelect) \"/\" \"Shared Folders/Jane Smith\"") - .assert_equals("* LIST () \"/\" \"Shared Folders/Jane Smith/Inbox\""); + .assert_equals("* LIST (\\NoSelect) \"/\" \"Shared Folders/jane.smith@example.com\"") + .assert_equals("* LIST () \"/\" \"Shared Folders/jane.smith@example.com/Inbox\""); // Grant access to Bill and check ACLs imap_jane.send("GETACL INBOX").await; @@ -93,7 +93,7 @@ pub async fn test(mut imap_john: &mut ImapConnection, _imap_check: &mut ImapConn imap_bill .assert_read(Type::Tagged, ResponseType::Ok) .await - .assert_contains("Shared Folders/Jane Smith/Inbox"); + .assert_contains("Shared Folders/jane.smith@example.com/Inbox"); // Namespace should now return the Shared Folders namespace imap_john.send("NAMESPACE").await; @@ -104,17 +104,17 @@ pub async fn test(mut imap_john: &mut ImapConnection, _imap_check: &mut ImapConn // List John's right on Jane's Inbox imap_john - .send("MYRIGHTS \"Shared Folders/Jane Smith/Inbox\"") + .send("MYRIGHTS \"Shared Folders/jane.smith@example.com/Inbox\"") .await; imap_john .assert_read(Type::Tagged, ResponseType::Ok) .await - .assert_equals("* MYRIGHTS \"Shared Folders/Jane Smith/Inbox\" rl"); + .assert_equals("* MYRIGHTS \"Shared Folders/jane.smith@example.com/Inbox\" rl"); // John should not be able to append messages assert_append_message( imap_john, - "Shared Folders/Jane Smith/Inbox", + "Shared Folders/jane.smith@example.com/Inbox", "From: john\n\ncontents", ResponseType::No, ) @@ -125,15 +125,15 @@ pub async fn test(mut imap_john: &mut ImapConnection, _imap_check: &mut ImapConn imap_jane.send("SETACL INBOX jdoe@example.com +i").await; imap_jane.assert_read(Type::Tagged, ResponseType::Ok).await; imap_john - .send("MYRIGHTS \"Shared Folders/Jane Smith/Inbox\"") + .send("MYRIGHTS \"Shared Folders/jane.smith@example.com/Inbox\"") .await; imap_john .assert_read(Type::Tagged, ResponseType::Ok) .await - .assert_equals("* MYRIGHTS \"Shared Folders/Jane Smith/Inbox\" rli"); + .assert_equals("* MYRIGHTS \"Shared Folders/jane.smith@example.com/Inbox\" rli"); assert_append_message( imap_john, - "Shared Folders/Jane Smith/Inbox", + "Shared Folders/jane.smith@example.com/Inbox", "From: john\n\ncontents", ResponseType::Ok, ) @@ -141,7 +141,7 @@ pub async fn test(mut imap_john: &mut ImapConnection, _imap_check: &mut ImapConn // Only Bill should be allowed to delete messages on Jane's Inbox for imap in [&mut imap_john, &mut imap_bill] { - imap.send("SELECT \"Shared Folders/Jane Smith/Inbox\"") + imap.send("SELECT \"Shared Folders/jane.smith@example.com/Inbox\"") .await; imap.assert_read(Type::Tagged, ResponseType::Ok).await; } @@ -152,7 +152,7 @@ pub async fn test(mut imap_john: &mut ImapConnection, _imap_check: &mut ImapConn imap_bill.assert_read(Type::Tagged, ResponseType::Ok).await; imap_john.send("UID EXPUNGE").await; - imap_john.assert_read(Type::Tagged, ResponseType::Ok).await; + imap_john.assert_read(Type::Tagged, ResponseType::No).await; imap_john.send("UID FETCH 1 (PREVIEW)").await; imap_john @@ -170,7 +170,7 @@ pub async fn test(mut imap_john: &mut ImapConnection, _imap_check: &mut ImapConn .assert_count("contents", 0); imap_bill - .send("STATUS \"Shared Folders/Jane Smith/Inbox\" (MESSAGES)") + .send("STATUS \"Shared Folders/jane.smith@example.com/Inbox\" (MESSAGES)") .await; imap_bill .assert_read(Type::Tagged, ResponseType::Ok) @@ -193,7 +193,7 @@ pub async fn test(mut imap_john: &mut ImapConnection, _imap_check: &mut ImapConn // Copy from John's Inbox to Jane's Inbox imap_john .send(&format!( - "UID COPY {} \"Shared Folders/Jane Smith/Inbox\"", + "UID COPY {} \"Shared Folders/jane.smith@example.com/Inbox\"", uid )) .await; diff --git a/tests/src/imap/condstore.rs b/tests/src/imap/condstore.rs index 64d82fa4..43f9aa95 100644 --- a/tests/src/imap/condstore.rs +++ b/tests/src/imap/condstore.rs @@ -181,7 +181,7 @@ pub async fn test(imap: &mut ImapConnection, imap_check: &mut ImapConnection) { imap.assert_read(Type::Tagged, ResponseType::Ok) .await .assert_count("VANISHED", 1) - .assert_contains("VANISHED (EARLIER) 2") + .assert_contains("VANISHED (EARLIER) 1:2") // .assert_contains("VANISHED (EARLIER) 2") .assert_count("FETCH (", 3); // Fetch changes since SEQ 4 @@ -193,7 +193,7 @@ pub async fn test(imap: &mut ImapConnection, imap_check: &mut ImapConnection) { imap.assert_read(Type::Tagged, ResponseType::Ok) .await .assert_count("VANISHED", 1) - .assert_contains("VANISHED (EARLIER) 2") + .assert_contains("VANISHED (EARLIER) 1:2") // .assert_contains("VANISHED (EARLIER) 2") .assert_count("FETCH (", 2); // Fetch changes since SEQ 6 @@ -205,7 +205,7 @@ pub async fn test(imap: &mut ImapConnection, imap_check: &mut ImapConnection) { imap.assert_read(Type::Tagged, ResponseType::Ok) .await .assert_count("VANISHED", 1) - .assert_contains("VANISHED (EARLIER) 2") + .assert_contains("VANISHED (EARLIER) 1:2") // .assert_contains("VANISHED (EARLIER) 2") .assert_count("FETCH (", 1); // Fetch changes since SEQ 7 @@ -217,7 +217,7 @@ pub async fn test(imap: &mut ImapConnection, imap_check: &mut ImapConnection) { imap.assert_read(Type::Tagged, ResponseType::Ok) .await .assert_count("VANISHED", 1) - .assert_contains("VANISHED (EARLIER) 2") + .assert_contains("VANISHED (EARLIER) 1:2") // .assert_contains("VANISHED (EARLIER) 2") .assert_count("FETCH (", 0); // Fetch changes since SEQ 8 @@ -238,6 +238,11 @@ pub async fn test(imap: &mut ImapConnection, imap_check: &mut ImapConnection) { .await .assert_contains("ALL 1:3 MODSEQ"); + imap_check.send("NOOP").await; + imap_check + .assert_read(Type::Tagged, ResponseType::Ok) + .await + .assert_contains("3 EXISTS"); imap_check .send(&format!("SEARCH MODSEQ {}", modseqs[4])) .await; @@ -252,7 +257,7 @@ pub async fn test(imap: &mut ImapConnection, imap_check: &mut ImapConnection) { modseqs[5] )) .await; - imap.assert_read(Type::Tagged, ResponseType::No) + imap.assert_read(Type::Tagged, ResponseType::Ok) .await .assert_contains("* 1 FETCH") .assert_contains("UID 3)") @@ -286,5 +291,5 @@ pub async fn test(imap: &mut ImapConnection, imap_check: &mut ImapConnection) { imap.assert_read(Type::Tagged, ResponseType::Ok) .await .assert_count("FETCH (", 3) - .assert_contains("VANISHED (EARLIER) 2"); + .assert_contains("VANISHED (EARLIER) 1:2"); // .assert_contains("VANISHED (EARLIER) 2"); } diff --git a/tests/src/imap/copy_move.rs b/tests/src/imap/copy_move.rs index 2fd7c8b8..46be2ef2 100644 --- a/tests/src/imap/copy_move.rs +++ b/tests/src/imap/copy_move.rs @@ -26,6 +26,14 @@ use imap_proto::ResponseType; use super::{AssertResult, ImapConnection, Type}; pub async fn test(imap: &mut ImapConnection, _imap_check: &mut ImapConnection) { + // Check status + imap.send("LIST \"\" % RETURN (STATUS (UIDNEXT MESSAGES UNSEEN SIZE))") + .await; + imap.assert_read(Type::Tagged, ResponseType::Ok) + .await + .assert_contains("\"INBOX\" (UIDNEXT 11 MESSAGES 10 UNSEEN 10 SIZE 12193)") + .assert_contains("\"All Mail\" (UIDNEXT 11 MESSAGES 10 UNSEEN 10 SIZE 12193)"); + // Select INBOX imap.send("SELECT INBOX").await; imap.assert_read(Type::Tagged, ResponseType::Ok).await; diff --git a/tests/src/imap/fetch.rs b/tests/src/imap/fetch.rs index 59507733..de07e67b 100644 --- a/tests/src/imap/fetch.rs +++ b/tests/src/imap/fetch.rs @@ -41,7 +41,7 @@ pub async fn test(imap: &mut ImapConnection, _imap_check: &mut ImapConnection) { .await; imap.assert_read(Type::Tagged, ResponseType::Ok) .await - .assert_contains("FLAGS (flag_009)") + .assert_contains("FLAGS (Flag_009)") .assert_contains("RFC822.SIZE 1457") .assert_contains("UID 10") .assert_contains("INTERNALDATE") @@ -104,7 +104,7 @@ pub async fn test(imap: &mut ImapConnection, _imap_check: &mut ImapConnection) { imap.send("UID FETCH 10 (FLAGS)").await; imap.assert_read(Type::Tagged, ResponseType::Ok) .await - .assert_contains("FLAGS (flag_009)"); + .assert_contains("FLAGS (Flag_009)"); // Switch to SELECT mode imap.send("SELECT INBOX").await; @@ -123,7 +123,7 @@ pub async fn test(imap: &mut ImapConnection, _imap_check: &mut ImapConnection) { imap.send("UID FETCH 10 (FLAGS)").await; imap.assert_read(Type::Tagged, ResponseType::Ok) .await - .assert_contains("FLAGS (flag_009)"); + .assert_contains("FLAGS (Flag_009)"); // Fetching a body section should set the \Seen flag imap.send("UID FETCH 10 (BODY[1.TEXT])").await; diff --git a/tests/src/imap/mailbox.rs b/tests/src/imap/mailbox.rs index d687b1a7..e17d8c7b 100644 --- a/tests/src/imap/mailbox.rs +++ b/tests/src/imap/mailbox.rs @@ -70,12 +70,17 @@ pub async fn test(mut imap: &mut ImapConnection, mut imap_check: &mut ImapConnec imap.send("CREATE \"All Mail/Untitled\"").await; imap.assert_read(Type::Tagged, ResponseType::No).await; + // Special use folders that already exist should not be allowed + imap.send("CREATE \"Second trash\" (USE (\\Trash))").await; + imap.assert_read(Type::Tagged, ResponseType::No).await; + // Enable IMAP4rev2 imap.send("ENABLE IMAP4rev2").await; imap.assert_read(Type::Tagged, ResponseType::Ok).await; // Create missing parent folders - imap.send("CREATE \"/Vegetable/Broccoli\"").await; + imap.send("CREATE \"/Vegetable/Broccoli\" (USE (\\Important))") + .await; imap.assert_read(Type::Tagged, ResponseType::Ok) .await .assert_contains("[MAILBOXID ("); @@ -100,7 +105,7 @@ pub async fn test(mut imap: &mut ImapConnection, mut imap_check: &mut ImapConnec ("Fruit/Apple", ["HasChildren", ""]), ("Fruit/Apple/Green", ["HasNoChildren", ""]), ("Vegetable", ["HasChildren", ""]), - ("Vegetable/Broccoli", ["HasNoChildren", ""]), + ("Vegetable/Broccoli", ["HasNoChildren", "\\Important"]), ("Tofu", ["HasNoChildren", ""]), ], true, diff --git a/tests/src/imap/managesieve.rs b/tests/src/imap/managesieve.rs index 40cbf8bc..597c5d69 100644 --- a/tests/src/imap/managesieve.rs +++ b/tests/src/imap/managesieve.rs @@ -24,10 +24,13 @@ use std::time::Duration; use imap_proto::ResponseType; +use mail_send::smtp::tls::build_tls_connector; +use rustls::ServerName; use tokio::{ io::{AsyncBufReadExt, AsyncWriteExt, BufReader, Lines, ReadHalf, WriteHalf}, net::TcpStream, }; +use tokio_rustls::client::TlsStream; use super::AssertResult; @@ -153,14 +156,21 @@ pub async fn test() { } pub struct SieveConnection { - reader: Lines>>, - writer: WriteHalf, + reader: Lines>>>, + writer: WriteHalf>, } impl SieveConnection { pub async fn connect() -> Self { - let (reader, writer) = - tokio::io::split(TcpStream::connect("127.0.0.1:4190").await.unwrap()); + let (reader, writer) = tokio::io::split( + build_tls_connector(true) + .connect( + ServerName::try_from("imap.example.org").unwrap(), + TcpStream::connect("127.0.0.1:4190").await.unwrap(), + ) + .await + .unwrap(), + ); SieveConnection { reader: BufReader::new(reader).lines(), writer, diff --git a/tests/src/imap/mod.rs b/tests/src/imap/mod.rs index 027243fa..50287fce 100644 --- a/tests/src/imap/mod.rs +++ b/tests/src/imap/mod.rs @@ -71,12 +71,13 @@ max-connections = 81920 bind = ["127.0.0.1:9992"] protocol = "imap" max-connections = 81920 -tls.implict = true +tls.implicit = true [server.listener.sieve] bind = ["127.0.0.1:4190"] protocol = "managesieve" max-connections = 81920 +tls.implicit = true [server.socket] reuse-addr = true @@ -305,6 +306,13 @@ async fn init_imap_tests(delete_if_exists: bool) -> IMAPTest { #[tokio::test] pub async fn imap_tests() { + /*tracing::subscriber::set_global_default( + tracing_subscriber::FmtSubscriber::builder() + .with_max_level(tracing::Level::TRACE) + .finish(), + ) + .unwrap();*/ + // Prepare settings let delete = true; let handle = init_imap_tests(delete).await; @@ -569,7 +577,7 @@ impl AssertResult for Vec { for line in &self { if let Some((_, code)) = line.split_once("[COPYUID ") { if let Some((code, _)) = code.split_once(']') { - if let Some((_, uid)) = code.split_once(' ') { + if let Some((_, uid)) = code.rsplit_once(' ') { return uid.to_string(); } } diff --git a/tests/src/imap/search.rs b/tests/src/imap/search.rs index efbef21f..ed6e9941 100644 --- a/tests/src/imap/search.rs +++ b/tests/src/imap/search.rs @@ -134,5 +134,5 @@ pub async fn test(imap: &mut ImapConnection, imap_check: &mut ImapConnection) { .await; imap.assert_read(Type::Tagged, ResponseType::Ok) .await - .assert_contains("COUNT 10 ALL 6,4:5,1,3,7:8,10,2,9"); + .assert_contains("COUNT 10 ALL 6,4:5,1,10,3,7:8,2,9"); } diff --git a/tests/src/imap/store.rs b/tests/src/imap/store.rs index ab1a734a..882553f5 100644 --- a/tests/src/imap/store.rs +++ b/tests/src/imap/store.rs @@ -60,6 +60,18 @@ pub async fn test(imap: &mut ImapConnection, _imap_check: &mut ImapConnection) { .assert_count("FLAGS", 10) .assert_count("Seen", 0); + // Check that the flags were removed + imap.send("UID FETCH 1:* (Flags)").await; + imap.assert_read(Type::Tagged, ResponseType::Ok) + .await + .assert_count("\\Seen", 0); + imap.send("STATUS INBOX (UIDNEXT MESSAGES UNSEEN)").await; + imap.assert_read(Type::Tagged, ResponseType::Ok) + .await + .assert_contains("MESSAGES 10") + .assert_contains("UNSEEN 10") + .assert_contains("UIDNEXT 11"); + // Store using saved searches imap.send("SEARCH RETURN (SAVE) FROM nathaniel").await; imap.assert_read(Type::Tagged, ResponseType::Ok).await; @@ -72,6 +84,6 @@ pub async fn test(imap: &mut ImapConnection, _imap_check: &mut ImapConnection) { imap.send("UID STORE 1:* -FLAGS (\\Answered)").await; imap.assert_read(Type::Tagged, ResponseType::Ok) .await - .assert_count("FLAGS", 10) + .assert_count("FLAGS", 3) .assert_count("Answered", 0); }