diff --git a/.circleci/config.yml b/.circleci/config.yml index 82d3fd361..7b02209b8 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -36,12 +36,6 @@ jobs: executor: default steps: - checkout - - run: - name: Update submodules - command: git submodule update --init --recursive - - run: - name: Calculate dependencies - command: cargo generate-lockfile - restore_cache: keys: - cargo-v3-{{ checksum "rust-toolchain" }}-{{ checksum "Cargo.toml" }}-{{ checksum "Cargo.lock" }}-{{ arch }} @@ -49,7 +43,6 @@ jobs: - run: rustup default $(cat rust-toolchain) - run: rustup component add --toolchain $(cat rust-toolchain) rustfmt - run: rustup component add --toolchain $(cat rust-toolchain) clippy-preview - - run: cargo update - run: cargo fetch - run: rustc +stable --version - run: rustc +$(cat rust-toolchain) --version @@ -91,7 +84,6 @@ jobs: curl https://sh.rustup.rs -sSf | sh -s -- -y - run: rustup install $(cat rust-toolchain) - run: rustup default $(cat rust-toolchain) - - run: cargo update - run: cargo fetch - run: name: Test @@ -187,7 +179,7 @@ jobs: - *restore-cache - run: name: Run cargo clippy - command: cargo clippy --all + command: cargo clippy workflows: diff --git a/Cargo.lock b/Cargo.lock index bfc1f5489..c864a0f09 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -656,12 +656,14 @@ dependencies = [ "chrono 0.4.10 (registry+https://github.com/rust-lang/crates.io-index)", "debug_stub_derive 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "deltachat_derive 0.1.0", + "email 0.0.21 (git+https://github.com/deltachat/rust-email)", "encoded-words 0.1.0 (git+https://github.com/async-email/encoded-words)", "escaper 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "failure 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)", "failure_derive 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)", "hex 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "image-meta 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + "indexmap 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "itertools 0.8.2 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "lettre_email 0.9.2 (git+https://github.com/deltachat/lettre?branch=feat/mail)", @@ -826,10 +828,11 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "email" version = "0.0.21" -source = "git+https://github.com/deltachat/rust-email#265a54a8c31355c506610c032c81112dc0953afb" +source = "git+https://github.com/deltachat/rust-email#b71c13d7d9a599ebc811a8e2ca350e2793fe58d3" dependencies = [ "base64 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)", "chrono 0.4.10 (registry+https://github.com/rust-lang/crates.io-index)", + "encoded-words 0.1.0 (git+https://github.com/async-email/encoded-words)", "encoding 0.2.33 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.7.2 (registry+https://github.com/rust-lang/crates.io-index)", diff --git a/Cargo.toml b/Cargo.toml index 1d87af162..deb1bd7b8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,6 +18,7 @@ reqwest = { version = "0.9.15", default-features = false, features = ["rustls-tl num-derive = "0.2.5" num-traits = "0.2.6" async-smtp = { git = "https://github.com/async-email/async-smtp" } +email = { git = "https://github.com/deltachat/rust-email" } lettre_email = { git = "https://github.com/deltachat/lettre", branch = "feat/mail" } async-imap = { git = "https://github.com/async-email/async-imap", branch="master" } async-tls = "0.6" @@ -30,6 +31,7 @@ serde_json = "1.0" chrono = "0.4.6" failure = "0.1.5" failure_derive = "0.1.5" +indexmap = "1.3.0" # TODO: make optional rustyline = "4.1.0" lazy_static = "1.4.0" diff --git a/appveyor.yml b/appveyor.yml index 132892d49..7276acac1 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -8,7 +8,6 @@ install: - set PATH=%PATH%;%USERPROFILE%\.cargo\bin - rustc -vV - cargo -vV - - cargo update build: false diff --git a/examples/repl/cmdline.rs b/examples/repl/cmdline.rs index dfdac887b..dc3a8b6f6 100644 --- a/examples/repl/cmdline.rs +++ b/examples/repl/cmdline.rs @@ -94,7 +94,9 @@ pub fn dc_reset_tables(context: &Context, bits: i32) -> i32 { fn dc_poke_eml_file(context: &Context, filename: impl AsRef) -> Result<(), Error> { let data = dc_read_file(context, filename)?; - dc_receive_imf(context, &data, "import", 0, 0); + if let Err(err) = dc_receive_imf(context, &data, "import", 0, 0) { + println!("dc_receive_imf errored: {:?}", err); + } Ok(()) } diff --git a/python/tests/conftest.py b/python/tests/conftest.py index f0fec7c7c..05896cb8c 100644 --- a/python/tests/conftest.py +++ b/python/tests/conftest.py @@ -85,16 +85,21 @@ class SessionLiveConfigFromFile: class SessionLiveConfigFromURL: def __init__(self, url, create_token): self.configlist = [] - for i in range(2): - res = requests.post(url, json={"token_create_user": int(create_token)}) + self.url = url + self.create_token = create_token + + def get(self, index): + try: + return self.configlist[index] + except IndexError: + assert index == len(self.configlist), index + res = requests.post(self.url, json={"token_create_user": int(self.create_token)}) if res.status_code != 200: pytest.skip("creating newtmpuser failed {!r}".format(res)) d = res.json() config = dict(addr=d["email"], mail_pw=d["password"]) self.configlist.append(config) - - def get(self, index): - return self.configlist[index] + return config def exists(self): return bool(self.configlist) diff --git a/python/tests/test_account.py b/python/tests/test_account.py index cc6db441f..d036c772f 100644 --- a/python/tests/test_account.py +++ b/python/tests/test_account.py @@ -382,6 +382,23 @@ class TestOfflineChat: assert not res.is_ask_verifygroup() assert res.contact_id == 10 + def test_group_chat_many_members_add_remove(self, ac1, lp): + lp.sec("ac1: creating group chat with 10 other members") + chat = ac1.create_group_chat(name="title1") + contacts = [] + for i in range(10): + contact = ac1.create_contact("some{}@example.org".format(i)) + contacts.append(contact) + chat.add_contact(contact) + + num_contacts = len(chat.get_contacts()) + assert num_contacts == 11 + + lp.sec("ac1: removing two contacts and checking things are right") + chat.remove_contact(contacts[9]) + chat.remove_contact(contacts[3]) + assert len(chat.get_contacts()) == 9 + class TestOnlineAccount: def get_chat(self, ac1, ac2, both_created=False): @@ -615,6 +632,9 @@ class TestOnlineAccount: def test_send_and_receive_message_markseen(self, acfactory, lp): ac1, ac2 = acfactory.get_two_online_accounts() + # make DC's life harder wrt to encodings + ac1.set_config("displayname", "ä name") + lp.sec("ac1: create chat with ac2") chat = self.get_chat(ac1, ac2) @@ -632,6 +652,7 @@ class TestOnlineAccount: msg_in = ac2.get_message_by_id(msg_out.id) assert msg_in.text == "message1" assert not msg_in.is_forwarded() + assert msg_in.get_sender_contact().display_name == ac1.get_config("displayname") lp.sec("check the message arrived in contact-requets/deaddrop") chat2 = msg_in.chat @@ -1057,6 +1078,52 @@ class TestOnlineAccount: assert not locations3 +class TestGroupStressTests: + def test_group_many_members_add_leave_remove(self, acfactory, lp): + lp.sec("creating and configuring five accounts") + ac1 = acfactory.get_online_configuring_account() + accounts = [acfactory.get_online_configuring_account() for i in range(3)] + wait_configuration_progress(ac1, 1000) + for acc in accounts: + wait_configuration_progress(acc, 1000) + + lp.sec("ac1: creating group chat with 3 other members") + chat = ac1.create_group_chat("title1") + contacts = [] + chars = list("äöüsr") + for acc in accounts: + contact = ac1.create_contact(acc.get_config("addr"), name=chars.pop()) + contacts.append(contact) + chat.add_contact(contact) + # make sure the other side accepts our messages + c1 = acc.create_contact(ac1.get_config("addr"), "ä member") + acc.create_chat_by_contact(c1) + + assert not chat.is_promoted() + + lp.sec("ac1: send mesage to new group chat") + chat.send_text("hello") + assert chat.is_promoted() + + num_contacts = len(chat.get_contacts()) + assert num_contacts == 3 + 1 + + lp.sec("ac2: checking that the chat arrived correctly") + ac2 = accounts[0] + msg = ac2.wait_next_incoming_message() + assert msg.text == "hello" + print("chat is", msg.chat) + assert len(msg.chat.get_contacts()) == 4 + + lp.sec("ac1: removing one contacts and checking things are right") + to_remove = msg.chat.get_contacts()[-1] + msg.chat.remove_contact(to_remove) + + sysmsg = ac1.wait_next_incoming_message() + assert to_remove.addr in sysmsg.text + assert len(sysmsg.chat.get_contacts()) == 3 + + class TestOnlineConfigureFails: def test_invalid_password(self, acfactory): ac1, configdict = acfactory.get_online_config() diff --git a/src/contact.rs b/src/contact.rs index 00f371ce7..4612b3f1b 100644 --- a/src/contact.rs +++ b/src/contact.rs @@ -10,7 +10,7 @@ use crate::constants::*; use crate::context::Context; use crate::dc_tools::*; use crate::e2ee; -use crate::error::Result; +use crate::error::{Error, Result}; use crate::events::Event; use crate::key::*; use crate::login_param::LoginParam; @@ -1021,6 +1021,18 @@ fn cat_fingerprint( } } +impl Context { + /// determine whether the specified addr maps to the/a self addr + pub fn is_self_addr(&self, addr: &str) -> Result { + let self_addr = match self.get_config(Config::ConfiguredAddr) { + Some(s) => s, + None => return Err(Error::NotConfigured), + }; + + Ok(addr_cmp(self_addr, addr)) + } +} + pub fn addr_cmp(addr1: impl AsRef, addr2: impl AsRef) -> bool { let norm1 = addr_normalize(addr1.as_ref()).to_lowercase(); let norm2 = addr_normalize(addr2.as_ref()).to_lowercase(); @@ -1028,15 +1040,6 @@ pub fn addr_cmp(addr1: impl AsRef, addr2: impl AsRef) -> bool { norm1 == norm2 } -pub fn addr_equals_self(context: &Context, addr: impl AsRef) -> bool { - if !addr.as_ref().is_empty() { - if let Some(self_addr) = context.get_config(Config::ConfiguredAddr) { - return addr_cmp(addr, self_addr); - } - } - false -} - fn split_address_book(book: &str) -> Vec<(&str, &str)> { book.lines() .chunks(2) @@ -1120,6 +1123,18 @@ mod tests { assert_eq!(contacts.len(), 0); } + #[test] + fn test_is_self_addr() -> Result<()> { + let t = test_context(None); + assert!(t.ctx.is_self_addr("me@me.org").is_err()); + + let addr = configure_alice_keypair(&t.ctx); + assert_eq!(t.ctx.is_self_addr("me@me.org")?, false); + assert_eq!(t.ctx.is_self_addr(&addr)?, true); + + Ok(()) + } + #[test] fn test_add_or_lookup() { // add some contacts, this also tests add_address_book() diff --git a/src/dc_receive_imf.rs b/src/dc_receive_imf.rs index 072732c4a..2b5e90393 100644 --- a/src/dc_receive_imf.rs +++ b/src/dc_receive_imf.rs @@ -23,6 +23,9 @@ use crate::securejoin::handle_securejoin_handshake; use crate::sql; use crate::stock::StockMessage; +// IndexSet is like HashSet but maintains order of insertion +type ContactIds = indexmap::IndexSet; + #[derive(Debug, PartialEq, Eq)] enum CreateEvent { MsgsChanged, @@ -36,7 +39,7 @@ pub fn dc_receive_imf( server_folder: impl AsRef, server_uid: u32, flags: u32, -) { +) -> Result<()> { info!( context, "Receiving message {}/{}...", @@ -53,26 +56,14 @@ pub fn dc_receive_imf( println!("{}", String::from_utf8_lossy(imf_raw)); } - let mime_parser = MimeParser::from_bytes(context, imf_raw); - let mut mime_parser = if let Err(err) = mime_parser { - warn!(context, "dc_receive_imf parse error: {}", err); - return; - } else { - mime_parser.unwrap() - }; + let mut mime_parser = MimeParser::from_bytes(context, imf_raw)?; - if mime_parser.get(HeaderDef::From_).is_none() { - // Error - even adding an empty record won't help as we do not know the sender - warn!(context, "No From header."); - return; - } + // we can not add even an empty record if we have no info whatsoever + ensure!(mime_parser.has_headers(), "No Headers Found"); // the function returns the number of created messages in the database let mut incoming = true; let mut incoming_origin = Origin::Unknown; - let mut to_self = false; - let mut from_id = 0u32; - let mut from_id_blocked = false; let mut to_id = 0u32; let mut chat_id = 0; let mut hidden = false; @@ -85,7 +76,7 @@ pub fn dc_receive_imf( let mut create_event_to_send = Some(CreateEvent::MsgsChanged); let mut rr_event_to_send = Vec::new(); - let mut to_ids = Vec::with_capacity(16); + let mut to_ids = ContactIds::new(); // helper method to handle early exit and memory cleanup let cleanup = |context: &Context, @@ -120,33 +111,6 @@ pub fn dc_receive_imf( sent_timestamp = mailparse::dateparse(value).unwrap_or_default(); } - // get From: and check if it is known (for known From:'s we add the other To:/Cc: in the 3rd pass) - // or if From: is equal to SELF (in this case, it is any outgoing messages, - // we do not check Return-Path any more as this is unreliable, see issue #150 - if let Some(field_from) = mime_parser.get(HeaderDef::From_) { - let mut check_self = false; - let mut from_list = Vec::with_capacity(16); - dc_add_or_lookup_contacts_by_address_list( - context, - &field_from, - Origin::IncomingUnknownFrom, - &mut from_list, - &mut check_self, - ); - if check_self { - incoming = false; - if mime_parser.sender_equals_recipient() { - from_id = DC_CONTACT_ID_SELF; - } - } else if !from_list.is_empty() { - // if there is no from given, from_id stays 0 which is just fine. These messages - // are very rare, however, we have to add them to the database (they go to the - // "deaddrop" chat) to avoid a re-download from the server. See also [**] - from_id = from_list[0]; - incoming_origin = Contact::get_origin_by_id(context, from_id, &mut from_id_blocked) - } - } - // Make sure, to_ids starts with the first To:-address (Cc: is added in the loop below pass) if let Some(field) = mime_parser.get(HeaderDef::To) { dc_add_or_lookup_contacts_by_address_list( @@ -160,8 +124,44 @@ pub fn dc_receive_imf( Origin::IncomingUnknownTo }, &mut to_ids, - &mut to_self, - ); + )?; + } + + // get From: (it can be an address list!) and check if it is known (for known From:'s we add + // the other To:/Cc: in the 3rd pass) + // or if From: is equal to SELF (in this case, it is any outgoing messages, + // we do not check Return-Path any more as this is unreliable, see issue #150) + let mut from_id = 0; + let mut from_id_blocked = false; + + if let Some(field_from) = mime_parser.get(HeaderDef::From_) { + let mut from_ids = ContactIds::new(); + dc_add_or_lookup_contacts_by_address_list( + context, + &field_from, + Origin::IncomingUnknownFrom, + &mut from_ids, + )?; + if from_ids.len() > 1 { + warn!( + context, + "mail has more than one address in From: {:?}", field_from + ); + } + if from_ids.contains(&DC_CONTACT_ID_SELF) { + incoming = false; + if to_ids.len() == 1 && to_ids.contains(&DC_CONTACT_ID_SELF) { + from_id = DC_CONTACT_ID_SELF; + } + } else if !from_ids.is_empty() { + from_id = from_ids.get_index(0).cloned().unwrap_or_default(); + incoming_origin = Contact::get_origin_by_id(context, from_id, &mut from_id_blocked) + } else { + warn!(context, "mail has an empty From header: {:?}", field_from); + // if there is no from given, from_id stays 0 which is just fine. These messages + // are very rare, however, we have to add them to the database (they go to the + // "deaddrop" chat) to avoid a re-download from the server. See also [**] + } } // Add parts @@ -175,14 +175,7 @@ pub fn dc_receive_imf( match dc_create_incoming_rfc724_mid(sent_timestamp, from_id, &to_ids) { Some(x) => x, None => { - error!(context, "can not create incoming rfc724_mid"); - cleanup( - context, - &create_event_to_send, - &created_db_entries, - &rr_event_to_send, - ); - return; + bail!("No Message-Id found and could not create incoming rfc724_mid"); } } } @@ -206,20 +199,17 @@ pub fn dc_receive_imf( &mut to_id, flags, &mut needs_delete_job, - to_self, &mut insert_msg_id, &mut created_db_entries, &mut create_event_to_send, ) { - warn!(context, "add_parts error: {:?}", err); - cleanup( context, &create_event_to_send, &created_db_entries, &rr_event_to_send, ); - return; + bail!("add_parts error: {:?}", err); } } else { // there are no non-meta data in message, do some basic calculations so that the varaiables @@ -272,6 +262,8 @@ pub fn dc_receive_imf( &created_db_entries, &rr_event_to_send, ); + + Ok(()) } fn add_parts( @@ -282,7 +274,7 @@ fn add_parts( incoming_origin: &mut Origin, server_folder: impl AsRef, server_uid: u32, - to_ids: &mut Vec, + to_ids: &mut ContactIds, rfc724_mid: &str, sent_timestamp: &mut i64, from_id: &mut u32, @@ -292,7 +284,6 @@ fn add_parts( to_id: &mut u32, flags: u32, needs_delete_job: &mut bool, - to_self: bool, insert_msg_id: &mut MsgId, created_db_entries: &mut Vec<(usize, MsgId)>, create_event_to_send: &mut Option, @@ -320,8 +311,7 @@ fn add_parts( Origin::IncomingUnknownCc }, to_ids, - &mut false, - ); + )?; } // check, if the mail is already in our database - if so, just update the folder/uid @@ -462,7 +452,7 @@ fn add_parts( chat::unblock(context, *chat_id); chat_id_blocked = Blocked::Not; } else if is_reply_to_known_message(context, mime_parser) { - // we do not want any chat to be created implicitly. Because of the origin-scale-up, + // we do not want any chat to be created implicitly. Because of the origin-scale-up, // the contact requests will pop up and this should be just fine. Contact::scaleup_origin_by_id(context, *from_id, Origin::IncomingReplyTo); info!( @@ -506,7 +496,7 @@ fn add_parts( state = MessageState::OutDelivered; *from_id = DC_CONTACT_ID_SELF; if !to_ids.is_empty() { - *to_id = to_ids[0]; + *to_id = to_ids.get_index(0).cloned().unwrap_or_default(); if *chat_id == 0 { let (new_chat_id, new_chat_id_blocked) = create_or_lookup_group( context, @@ -545,7 +535,11 @@ fn add_parts( } } } - if *chat_id == 0 && to_ids.is_empty() && to_self { + let self_sent = *from_id == DC_CONTACT_ID_SELF + && to_ids.len() == 1 + && to_ids.contains(&DC_CONTACT_ID_SELF); + + if *chat_id == 0 && self_sent { // from_id==to_id==DC_CONTACT_ID_SELF - this is a self-sent messages, // maybe an Autocrypt Setup Messag let (id, bl) = @@ -795,10 +789,9 @@ fn create_or_lookup_group( allow_creation: bool, create_blocked: Blocked, from_id: u32, - to_ids: &[u32], + to_ids: &ContactIds, ) -> Result<(u32, Blocked)> { let mut chat_id_blocked = Blocked::Not; - let to_ids_cnt = to_ids.len(); let mut recreate_member_list = false; let mut send_EVENT_CHAT_MODIFIED = false; let mut X_MrRemoveFromGrp = None; @@ -809,9 +802,9 @@ fn create_or_lookup_group( if mime_parser.is_system_message == SystemMessage::LocationStreamingEnabled { better_msg = - context.stock_system_msg(StockMessage::MsgLocationEnabled, "", "", from_id as u32) + context.stock_system_msg(StockMessage::MsgLocationEnabled, "", "", from_id as u32); + set_better_msg(mime_parser, &better_msg); } - set_better_msg(mime_parser, &better_msg); let mut grpid = "".to_string(); if let Some(optional_field) = mime_parser.get(HeaderDef::ChatGroupId) { @@ -846,6 +839,8 @@ fn create_or_lookup_group( } } } + // now we have a grpid that is non-empty + // but we might not know about this group let grpname = mime_parser.get(HeaderDef::ChatGroupName).cloned(); @@ -999,6 +994,18 @@ fn create_or_lookup_group( }; } + // We have a valid chat_id > DC_CHAT_ID_LAST_SPECIAL. + // + // However, it's possible that we got a non-DC message + // and the user hit "reply" instead of "reply-all". + // We heuristically detect this case and show + // a placeholder-system-message to warn about this + // and refer to "message-info" to see the message. + // This is similar to how we show messages arriving + // in verified chat using an un-verified key or cleartext. + + // XXX insert code in a different PR :) + // execute group commands if X_MrAddToGrp.is_some() || X_MrRemoveFromGrp.is_some() { recreate_member_list = true; @@ -1086,6 +1093,7 @@ fn create_or_lookup_group( chat::add_to_chat_contacts_table(context, chat_id, from_id as u32); } for &to_id in to_ids.iter() { + info!(context, "adding to={:?} to chat id={}", to_id, chat_id); if !Contact::addr_equals_contact(context, &self_addr, to_id) && (skip.is_none() || !Contact::addr_equals_contact(context, skip.unwrap(), to_id)) { @@ -1099,29 +1107,6 @@ fn create_or_lookup_group( if send_EVENT_CHAT_MODIFIED { context.call_cb(Event::ChatModified(chat_id)); } - - // check the number of receivers - - // the only critical situation is if the user hits "Reply" instead - // of "Reply all" in a non-messenger-client */ - if to_ids_cnt == 1 - && !mime_parser.has_chat_version() - && chat::get_chat_contact_cnt(context, chat_id) > 3 - { - // to_ids_cnt==1 may be "From: A, To: B, SELF" as SELF is not counted in to_ids_cnt. - // So everything up to 3 is no error. - create_or_lookup_adhoc_group( - context, - mime_parser, - allow_creation, - create_blocked, - from_id, - to_ids, - ) - .map_err(|err| { - warn!(context, "could not create ad-hoc group: {:?}", err); - err - })?; - } Ok((chat_id, chat_id_blocked)) } @@ -1146,7 +1131,7 @@ fn create_or_lookup_adhoc_group( allow_creation: bool, create_blocked: Blocked, from_id: u32, - to_ids: &[u32], + to_ids: &ContactIds, ) -> Result<(u32, Blocked)> { // if we're here, no grpid was found, check if there is an existing // ad-hoc group matching the to-list or if we should and can create one @@ -1162,7 +1147,7 @@ fn create_or_lookup_adhoc_group( return Ok((0, Blocked::Not)); } - let mut member_ids = to_ids.to_vec(); + let mut member_ids: Vec = to_ids.iter().copied().collect(); if !member_ids.contains(&from_id) { member_ids.push(from_id); } @@ -1381,7 +1366,7 @@ fn check_verified_properties( context: &Context, mimeparser: &MimeParser, from_id: u32, - to_ids: &[u32], + to_ids: &ContactIds, ) -> Result<()> { let contact = Contact::load_from_db(context, from_id)?; @@ -1412,13 +1397,20 @@ fn check_verified_properties( } } + // we do not need to check if we are verified with ourself + let mut to_ids = to_ids.clone(); + to_ids.remove(&DC_CONTACT_ID_SELF); + + if to_ids.is_empty() { + return Ok(()); + } let to_ids_str = join(to_ids.iter().map(|x| x.to_string()), ","); let rows = context.sql.query_map( format!( "SELECT c.addr, LENGTH(ps.verified_key_fingerprint) FROM contacts c \ LEFT JOIN acpeerstates ps ON c.addr=ps.addr WHERE c.id IN({}) ", - to_ids_str, + to_ids_str ), params![], |row| Ok((row.get::<_, String>(0)?, row.get::<_, i32>(1).unwrap_or(0))), @@ -1429,6 +1421,12 @@ fn check_verified_properties( )?; for (to_addr, _is_verified) in rows.into_iter() { + info!( + context, + "check_verified_properties: {:?} self={:?}", + to_addr, + context.is_self_addr(&to_addr) + ); let mut is_verified = _is_verified != 0; let mut peerstate = Peerstate::from_addr(context, &context.sql, &to_addr); @@ -1583,39 +1581,43 @@ fn dc_add_or_lookup_contacts_by_address_list( context: &Context, addr_list_raw: &str, origin: Origin, - ids: &mut Vec, - check_self: &mut bool, -) { - let addrs = mailparse::addrparse(addr_list_raw); - if addrs.is_err() { - return; - } - for addr in addrs.unwrap().iter() { + to_ids: &mut ContactIds, +) -> Result<()> { + let addrs = match mailparse::addrparse(addr_list_raw) { + Ok(addrs) => addrs, + Err(err) => { + bail!("could not parse {:?}: {:?}", addr_list_raw, err); + } + }; + + info!( + context, + "dc_add_or_lookup_contacts_by_address raw={:?} addrs={:?}", addr_list_raw, addrs + ); + for addr in addrs.iter() { match addr { mailparse::MailAddr::Single(info) => { - add_or_lookup_contact_by_addr( + to_ids.insert(add_or_lookup_contact_by_addr( context, &info.display_name, &info.addr, origin, - ids, - check_self, - ); + )?); } mailparse::MailAddr::Group(infos) => { for info in &infos.addrs { - add_or_lookup_contact_by_addr( + to_ids.insert(add_or_lookup_contact_by_addr( context, &info.display_name, &info.addr, origin, - ids, - check_self, - ); + )?); } } } } + + Ok(()) } /// Add contacts to database on receiving messages. @@ -1624,36 +1626,40 @@ fn add_or_lookup_contact_by_addr( display_name: &Option, addr: &str, origin: Origin, - ids: &mut Vec, - check_self: &mut bool, -) { - // is addr_spec equal to SELF? - let self_addr = context - .get_config(Config::ConfiguredAddr) - .unwrap_or_default(); - - if addr_cmp(self_addr, addr) { - *check_self = true; +) -> Result { + if context.is_self_addr(addr)? { + return Ok(DC_CONTACT_ID_SELF); } - - if *check_self { - return; - } - - // add addr_spec if missing, update otherwise let display_name_normalized = display_name .as_ref() .map(normalize_name) .unwrap_or_default(); - // can be NULL - let row_id = Contact::add_or_lookup(context, display_name_normalized, addr, origin) - .map(|(id, _)| id) - .unwrap_or_default(); + info!( + context, + "looking up addr={:?} display_name={:?}", addr, display_name_normalized + ); + let (row_id, _modified) = + Contact::add_or_lookup(context, display_name_normalized, addr, origin)?; + ensure!(row_id > 0, "could not add contact: {:?}", addr); - if 0 != row_id && !ids.contains(&row_id) { - ids.push(row_id); - }; + Ok(row_id) +} + +fn dc_create_incoming_rfc724_mid( + message_timestamp: i64, + contact_id_from: u32, + contact_ids_to: &ContactIds, +) -> Option { + /* create a deterministic rfc724_mid from input such that + repeatedly calling it with the same input results in the same Message-id */ + + let largest_id_to = contact_ids_to.iter().max().copied().unwrap_or_default(); + let result = format!( + "{}-{}-{}@stub", + message_timestamp, contact_id_from, largest_id_to + ); + Some(result) } #[cfg(test)] @@ -1698,4 +1704,24 @@ mod tests { assert_eq!(extract_grpid(&mimeparser, HeaderDef::InReplyTo), grpid); assert_eq!(extract_grpid(&mimeparser, HeaderDef::References), grpid); } + + #[test] + fn test_dc_create_incoming_rfc724_mid() { + let mut members = ContactIds::new(); + assert_eq!( + dc_create_incoming_rfc724_mid(123, 45, &members), + Some("123-45-0@stub".into()) + ); + members.insert(7); + members.insert(3); + assert_eq!( + dc_create_incoming_rfc724_mid(123, 45, &members), + Some("123-45-7@stub".into()) + ); + members.insert(9); + assert_eq!( + dc_create_incoming_rfc724_mid(123, 45, &members), + Some("123-45-9@stub".into()) + ); + } } diff --git a/src/dc_tools.rs b/src/dc_tools.rs index 493821865..c6067ad7c 100644 --- a/src/dc_tools.rs +++ b/src/dc_tools.rs @@ -182,22 +182,6 @@ fn encode_66bits_as_base64(v1: u32, v2: u32, fill: u32) -> String { String::from_utf8(wrapped_writer).unwrap() } -pub(crate) fn dc_create_incoming_rfc724_mid( - message_timestamp: i64, - contact_id_from: u32, - contact_ids_to: &[u32], -) -> Option { - /* create a deterministic rfc724_mid from input such that - repeatedly calling it with the same input results in the same Message-id */ - - let largest_id_to = contact_ids_to.iter().max().copied().unwrap_or_default(); - let result = format!( - "{}-{}-{}@stub", - message_timestamp, contact_id_from, largest_id_to - ); - Some(result) -} - /// Function generates a Message-ID that can be used for a new outgoing message. /// - this function is called for all outgoing messages. /// - the message ID should be globally unique @@ -780,14 +764,6 @@ mod tests { } } - #[test] - fn test_dc_create_incoming_rfc724_mid() { - let res = dc_create_incoming_rfc724_mid(123, 45, &[6, 7]); - assert_eq!(res, Some("123-45-7@stub".into())); - let res = dc_create_incoming_rfc724_mid(123, 45, &[]); - assert_eq!(res, Some("123-45-0@stub".into())); - } - #[test] fn test_file_get_safe_basename() { assert_eq!(get_safe_basename("12312/hello"), "hello"); diff --git a/src/error.rs b/src/error.rs index 6f1139094..5f0558f48 100644 --- a/src/error.rs +++ b/src/error.rs @@ -28,12 +28,14 @@ pub enum Error { InvalidMsgId, #[fail(display = "Watch folder not found {:?}", _0)] WatchFolderNotFound(String), - #[fail(display = "Inalid Email: {:?}", _0)] + #[fail(display = "Invalid Email: {:?}", _0)] MailParseError(#[cause] mailparse::MailParseError), #[fail(display = "Building invalid Email: {:?}", _0)] LettreError(#[cause] lettre_email::error::Error), #[fail(display = "FromStr error: {:?}", _0)] FromStr(#[cause] mime::FromStrError), + #[fail(display = "Not Configured")] + NotConfigured, } pub type Result = std::result::Result; diff --git a/src/imap/mod.rs b/src/imap/mod.rs index eb7bf5f88..62f36cf00 100644 --- a/src/imap/mod.rs +++ b/src/imap/mod.rs @@ -7,7 +7,7 @@ use std::sync::atomic::{AtomicBool, Ordering}; use async_imap::{ error::Result as ImapResult, - types::{Fetch, Flag, Mailbox, Name, NameAttribute}, + types::{Capability, Fetch, Flag, Mailbox, Name, NameAttribute}, }; use async_std::sync::{Mutex, RwLock}; use async_std::task; @@ -389,9 +389,14 @@ impl Imap { } else { let can_idle = caps.has_str("IDLE"); let has_xlist = caps.has_str("XLIST"); - let caps_list = caps - .iter() - .fold(String::new(), |s, c| s + &format!(" {:?}", c)); + let caps_list = caps.iter().fold(String::new(), |s, c| { + if let Capability::Atom(x) = c { + s + &format!(" {}", x) + } else { + s + &format!(" {:?}", c) + } + }); + self.config.write().await.can_idle = can_idle; self.config.write().await.has_xlist = has_xlist; *self.connected.lock().await = true; @@ -713,7 +718,17 @@ impl Imap { if !is_deleted && msg.body().is_some() { let body = msg.body().unwrap_or_default(); - dc_receive_imf(context, &body, folder.as_ref(), server_uid, flags as u32); + if let Err(err) = + dc_receive_imf(context, &body, folder.as_ref(), server_uid, flags as u32) + { + warn!( + context, + "dc_receive_imf failed for imap-message {}/{}: {:?}", + folder.as_ref(), + server_uid, + err + ); + } } } diff --git a/src/mimefactory.rs b/src/mimefactory.rs index d047d9a40..fd441ccbd 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -382,7 +382,7 @@ impl<'a, 'b> MimeFactory<'a, 'b> { let mut unprotected_headers: Vec
= Vec::new(); let from = Address::new_mailbox_with_name( - encode_words(&self.from_displayname), + self.from_displayname.to_string(), self.from_addr.clone(), ); @@ -394,7 +394,7 @@ impl<'a, 'b> MimeFactory<'a, 'b> { to.push(Address::new_mailbox(addr.clone())); } else { to.push(Address::new_mailbox_with_name( - encode_words(name), + name.to_string(), addr.clone(), )); } @@ -1051,3 +1051,25 @@ pub fn needs_encoding(to_check: impl AsRef) -> bool { !c.is_ascii_alphanumeric() && c != '-' && c != '_' && c != '.' && c != '~' && c != '%' }) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_render_email_address() { + let display_name = "ä space"; + let addr = "x@y.org"; + + assert!(!display_name.is_ascii()); + + let s = format!( + "{}", + Address::new_mailbox_with_name(display_name.to_string(), addr.to_string()) + ); + + println!("{}", s); + + assert_eq!(s, "=?utf-8?q?=C3=A4_space?= "); + } +} diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 171717073..ab3073cee 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -338,6 +338,10 @@ impl<'a> MimeParser<'a> { self.header.contains_key("chat-version") } + pub(crate) fn has_headers(&self) -> bool { + !self.header.is_empty() + } + pub(crate) fn get_subject(&self) -> Option { if let Some(s) = self.get(HeaderDef::Subject) { if s.is_empty() { @@ -523,7 +527,7 @@ impl<'a> MimeParser<'a> { let filename = get_attachment_filename(mail); info!( self.context, - "add_single_part_if_known {:?} {:?} {:?}", mime_type, msg_type, filename + "add_single_part_if_known {:?} {:?}", mime_type, msg_type ); let old_part_count = self.parts.len(); @@ -661,25 +665,6 @@ impl<'a> MimeParser<'a> { } } - pub fn sender_equals_recipient(&self) -> bool { - /* get From: and check there is exactly one sender */ - if let Some(field) = self.get(HeaderDef::From_) { - if let Ok(addrs) = mailparse::addrparse(field) { - if addrs.len() != 1 { - return false; - } - if let mailparse::MailAddr::Single(ref info) = addrs[0] { - let from_addr_norm = addr_normalize(&info.addr); - let recipients = get_recipients(self.header.iter()); - if recipients.len() == 1 && recipients.contains(from_addr_norm) { - return true; - } - } - } - } - false - } - pub fn repl_msg_by_error(&mut self, error_msg: impl AsRef) { if self.parts.is_empty() { return; diff --git a/src/securejoin.rs b/src/securejoin.rs index 26624958b..2c1bb89b2 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -604,7 +604,7 @@ pub(crate) fn handle_securejoin_handshake( .get(HeaderDef::ChatGroupMemberAdded) .map(|s| s.as_str()) .unwrap_or_else(|| ""); - if join_vg && !addr_equals_self(context, cg_member_added) { + if join_vg && !context.is_self_addr(cg_member_added)? { info!(context, "Message belongs to a different handshake (scaled up contact anyway to allow creation of group)."); return Ok(ret); }