fix grpid extraction from In-Reply-To and References headers

more cleanups
make to_ids const in some places, and simplify returns from create_or_lookup_adhoc_group
2026-06-21 07:06:44 +03:00 · 2019-12-05 01:05:18 +01:00 · 2019-12-05 00:56:09 +01:00 · 2019-12-05 00:56:09 +01:00 · 2019-12-05 00:56:09 +01:00 · 2019-12-04 22:32:56 +01:00
23 changed files with 264 additions and 205 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -22,7 +22,9 @@
 - fix flakyness/sometimes-failing verified/join-protocols, 
  thanks @flub, @r10s, @hpk42

- new DC_EVENT_SECUREJOIN_SUCCEEDED event 
+- fix reply-to-encrypted message to keep encryption 
+
+- new DC_EVENT_SECUREJOIN_MEMBER_ADDED event 

 - many little fixes and rustifications (@link2xt, @flub, @hpk42)

--- a/Cargo.lock
+++ b/Cargo.lock
@@ -607,7 +607,7 @@ dependencies = [

 [[package]]
 name = "deltachat"
-version = "1.0.0-beta.8"
+version = "1.0.0-beta.9"
 dependencies = [
 "async-imap 0.1.1 (git+https://github.com/async-email/async-imap)",
 "async-std 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -682,9 +682,9 @@ dependencies = [

 [[package]]
 name = "deltachat_ffi"
-version = "1.0.0-beta.8"
+version = "1.0.0-beta.9"
 dependencies = [
- "deltachat 1.0.0-beta.8",
+ "deltachat 1.0.0-beta.9",
 "deltachat-provider-database 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
 "failure 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)",
 "human-panic 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1152,7 +1152,7 @@ dependencies = [
 "bytes 0.4.12 (registry+https://github.com/rust-lang/crates.io-index)",
 "fnv 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)",
 "futures 0.1.29 (registry+https://github.com/rust-lang/crates.io-index)",
- "http 0.1.20 (registry+https://github.com/rust-lang/crates.io-index)",
+ "http 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)",
 "indexmap 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
 "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
 "slab 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1192,7 +1192,7 @@ dependencies = [

 [[package]]
 name = "http"
-version = "0.1.20"
+version = "0.1.21"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
 "bytes 0.4.12 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1207,7 +1207,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
 "bytes 0.4.12 (registry+https://github.com/rust-lang/crates.io-index)",
 "futures 0.1.29 (registry+https://github.com/rust-lang/crates.io-index)",
- "http 0.1.20 (registry+https://github.com/rust-lang/crates.io-index)",
+ "http 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)",
 "tokio-buf 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
 ]

@@ -1249,7 +1249,7 @@ dependencies = [
 "futures 0.1.29 (registry+https://github.com/rust-lang/crates.io-index)",
 "futures-cpupool 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)",
 "h2 0.1.26 (registry+https://github.com/rust-lang/crates.io-index)",
- "http 0.1.20 (registry+https://github.com/rust-lang/crates.io-index)",
+ "http 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)",
 "http-body 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
 "httparse 1.3.4 (registry+https://github.com/rust-lang/crates.io-index)",
 "iovec 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -1323,7 +1323,7 @@ dependencies = [
 [[package]]
 name = "imap-proto"
 version = "0.9.1"
-source = "git+https://github.com/djc/tokio-imap#a26056915f1d715f97935da1f0c97c6d0174f292"
+source = "git+https://github.com/djc/tokio-imap#2b8701b83c50085b7ffcdf26a95146b91f93a6d5"
 dependencies = [
 "nom 5.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
@@ -1456,7 +1456,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
 "cc 1.0.47 (registry+https://github.com/rust-lang/crates.io-index)",
 "pkg-config 0.3.17 (registry+https://github.com/rust-lang/crates.io-index)",
- "vcpkg 0.2.7 (registry+https://github.com/rust-lang/crates.io-index)",
+ "vcpkg 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)",
 ]

 [[package]]
@@ -1803,7 +1803,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 [[package]]
 name = "pgp"
 version = "0.3.2"
-source = "git+https://github.com/rpgp/rpgp#4cc60a1e45a781ea6e7f394ae2583844ac75d214"
+source = "git+https://github.com/rpgp/rpgp#8928a249f848d46889f618fde914417f698ea76f"
 dependencies = [
 "aes 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)",
 "base64 0.10.1 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -2263,7 +2263,7 @@ dependencies = [
 "encoding_rs 0.8.20 (registry+https://github.com/rust-lang/crates.io-index)",
 "flate2 1.0.13 (registry+https://github.com/rust-lang/crates.io-index)",
 "futures 0.1.29 (registry+https://github.com/rust-lang/crates.io-index)",
- "http 0.1.20 (registry+https://github.com/rust-lang/crates.io-index)",
+ "http 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)",
 "hyper 0.12.35 (registry+https://github.com/rust-lang/crates.io-index)",
 "hyper-rustls 0.17.1 (registry+https://github.com/rust-lang/crates.io-index)",
 "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -3069,7 +3069,7 @@ dependencies = [

 [[package]]
 name = "vcpkg"
-version = "0.2.7"
+version = "0.2.8"
 source = "registry+https://github.com/rust-lang/crates.io-index"

 [[package]]
@@ -3477,7 +3477,7 @@ dependencies = [
 "checksum hermit-abi 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "307c3c9f937f38e3534b1d6447ecf090cafcc9744e4a6360e8b037b2cf5af120"
 "checksum hex 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "023b39be39e3a2da62a94feb433e91e8bcd37676fbc8bea371daf52b7a769a3e"
 "checksum hostname 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)" = "21ceb46a83a85e824ef93669c8b390009623863b5c195d1ba747292c0c72f94e"
-"checksum http 0.1.20 (registry+https://github.com/rust-lang/crates.io-index)" = "2790658cddc82e82b08e25176c431d7015a0adeb1718498715cbd20138a0bf68"
+"checksum http 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)" = "d6ccf5ede3a895d8856620237b2f02972c1bbc78d2965ad7fe8838d4a0ed41f0"
 "checksum http-body 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "6741c859c1b2463a423a1dbce98d418e6c3c3fc720fb0d45528657320920292d"
 "checksum httparse 1.3.4 (registry+https://github.com/rust-lang/crates.io-index)" = "cd179ae861f0c2e53da70d892f5f3029f9594be0c41dc5269cd371691b1dc2f9"
 "checksum human-panic 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "21638c5955a6daf3ecc42cae702335fc37a72a4abcc6959ce457b31a7d43bbdd"
@@ -3679,7 +3679,7 @@ dependencies = [
 "checksum utf8parse 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "8772a4ccbb4e89959023bc5b7cb8623a795caa7092d99f3aa9501b9484d4557d"
 "checksum uuid 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)" = "e1436e58182935dcd9ce0add9ea0b558e8a87befe01c1a301e6020aeb0876363"
 "checksum uuid 0.7.4 (registry+https://github.com/rust-lang/crates.io-index)" = "90dbc611eb48397705a6b0f6e917da23ae517e4d127123d2cf7674206627d32a"
-"checksum vcpkg 0.2.7 (registry+https://github.com/rust-lang/crates.io-index)" = "33dd455d0f96e90a75803cfeb7f948768c08d70a6de9a8d2362461935698bf95"
+"checksum vcpkg 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)" = "3fc439f2794e98976c88a2a2dafce96b930fe8010b0a256b3c2199a773933168"
 "checksum version_check 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)" = "914b1a6776c4c929a602fafd8bc742e06365d4bcbe48c30f9cca5824f70dc9dd"
 "checksum version_check 0.9.1 (registry+https://github.com/rust-lang/crates.io-index)" = "078775d0255232fb988e6fccf26ddc9d1ac274299aaedcedce21c6f72cc533ce"
 "checksum void 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "6a02e4885ed3bc0f2de90ea6dd45ebcbb66dacffe03547fadbb0eeae2770887d"
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "deltachat"
-version = "1.0.0-beta.8"
+version = "1.0.0-beta.9"
 authors = ["Delta Chat Developers (ML) <delta@codespeak.net>"]
 edition = "2018"
 license = "MPL"
--- a/deltachat-ffi/Cargo.toml
+++ b/deltachat-ffi/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "deltachat_ffi"
-version = "1.0.0-beta.8"
+version = "1.0.0-beta.9"
 description = "Deltachat FFI"
 authors = ["Delta Chat Developers (ML) <delta@codespeak.net>"]
 edition = "2018"
--- a/python/tests/test_account.py
+++ b/python/tests/test_account.py
@@ -475,27 +475,32 @@ class TestOnlineAccount:
        ac1._evlogger.get_matching("DC_EVENT_IMAP_MESSAGE_MOVED")
        ac1._evlogger.get_matching("DC_EVENT_IMAP_MESSAGE_MOVED")

-    def test_forward_messages(self, acfactory):
+    def test_forward_messages(self, acfactory, lp):
        ac1, ac2 = acfactory.get_two_online_accounts()
        chat = self.get_chat(ac1, ac2)

+        lp.sec("ac1: send message to ac2")
        msg_out = chat.send_text("message2")

-        # wait for other account to receive
+        lp.sec("ac2: wait for receive")
        ev = ac2._evlogger.get_matching("DC_EVENT_INCOMING_MSG|DC_EVENT_MSGS_CHANGED")
        assert ev[2] == msg_out.id
        msg_in = ac2.get_message_by_id(msg_out.id)
        assert msg_in.text == "message2"

-        # check the message arrived in contact-requests/deaddrop
+        lp.sec("ac2: check that the message arrive in deaddrop")
        chat2 = msg_in.chat
        assert msg_in in chat2.get_messages()
        assert not msg_in.is_forwarded()
        assert chat2.is_deaddrop()
        assert chat2 == ac2.get_deaddrop_chat()
+
+        lp.sec("ac2: create new chat and forward message to it")
        chat3 = ac2.create_group_chat("newgroup")
        assert not chat3.is_promoted()
        ac2.forward_messages([msg_in], chat3)
+
+        lp.sec("ac2: check new chat has a forwarded message")
        assert chat3.is_promoted()
        messages = chat3.get_messages()
        msg = messages[-1]
--- a/python/tests/test_increation.py
+++ b/python/tests/test_increation.py
@@ -57,13 +57,13 @@ class TestOnlineInCreation:

        lp.sec("wait1 for original or forwarded messages to arrive")
        ev1 = ac2._evlogger.get_matching("DC_EVENT_MSGS_CHANGED")
-        assert ev1[1] >= const.DC_CHAT_ID_LAST_SPECIAL
+        assert ev1[1] > const.DC_CHAT_ID_LAST_SPECIAL
        received_original = ac2.get_message_by_id(ev1[2])
        assert cmp(received_original.filename, path, False)

        lp.sec("wait2 for original or forwarded messages to arrive")
        ev2 = ac2._evlogger.get_matching("DC_EVENT_MSGS_CHANGED")
-        assert ev2[1] >= const.DC_CHAT_ID_LAST_SPECIAL
+        assert ev2[1] > const.DC_CHAT_ID_LAST_SPECIAL
        assert ev2[1] != ev1[1]
        received_copy = ac2.get_message_by_id(ev2[2])
        assert cmp(received_copy.filename, path, False)
--- a/set_core_version.py
+++ b/set_core_version.py
@@ -33,6 +33,8 @@ def replace_toml_version(relpath, newversion):
 if __name__ == "__main__":

    if len(sys.argv) < 2:
+        for x in ("Cargo.toml", "deltachat-ffi/Cargo.toml"):
+            print("{}: {}".format(x, read_toml_version(x)))
        raise SystemExit("need argument: new version, example 1.0.0-beta.27")
    newversion = sys.argv[1]
    if newversion.count(".") < 2:
--- a/src/aheader.rs
+++ b/src/aheader.rs
@@ -76,7 +76,6 @@ impl Aheader {
        if let Ok(Some(value)) = headers.get_first_value("Autocrypt") {
            match Self::from_str(&value) {
                Ok(header) => {
-                    info!(context, "comparing {} - {}", header.addr, wanted_from);
                    if addr_cmp(&header.addr, wanted_from) {
                        return Some(header);
                    }
--- a/src/chat.rs
+++ b/src/chat.rs
@@ -497,7 +497,7 @@ impl Chat {
 /// chat messages, use dc_get_chat_msgs().
 ///
 /// If the user is asked before creation, he should be
-/// asked whether he wants to chat with the _contact_ belonging to the message;
+/// asked whether he wants to chat with the *contact* belonging to the message;
 /// the group names may be really weird when taken from the subject of implicit
 /// groups and this may look confusing.
 ///
@@ -579,6 +579,10 @@ pub fn unblock(context: &Context, chat_id: u32) {
 }

 pub fn set_blocking(context: &Context, chat_id: u32, new_blocking: Blocked) -> bool {
+    if chat_id == 0 {
+        warn!(context, "ignoring setting of Block-status for chat_id=0");
+        return false;
+    }
    sql::execute(
        context,
        &context.sql,
--- a/src/chatlist.rs
+++ b/src/chatlist.rs
@@ -60,7 +60,7 @@ impl Chatlist {
    ///   or "Not now".
    ///   The UI can also offer a "Close" button that calls dc_marknoticed_contact() then.
    /// - DC_CHAT_ID_ARCHIVED_LINK (6) - this special chat is present if the user has
-    ///   archived _any_ chat using dc_archive_chat(). The UI should show a link as
+    ///   archived *any* chat using dc_archive_chat(). The UI should show a link as
    ///   "Show archived chats", if the user clicks this item, the UI should show a
    ///   list of all archived chats that can be created by this function hen using
    ///   the DC_GCL_ARCHIVED_ONLY flag.
@@ -71,7 +71,7 @@ impl Chatlist {
    /// The `listflags` is a combination of flags:
    /// - if the flag DC_GCL_ARCHIVED_ONLY is set, only archived chats are returned.
    ///   if DC_GCL_ARCHIVED_ONLY is not set, only unarchived chats are returned and
-    ///   the pseudo-chat DC_CHAT_ID_ARCHIVED_LINK is added if there are _any_ archived
+    ///   the pseudo-chat DC_CHAT_ID_ARCHIVED_LINK is added if there are *any* archived
    ///   chats
    /// - if the flag DC_GCL_NO_SPECIALS is set, deaddrop and archive link are not added
    ///   to the list (may be used eg. for selecting chats on forwarding, the flag is
--- a/src/contact.rs
+++ b/src/contact.rs
@@ -23,15 +23,17 @@ use crate::stock::StockMessage;
 const DC_ORIGIN_MIN_CONTACT_LIST: i32 = 0x100;

 /// An object representing a single contact in memory.
+///
 /// The contact object is not updated.
 /// If you want an update, you have to recreate the object.
 ///
 /// The library makes sure
 /// only to use names _authorized_ by the contact in `To:` or `Cc:`.
-/// _Given-names _as "Daddy" or "Honey" are not used there.
+/// *Given-names* as "Daddy" or "Honey" are not used there.
 /// For this purpose, internally, two names are tracked -
-/// authorized-name and given-name.
+/// authorized name and given name.
 /// By default, these names are equal, but functions working with contact names
+/// only affect the given name.
 #[derive(Debug)]
 pub struct Contact {
    /// The contact ID.
@@ -203,7 +205,7 @@ impl Contact {
    /// Add a single contact as a result of an _explicit_ user action.
    ///
    /// We assume, the contact name, if any, is entered by the user and is used "as is" therefore,
-    /// normalize() is _not_ called for the name. If the contact is blocked, it is unblocked.
+    /// normalize() is *not* called for the name. If the contact is blocked, it is unblocked.
    ///
    /// To add a number of contacts, see `dc_add_address_book()` which is much faster for adding
    /// a bunch of addresses.
@@ -233,7 +235,7 @@ impl Contact {
    }

    /// Mark all messages sent by the given contact
-    /// as _noticed_.  See also dc_marknoticed_chat() and dc_markseen_msgs()
+    /// as *noticed*.  See also dc_marknoticed_chat() and dc_markseen_msgs()
    ///
    /// Calling this function usually results in the event `#DC_EVENT_MSGS_CHANGED`.
    pub fn mark_noticed(context: &Context, id: u32) {
@@ -423,7 +425,7 @@ impl Contact {
    /// the event `DC_EVENT_CONTACTS_CHANGED` is sent.
    ///
    /// To add a single contact entered by the user, you should prefer `Contact::create`,
-    /// however, for adding a bunch of addresses, this function is _much_ faster.
+    /// however, for adding a bunch of addresses, this function is much faster.
    ///
    /// The `addr_book` is a multiline string in the format `Name one\nAddress one\nName two\nAddress two`.
    ///
--- a/src/context.rs
+++ b/src/context.rs
@@ -1,3 +1,5 @@
+//! Contacts module
+
 use std::collections::HashMap;
 use std::ffi::OsString;
 use std::path::{Path, PathBuf};
--- a/src/dc_receive_imf.rs
+++ b/src/dc_receive_imf.rs
@@ -47,11 +47,6 @@ pub fn dc_receive_imf(
        server_uid,
    );

-    // Parse the imf to mailimf_message. normally, this is done by mailimf_message_parse(),
-    // however, as we also need the MIME data,
-    // we use mailmime_parse() through dc_mimeparser (both call mailimf_struct_multiple_parse()
-    // somewhen, I did not found out anything that speaks against this approach yet)
-
    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);
@@ -210,7 +205,7 @@ pub fn dc_receive_imf(
            &mut created_db_entries,
            &mut create_event_to_send,
        ) {
-            warn!(context, "{}", err);
+            warn!(context, "add_parts error: {:?}", err);

            cleanup(
                context,
@@ -414,18 +409,18 @@ fn add_parts(
                    Blocked::Deaddrop
                };

-            create_or_lookup_group(
+            let (new_chat_id, new_chat_id_blocked) = create_or_lookup_group(
                context,
                &mut mime_parser,
                allow_creation,
                create_blocked,
                *from_id,
                to_ids,
-                chat_id,
-                &mut chat_id_blocked,
            )?;
-            if 0 != *chat_id && Blocked::Not != chat_id_blocked && create_blocked == Blocked::Not {
-                chat::unblock(context, *chat_id);
+            *chat_id = new_chat_id;
+            chat_id_blocked = new_chat_id_blocked;
+            if *chat_id != 0 && chat_id_blocked != Blocked::Not && create_blocked == Blocked::Not {
+                chat::unblock(context, new_chat_id);
                chat_id_blocked = Blocked::Not;
            }
        }
@@ -507,18 +502,19 @@ fn add_parts(
        if !to_ids.is_empty() {
            *to_id = to_ids[0];
            if *chat_id == 0 {
-                create_or_lookup_group(
+                let (new_chat_id, new_chat_id_blocked) = create_or_lookup_group(
                    context,
                    &mut mime_parser,
                    allow_creation,
                    Blocked::Not,
                    *from_id,
                    to_ids,
-                    chat_id,
-                    &mut chat_id_blocked,
                )?;
-                if 0 != *chat_id && Blocked::Not != chat_id_blocked {
-                    chat::unblock(context, *chat_id);
+                *chat_id = new_chat_id;
+                chat_id_blocked = new_chat_id_blocked;
+                // automatically unblock chat when the user sends a message
+                if *chat_id != 0 && chat_id_blocked != Blocked::Not {
+                    chat::unblock(context, new_chat_id);
                    chat_id_blocked = Blocked::Not;
                }
            }
@@ -789,7 +785,7 @@ fn calc_timestamps(
 /// - is there a group with the same recipients? if so, use this (if there are multiple, use the most recent one)
 /// - create an ad-hoc group based on the recipient list
 ///
-/// So when the function returns, the caller has the group id matching the current state of the group.
+/// on success the function returns the found/created (chat_id, chat_blocked) tuple .
 #[allow(non_snake_case)]
 fn create_or_lookup_group(
    context: &Context,
@@ -797,14 +793,9 @@ fn create_or_lookup_group(
    allow_creation: i32,
    create_blocked: Blocked,
    from_id: u32,
-    to_ids: &mut Vec<u32>,
-    ret_chat_id: *mut u32,
-    ret_chat_id_blocked: &mut Blocked,
-) -> Result<()> {
-    let group_explicitly_left: bool;
-    let mut chat_id = 0;
+    to_ids: &[u32],
+) -> Result<(u32, Blocked)> {
    let mut chat_id_blocked = Blocked::Not;
-    let mut grpid = "".to_string();
    let mut grpname = None;
    let to_ids_cnt = to_ids.len();
    let mut recreate_member_list = 0;
@@ -815,26 +806,13 @@ fn create_or_lookup_group(
    let mut X_MrGrpImageChanged = "".to_string();
    let mut better_msg: String = From::from("");

-    let cleanup = |ret_chat_id: *mut u32,
-                   ret_chat_id_blocked: &mut Blocked,
-                   chat_id: u32,
-                   chat_id_blocked: Blocked| {
-        if !ret_chat_id.is_null() {
-            unsafe { *ret_chat_id = chat_id };
-        }
-        *ret_chat_id_blocked = if 0 != chat_id {
-            chat_id_blocked
-        } else {
-            Blocked::Not
-        };
-    };
-
    if mime_parser.is_system_message == SystemMessage::LocationStreamingEnabled {
        better_msg =
            context.stock_system_msg(StockMessage::MsgLocationEnabled, "", "", from_id as u32)
    }
    set_better_msg(mime_parser, &better_msg);

+    let mut grpid = "".to_string();
    if let Some(optional_field) = mime_parser.lookup_field("Chat-Group-ID") {
        grpid = optional_field.clone();
    }
@@ -843,33 +821,26 @@ fn create_or_lookup_group(
        if let Some(value) = mime_parser.lookup_field("Message-ID") {
            if let Some(extracted_grpid) = dc_extract_grpid_from_rfc724_mid(&value) {
                grpid = extracted_grpid.to_string();
-            } else {
-                grpid = "".to_string();
            }
        }
        if grpid.is_empty() {
-            if let Some(value) = mime_parser.lookup_field("In-Reply-To") {
-                grpid = value.clone();
-            }
-            if grpid.is_empty() {
-                if let Some(value) = mime_parser.lookup_field("References") {
-                    grpid = value.clone();
-                }
-
-                if grpid.is_empty() {
-                    create_or_lookup_adhoc_group(
-                        context,
-                        mime_parser,
-                        allow_creation,
-                        create_blocked,
-                        from_id,
-                        to_ids,
-                        &mut chat_id,
-                        &mut chat_id_blocked,
-                    )?;
-                    cleanup(ret_chat_id, ret_chat_id_blocked, chat_id, chat_id_blocked);
-                    return Ok(());
-                }
+            if let Some(extracted_grpid) = get_grpid_from_list(mime_parser, "In-Reply-To") {
+                grpid = extracted_grpid;
+            } else if let Some(extracted_grpid) = get_grpid_from_list(mime_parser, "References") {
+                grpid = extracted_grpid;
+            } else {
+                return create_or_lookup_adhoc_group(
+                    context,
+                    mime_parser,
+                    allow_creation,
+                    create_blocked,
+                    from_id,
+                    to_ids,
+                )
+                .map_err(|err| {
+                    info!(context, "could not create adhoc-group: {:?}", err);
+                    err
+                });
            }
        }
    }
@@ -948,26 +919,29 @@ fn create_or_lookup_group(

    // check, if we have a chat with this group ID
    let (mut chat_id, chat_id_verified, _blocked) = chat::get_chat_id_by_grpid(context, &grpid);
-    if chat_id != 0 && chat_id_verified {
-        if let Err(err) = check_verified_properties(context, mime_parser, from_id as u32, to_ids) {
-            warn!(context, "verification problem: {}", err);
-            let s = format!("{}. See 'Info' for more details", err);
-            mime_parser.repl_msg_by_error(s);
+    if chat_id != 0 {
+        if chat_id_verified {
+            if let Err(err) =
+                check_verified_properties(context, mime_parser, from_id as u32, to_ids)
+            {
+                warn!(context, "verification problem: {}", err);
+                let s = format!("{}. See 'Info' for more details", err);
+                mime_parser.repl_msg_by_error(s);
+            }
+        }
+        // check if the sender is a member of the existing group -
+        // if not, we'll recreate the group list
+        if !chat::is_contact_in_chat(context, chat_id, from_id as u32) {
+            recreate_member_list = 1;
        }
    }

-    // check if the sender is a member of the existing group -
-    // if not, we'll recreate the group list
-    if chat_id != 0 && !chat::is_contact_in_chat(context, chat_id, from_id as u32) {
-        recreate_member_list = 1;
-    }
-
    // check if the group does not exist but should be created
-    group_explicitly_left = chat::is_group_explicitly_left(context, &grpid).unwrap_or_default();
-
+    let group_explicitly_left = chat::is_group_explicitly_left(context, &grpid).unwrap_or_default();
    let self_addr = context
        .get_config(Config::ConfiguredAddr)
        .unwrap_or_default();
+
    if chat_id == 0
            && !mime_parser.is_mailinglist_message()
            && !grpid.is_empty()
@@ -978,10 +952,8 @@ fn create_or_lookup_group(
            && (!group_explicitly_left
                || X_MrAddToGrp.is_some() && addr_cmp(&self_addr, X_MrAddToGrp.as_ref().unwrap()))
    {
-        let mut create_verified = VerifiedStatus::Unverified;
-        if mime_parser.lookup_field("Chat-Verified").is_some() {
-            create_verified = VerifiedStatus::Verified;
-
+        // group does not exist but should be created
+        let create_verified = if mime_parser.lookup_field("Chat-Verified").is_some() {
            if let Err(err) =
                check_verified_properties(context, mime_parser, from_id as u32, to_ids)
            {
@@ -989,11 +961,16 @@ fn create_or_lookup_group(
                let s = format!("{}. See 'Info' for more details", err);
                mime_parser.repl_msg_by_error(&s);
            }
+            VerifiedStatus::Verified
+        } else {
+            VerifiedStatus::Unverified
+        };
+
+        if allow_creation == 0 {
+            info!(context, "creating group forbidden by caller");
+            return Ok((0, Blocked::Not));
        }
-        if 0 == allow_creation {
-            cleanup(ret_chat_id, ret_chat_id_blocked, chat_id, chat_id_blocked);
-            return Ok(());
-        }
+
        chat_id = create_group_record(
            context,
            &grpid,
@@ -1007,9 +984,8 @@ fn create_or_lookup_group(

    // again, check chat_id
    if chat_id <= DC_CHAT_ID_LAST_SPECIAL {
-        chat_id = 0;
-        if group_explicitly_left {
-            chat_id = DC_CHAT_ID_TRASH;
+        return if group_explicitly_left {
+            Ok((DC_CHAT_ID_TRASH, chat_id_blocked))
        } else {
            create_or_lookup_adhoc_group(
                context,
@@ -1018,12 +994,12 @@ fn create_or_lookup_group(
                create_blocked,
                from_id,
                to_ids,
-                &mut chat_id,
-                &mut chat_id_blocked,
-            )?;
-        }
-        cleanup(ret_chat_id, ret_chat_id_blocked, chat_id, chat_id_blocked);
-        return Ok(());
+            )
+            .map_err(|err| {
+                warn!(context, "failed to create ad-hoc group: {:?}", err);
+                err
+            })
+        };
    }

    // execute group commands
@@ -1128,74 +1104,78 @@ fn create_or_lookup_group(
    }

    // 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.is_send_by_messenger {
-        let is_contact_cnt = chat::get_chat_contact_cnt(context, chat_id);
-        if is_contact_cnt > 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.
-            chat_id = 0;
-            create_or_lookup_adhoc_group(
-                context,
-                mime_parser,
-                allow_creation,
-                create_blocked,
-                from_id,
-                to_ids,
-                &mut chat_id,
-                &mut chat_id_blocked,
-            )?;
-        }
+    // 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.is_send_by_messenger
+        && 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
+        })?;
    }
-
-    cleanup(ret_chat_id, ret_chat_id_blocked, chat_id, chat_id_blocked);
-    Ok(())
+    Ok((chat_id, chat_id_blocked))
 }

-/// Handle groups for received messages
+/// try extract a grpid from a message-id list header value
+fn get_grpid_from_list(mime_parser: &MimeParser, header_key: &str) -> Option<String> {
+    if let Some(value) = mime_parser.lookup_field(header_key) {
+        for part in value.split(',').map(str::trim) {
+            if !part.is_empty() {
+                if let Some(extracted_grpid) = dc_extract_grpid_from_rfc724_mid(part) {
+                    return Some(extracted_grpid.to_string());
+                }
+            }
+        }
+    }
+    None
+}
+
+/// Handle groups for received messages, return chat_id/Blocked status on success
 fn create_or_lookup_adhoc_group(
    context: &Context,
    mime_parser: &MimeParser,
    allow_creation: i32,
    create_blocked: Blocked,
    from_id: u32,
-    to_ids: &mut Vec<u32>,
-    ret_chat_id: *mut u32,
-    ret_chat_id_blocked: &mut Blocked,
-) -> Result<()> {
-    // if we're here, no grpid was found, check there is an existing ad-hoc
-    // group matching the to-list or if we can create one
-    let mut chat_id = 0;
-    let mut chat_id_blocked = Blocked::Not;
+    to_ids: &[u32],
+) -> 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
+    // (we do not want to heuristically look at the likely mangled Subject)

-    let cleanup = |ret_chat_id: *mut u32,
-                   ret_chat_id_blocked: &mut Blocked,
-                   chat_id: u32,
-                   chat_id_blocked: Blocked| {
-        if !ret_chat_id.is_null() {
-            unsafe { *ret_chat_id = chat_id };
-        }
-        *ret_chat_id_blocked = chat_id_blocked;
-    };
-
-    // build member list from the given ids
-    if to_ids.is_empty() || mime_parser.is_mailinglist_message() {
-        // too few contacts or a mailinglist
-        cleanup(ret_chat_id, ret_chat_id_blocked, chat_id, chat_id_blocked);
-        return Ok(());
+    if mime_parser.is_mailinglist_message() {
+        // XXX we could parse List-* headers and actually create and
+        // manage a mailing list group, eventually
+        info!(
+            context,
+            "not creating ad-hoc group for mailing list message"
+        );
+        return Ok((0, Blocked::Not));
    }

-    let mut member_ids = to_ids.clone();
+    let mut member_ids = to_ids.to_vec();
    if !member_ids.contains(&from_id) {
        member_ids.push(from_id);
    }
    if !member_ids.contains(&DC_CONTACT_ID_SELF) {
        member_ids.push(DC_CONTACT_ID_SELF);
    }
+
    if member_ids.len() < 3 {
-        // too few contacts given
-        cleanup(ret_chat_id, ret_chat_id_blocked, chat_id, chat_id_blocked);
-        return Ok(());
+        info!(context, "not creating ad-hoc group: too few contacts");
+        return Ok((0, Blocked::Not));
    }

    let chat_ids = search_chat_ids_by_contact_ids(context, &member_ids)?;
@@ -1214,18 +1194,16 @@ fn create_or_lookup_adhoc_group(
        );

        if let Ok((id, id_blocked)) = res {
-            chat_id = id as u32;
-            chat_id_blocked = id_blocked;
            /* success, chat found */
-            cleanup(ret_chat_id, ret_chat_id_blocked, chat_id, chat_id_blocked);
-            return Ok(());
+            return Ok((id as u32, id_blocked));
        }
    }

-    if 0 == allow_creation {
-        cleanup(ret_chat_id, ret_chat_id_blocked, chat_id, chat_id_blocked);
-        return Ok(());
+    if allow_creation == 0 {
+        info!(context, "creating ad-hoc group prevented from caller");
+        return Ok((0, Blocked::Not));
    }
+
    // we do not check if the message is a reply to another group, this may result in
    // chats with unclear member list. instead we create a new group in the following lines ...

@@ -1233,10 +1211,12 @@ fn create_or_lookup_adhoc_group(
    // - there is no need to check if this group exists; otherwise we would have caught it above
    let grpid = create_adhoc_grp_id(context, &member_ids);
    if grpid.is_empty() {
-        cleanup(ret_chat_id, ret_chat_id_blocked, chat_id, chat_id_blocked);
-        return Ok(());
+        warn!(
+            context,
+            "failed to create ad-hoc grpid for {:?}", member_ids
+        );
+        return Ok((0, Blocked::Not));
    }
-
    // use subject as initial chat name
    let grpname = if let Some(subject) = mime_parser.subject.as_ref().filter(|s| !s.is_empty()) {
        subject.to_string()
@@ -1245,22 +1225,20 @@ fn create_or_lookup_adhoc_group(
    };

    // create group record
-    chat_id = create_group_record(
+    let new_chat_id = create_group_record(
        context,
        &grpid,
        grpname,
        create_blocked,
        VerifiedStatus::Unverified,
    );
-    chat_id_blocked = create_blocked;
    for &member_id in &member_ids {
-        chat::add_to_chat_contacts_table(context, chat_id, member_id);
+        chat::add_to_chat_contacts_table(context, new_chat_id, member_id);
    }

-    context.call_cb(Event::ChatModified(chat_id));
+    context.call_cb(Event::ChatModified(new_chat_id));

-    cleanup(ret_chat_id, ret_chat_id_blocked, chat_id, chat_id_blocked);
-    Ok(())
+    Ok((new_chat_id, create_blocked))
 }

 fn create_group_record(
@@ -1687,6 +1665,7 @@ fn add_or_lookup_contact_by_addr(
 #[cfg(test)]
 mod tests {
    use super::*;
+    use crate::test_utils::dummy_context;

    #[test]
    fn test_hex_hash() {
@@ -1695,4 +1674,34 @@ mod tests {
        let res = hex_hash(data);
        assert_eq!(res, "b94d27b9934d3e08");
    }
+
+    #[test]
+    fn test_grpid_simple() {
+        let context = dummy_context();
+        let raw = b"From: hello\n\
+                    Subject: outer-subject\n\
+                    In-Reply-To: <lqkjwelq123@123123>\n\
+                    References: <Gr.HcxyMARjyJy.9-uvzWPTLtV@nauta.cu>\n\
+                    \n\
+                    hello\x00";
+        let mimeparser = MimeParser::from_bytes(&context.ctx, &raw[..]).unwrap();
+        assert_eq!(get_grpid_from_list(&mimeparser, "In-Reply-To"), None);
+        let grpid = Some("HcxyMARjyJy".to_string());
+        assert_eq!(get_grpid_from_list(&mimeparser, "References"), grpid);
+    }
+
+    #[test]
+    fn test_grpid_from_multiple() {
+        let context = dummy_context();
+        let raw = b"From: hello\n\
+                    Subject: outer-subject\n\
+                    In-Reply-To: <Gr.HcxyMARjyJy.9-qweqwe@asd.net>\n\
+                    References: <qweqweqwe>, <Gr.HcxyMARjyJy.9-uvzWPTLtV@nau.ca>\n\
+                    \n\
+                    hello\x00";
+        let mimeparser = MimeParser::from_bytes(&context.ctx, &raw[..]).unwrap();
+        let grpid = Some("HcxyMARjyJy".to_string());
+        assert_eq!(get_grpid_from_list(&mimeparser, "In-Reply-To"), grpid);
+        assert_eq!(get_grpid_from_list(&mimeparser, "References"), grpid);
+    }
 }
--- a/src/dc_tools.rs
+++ b/src/dc_tools.rs
@@ -217,8 +217,11 @@ pub(crate) fn dc_create_outgoing_rfc724_mid(grpid: Option<&str>, from_addr: &str
 ///
 /// # Arguments
 ///
-/// * `mid` - A string that holds the message id
+/// * `mid` - A string that holds the message id.  Leading/Trailing <>
+/// characters are automatically stripped.
 pub(crate) fn dc_extract_grpid_from_rfc724_mid(mid: &str) -> Option<&str> {
+    let mid = mid.trim_start_matches('<').trim_end_matches('>');
+
    if mid.len() < 9 || !mid.starts_with("Gr.") {
        return None;
    }
@@ -688,6 +691,16 @@ mod tests {
        let mid = "Gr.1234567890123456.morerandom@domain.de";
        let grpid = dc_extract_grpid_from_rfc724_mid(mid);
        assert_eq!(grpid, Some("1234567890123456"));
+
+        // Should return extracted grpid for grpid with length of 11
+        let mid = "<Gr.12345678901.morerandom@domain.de>";
+        let grpid = dc_extract_grpid_from_rfc724_mid(mid);
+        assert_eq!(grpid, Some("12345678901"));
+
+        // Should return extracted grpid for grpid with length of 11
+        let mid = "<Gr.1234567890123456.morerandom@domain.de>";
+        let grpid = dc_extract_grpid_from_rfc724_mid(mid);
+        assert_eq!(grpid, Some("1234567890123456"));
    }

    #[test]
--- a/src/error.rs
+++ b/src/error.rs
@@ -1,3 +1,5 @@
+//! # Error handling
+
 use failure::Fail;
 use lettre_email::mime;

--- a/src/events.rs
+++ b/src/events.rs
@@ -1,3 +1,5 @@
+//! # Events specification
+
 use std::path::PathBuf;

 use strum::EnumProperty;
@@ -90,7 +92,7 @@ pub enum Event {
    /// However, for ongoing processes (eg. configure())
    /// or for functions that are expected to fail (eg. dc_continue_key_transfer())
    /// it might be better to delay showing these events until the function has really
-    /// failed (returned false). It should be sufficient to report only the _last_ error
+    /// failed (returned false). It should be sufficient to report only the *last* error
    /// in a messasge box then.
    ///
    /// @return
--- a/src/imex.rs
+++ b/src/imex.rs
@@ -53,7 +53,7 @@ pub enum ImexMode {
 /// For this purpose, the function creates a job that is executed in the IMAP-thread then;
 /// this requires to call dc_perform_inbox_jobs() regularly.
 ///
-/// What to do is defined by the _what_ parameter.
+/// What to do is defined by the *what* parameter.
 ///
 /// While dc_imex() returns immediately, the started job may take a while,
 /// you can stop it using dc_stop_ongoing_process(). During execution of the job,
--- a/src/job.rs
+++ b/src/job.rs
@@ -1,3 +1,8 @@
+//! # Job module
+//!
+//! This module implements a job queue maintained in the SQLite database
+//! and job types.
+
 use std::time::Duration;

 use deltachat_derive::{FromSql, ToSql};
@@ -117,6 +122,7 @@ pub struct Job {
 }

 impl Job {
+    /// Deletes the job from the database.
    fn delete(&self, context: &Context) -> bool {
        context
            .sql
@@ -124,6 +130,9 @@ impl Job {
            .is_ok()
    }

+    /// Updates the job already stored in the database.
+    ///
+    /// To add a new job, use [job_add].
    fn update(&self, context: &Context) -> bool {
        sql::execute(
            context,
@@ -893,6 +902,8 @@ fn add_smtp_job(
    Ok(())
 }

+/// Adds a job to the database, scheduling it `delay_seconds`
+/// after the current time.
 pub fn job_add(
    context: &Context,
    action: Action,
--- a/src/login_param.rs
+++ b/src/login_param.rs
@@ -1,3 +1,5 @@
+//! # Login parameters
+
 use std::borrow::Cow;
 use std::fmt;

--- a/src/lot.rs
+++ b/src/lot.rs
@@ -5,7 +5,7 @@ use deltachat_derive::{FromSql, ToSql};
 /// Lot objects are created
 /// eg. by chatlist.get_summary() or dc_msg_get_summary().
 ///
-/// _Lot_ is used in the meaning _heap_ here.
+/// *Lot* is used in the meaning *heap* here.
 #[derive(Default, Debug, Clone)]
 pub struct Lot {
    pub(crate) text1_meaning: Meaning,
--- a/src/message.rs
+++ b/src/message.rs
@@ -1,3 +1,5 @@
+//! # Messages and their identifiers
+
 use std::path::{Path, PathBuf};

 use deltachat_derive::{FromSql, ToSql};
--- a/src/mimefactory.rs
+++ b/src/mimefactory.rs
@@ -467,17 +467,6 @@ impl<'a, 'b> MimeFactory<'a, 'b> {
            encrypt_helper.should_encrypt(self.context, e2ee_guranteed, &peerstates)?;
        let is_encrypted = should_encrypt && force_plaintext == 0;

-        // Add gossip headers
-        if do_gossip {
-            for peerstate in peerstates.iter().filter_map(|(state, _)| state.as_ref()) {
-                if peerstate.peek_key(min_verified).is_some() {
-                    if let Some(header) = peerstate.render_gossip_header(min_verified) {
-                        protected_headers.push(Header::new("Autocrypt-Gossip".into(), header));
-                    }
-                }
-            }
-        }
-
        let rfc724_mid = match self.loaded {
            Loaded::Message => self.msg.rfc724_mid.clone(),
            Loaded::MDN => dc_create_outgoing_rfc724_mid(None, &self.from_addr),
@@ -489,10 +478,23 @@ impl<'a, 'b> MimeFactory<'a, 'b> {
        unprotected_headers.push(Header::new_with_value("From".into(), vec![from]).unwrap());

        let outer_message = if is_encrypted {
+            // Add gossip headers
+            if do_gossip {
+                for peerstate in peerstates.iter().filter_map(|(state, _)| state.as_ref()) {
+                    if peerstate.peek_key(min_verified).is_some() {
+                        if let Some(header) = peerstate.render_gossip_header(min_verified) {
+                            message =
+                                message.header(Header::new("Autocrypt-Gossip".into(), header));
+                        }
+                    }
+                }
+            }
+
            // Store protected headers in the inner message.
            for header in protected_headers.into_iter() {
                message = message.header(header);
            }
+
            // Set the appropriate Content-Type for the inner message.
            let mut existing_ct = message
                .get_header("Content-Type".to_string())
--- a/src/param.rs
+++ b/src/param.rs
@@ -50,8 +50,8 @@ pub enum Param {
    Error = b'L',
    /// For Messages: space-separated list of messaged IDs of forwarded copies.
    ///
-    /// This is used when a [Message] is in the
-    /// [MessageState::OutPending] state but is already forwarded.
+    /// This is used when a [crate::message::Message] is in the
+    /// [crate::message::MessageState::OutPending] state but is already forwarded.
    /// In this case the forwarded messages are written to the
    /// database and their message IDs are added to this parameter of
    /// the original message, which is also saved in the database.
Author	SHA1	Message	Date
holger krekel	6e262420de	fix grpid extraction from In-Reply-To and References headers	2019-12-05 01:05:18 +01:00
holger krekel	9b10f31fb3	more cleanups	2019-12-05 00:56:09 +01:00
holger krekel	63ad7b8d34	make to_ids const in some places, and simplify returns from create_or_lookup_adhoc_group	2019-12-05 00:56:09 +01:00
holger krekel	86baaab2e9	get rid of unsafe and indirect return values for create_or_lookup.*group	2019-12-05 00:56:09 +01:00
holger krekel	3e66d23367	make set_core_version return the versions if no args are specified	2019-12-04 22:32:56 +01:00
dignifiedquire	609b5588fa	fix(mimefactory): only send Autocrypt-Gossip headers on encrypted messages	2019-12-04 13:58:33 +01:00
pabzm	798072b8ba	Fix new event name in Changelog.	2019-12-04 13:52:02 +01:00
Alexander Krotov	d950a58613	Improve documentation	2019-12-04 10:15:40 +01:00
holger krekel	4c68e6fe41	update deps	2019-12-04 09:11:21 +01:00
holger krekel	914ce77b50	adapt changelog	2019-12-04 08:58:48 +01:00
holger krekel	d09989a4ed	fix implementation of Autocrypt to encrypt if answering to an encrypted message. Previously, if any of a chat's peers set prefer_encrypt to false (i.e. "e2ee_enabled=0" in the config, a misnomer btw) then a previously encrypted chat would drop to cleartext easily.	2019-12-04 08:57:42 +01:00
holger krekel	6999a4e3a9	streamline decrypt/encrypt logging	2019-12-04 08:57:42 +01:00