fix: trash message about group name change from non-member

src/net/proxy.rs

@@ -19,6 +19,7 @@ use tokio_io_timeout::TimeoutStream;
 use url::Url;
 
 use crate::config::Config;
+use crate::constants::NON_ALPHANUMERIC_WITHOUT_DOT;
 use crate::context::Context;
 use crate::net::connect_tcp;
 use crate::net::session::SessionStream;
@@ -92,12 +93,13 @@ impl HttpConfig {
     }
 
     fn to_url(&self, scheme: &str) -> String {
+        let host = utf8_percent_encode(&self.host, NON_ALPHANUMERIC_WITHOUT_DOT);
         if let Some((user, password)) = &self.user_password {
             let user = utf8_percent_encode(user, NON_ALPHANUMERIC);
             let password = utf8_percent_encode(password, NON_ALPHANUMERIC);
-            format!("{scheme}://{user}:{password}@{}:{}", self.host, self.port)
+            format!("{scheme}://{user}:{password}@{host}:{}", self.port)
         } else {
-            format!("{scheme}://{}:{}", self.host, self.port)
+            format!("{scheme}://{host}:{}", self.port)
         }
     }
 }
@@ -141,12 +143,13 @@ impl Socks5Config {
     }
 
     fn to_url(&self) -> String {
+        let host = utf8_percent_encode(&self.host, NON_ALPHANUMERIC_WITHOUT_DOT);
         if let Some((user, password)) = &self.user_password {
             let user = utf8_percent_encode(user, NON_ALPHANUMERIC);
             let password = utf8_percent_encode(password, NON_ALPHANUMERIC);
-            format!("socks5://{user}:{password}@{}:{}", self.host, self.port)
+            format!("socks5://{user}:{password}@{host}:{}", self.port)
         } else {
-            format!("socks5://{}:{}", self.host, self.port)
+            format!("socks5://{host}:{}", self.port)
         }
     }
 }
@@ -562,20 +565,6 @@ mod tests {
                 user_password: None
             })
         );
-
-        let proxy_config = ProxyConfig::from_url("socks5://my-proxy.example.org").unwrap();
-        assert_eq!(
-            proxy_config,
-            ProxyConfig::Socks5(Socks5Config {
-                host: "my-proxy.example.org".to_string(),
-                port: 1080,
-                user_password: None
-            })
-        );
-        assert_eq!(
-            proxy_config.to_url(),
-            "socks5://my-proxy.example.org:1080".to_string()
-        );
     }
 
     #[test]
@@ -609,20 +598,6 @@ mod tests {
                 user_password: None
             })
         );
-
-        let proxy_config = ProxyConfig::from_url("http://my-proxy.example.org").unwrap();
-        assert_eq!(
-            proxy_config,
-            ProxyConfig::Http(HttpConfig {
-                host: "my-proxy.example.org".to_string(),
-                port: 80,
-                user_password: None
-            })
-        );
-        assert_eq!(
-            proxy_config.to_url(),
-            "http://my-proxy.example.org:80".to_string()
-        );
     }
 
     #[test]
@@ -656,20 +631,6 @@ mod tests {
                 user_password: None
             })
         );
-
-        let proxy_config = ProxyConfig::from_url("https://my-proxy.example.org").unwrap();
-        assert_eq!(
-            proxy_config,
-            ProxyConfig::Https(HttpConfig {
-                host: "my-proxy.example.org".to_string(),
-                port: 443,
-                user_password: None
-            })
-        );
-        assert_eq!(
-            proxy_config.to_url(),
-            "https://my-proxy.example.org:443".to_string()
-        );
     }
 
     #[test]

src/qr/qr_tests.rs

@@ -892,32 +892,6 @@ async fn test_set_proxy_config_from_qr() -> Result<()> {
     Ok(())
 }
 
-#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
-async fn test_dont_encode_hyphen_in_proxy_hostnames() -> Result<()> {
-    let mut tcm = TestContextManager::new();
-    let t = &tcm.alice().await;
-
-    let qr_text = "socks5://my-proxy.example.org";
-
-    let qr = check_qr(t, qr_text).await?;
-    assert_eq!(
-        qr,
-        Qr::Proxy {
-            url: "socks5://my-proxy.example.org".to_string(),
-            host: "my-proxy.example.org".to_string(),
-            port: 1080,
-        }
-    );
-
-    set_config_from_qr(t, "socks5://my-proxy.example.org").await?;
-    assert_eq!(
-        t.get_config(Config::ProxyUrl).await?,
-        Some("socks5://my-proxy.example.org:1080".to_string())
-    );
-
-    Ok(())
-}
-
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_decode_shadowsocks() -> Result<()> {
     let ctx = TestContext::new().await;

src/receive_imf.rs

@@ -14,7 +14,9 @@ use mailparse::SingleInfo;
 use num_traits::FromPrimitive;
 use regex::Regex;
 
-use crate::chat::{self, Chat, ChatId, ChatIdBlocked, ChatVisibility, save_broadcast_secret};
+use crate::chat::{
+    self, Chat, ChatId, ChatIdBlocked, ChatVisibility, is_contact_in_chat, save_broadcast_secret,
+};
 use crate::config::Config;
 use crate::constants::{self, Blocked, Chattype, DC_CHAT_ID_TRASH, EDITED_PREFIX, ShowEmails};
 use crate::contact::{self, Contact, ContactId, Origin, mark_contact_id_as_verified};
@@ -3121,17 +3123,18 @@ async fn apply_group_changes(
         }
     }
 
-    if is_from_in_chat {
-        apply_chat_name_avatar_and_description_changes(
-            context,
-            mime_parser,
-            from_id,
-            chat,
-            &mut send_event_chat_modified,
-            &mut better_msg,
-        )
-        .await?;
+    apply_chat_name_avatar_and_description_changes(
+        context,
+        mime_parser,
+        from_id,
+        is_from_in_chat,
+        chat,
+        &mut send_event_chat_modified,
+        &mut better_msg,
+    )
+    .await?;
 
+    if is_from_in_chat {
         // Avoid insertion of `from_id` into a group with inappropriate encryption state.
         if from_is_key_contact != chat.grpid.is_empty()
             && chat.member_list_is_stale(context).await?
@@ -3305,6 +3308,7 @@ async fn apply_chat_name_avatar_and_description_changes(
     context: &Context,
     mime_parser: &MimeMessage,
     from_id: ContactId,
+    is_from_in_chat: bool,
     chat: &mut Chat,
     send_event_chat_modified: &mut bool,
     better_msg: &mut Option<String>,
@@ -3333,7 +3337,8 @@ async fn apply_chat_name_avatar_and_description_changes(
         let chat_group_name_timestamp = chat.param.get_i64(Param::GroupNameTimestamp).unwrap_or(0);
         let group_name_timestamp = group_name_timestamp.unwrap_or(mime_parser.timestamp_sent);
         // To provide group name consistency, compare names if timestamps are equal.
-        if (chat_group_name_timestamp, grpname) < (group_name_timestamp, &chat.name)
+        if is_from_in_chat
+            && (chat_group_name_timestamp, grpname) < (group_name_timestamp, &chat.name)
             && chat
                 .id
                 .update_timestamp(context, Param::GroupNameTimestamp, group_name_timestamp)
@@ -3354,14 +3359,19 @@ async fn apply_chat_name_avatar_and_description_changes(
             .get_header(HeaderDef::ChatGroupNameChanged)
             .is_some()
         {
-            let old_name = &sanitize_single_line(old_name);
-            better_msg.get_or_insert(
-                if matches!(chat.typ, Chattype::InBroadcast | Chattype::OutBroadcast) {
-                    stock_str::msg_broadcast_name_changed(context, old_name, grpname)
-                } else {
-                    stock_str::msg_grp_name(context, old_name, grpname, from_id).await
-                },
-            );
+            if is_from_in_chat {
+                let old_name = &sanitize_single_line(old_name);
+                better_msg.get_or_insert(
+                    if matches!(chat.typ, Chattype::InBroadcast | Chattype::OutBroadcast) {
+                        stock_str::msg_broadcast_name_changed(context, old_name, grpname)
+                    } else {
+                        stock_str::msg_grp_name(context, old_name, grpname, from_id).await
+                    },
+                );
+            } else {
+                // Attempt to change group name by non-member, trash it.
+                *better_msg = Some(String::new());
+            }
         }
     }
 
@@ -3384,7 +3394,8 @@ async fn apply_chat_name_avatar_and_description_changes(
 
         let new_timestamp = timestamp_in_header.unwrap_or(mime_parser.timestamp_sent);
         // To provide consistency, compare descriptions if timestamps are equal.
-        if (old_timestamp, &old_description) < (new_timestamp, &new_description)
+        if is_from_in_chat
+            && (old_timestamp, &old_description) < (new_timestamp, &new_description)
             && chat
                 .id
                 .update_timestamp(context, Param::GroupDescriptionTimestamp, new_timestamp)
@@ -3405,8 +3416,13 @@ async fn apply_chat_name_avatar_and_description_changes(
             .get_header(HeaderDef::ChatGroupDescriptionChanged)
             .is_some()
         {
-            better_msg
-                .get_or_insert(stock_str::msg_chat_description_changed(context, from_id).await);
+            if is_from_in_chat {
+                better_msg
+                    .get_or_insert(stock_str::msg_chat_description_changed(context, from_id).await);
+            } else {
+                // Attempt to change group description by non-member, trash it.
+                *better_msg = Some(String::new());
+            }
         }
     }
 
@@ -3416,39 +3432,46 @@ async fn apply_chat_name_avatar_and_description_changes(
         && value == "group-avatar-changed"
         && let Some(avatar_action) = &mime_parser.group_avatar
     {
-        // this is just an explicit message containing the group-avatar,
-        // apart from that, the group-avatar is send along with various other messages
-        better_msg.get_or_insert(
-            if matches!(chat.typ, Chattype::InBroadcast | Chattype::OutBroadcast) {
-                stock_str::msg_broadcast_img_changed(context)
-            } else {
-                match avatar_action {
-                    AvatarAction::Delete => stock_str::msg_grp_img_deleted(context, from_id).await,
-                    AvatarAction::Change(_) => {
-                        stock_str::msg_grp_img_changed(context, from_id).await
+        if is_from_in_chat {
+            // this is just an explicit message containing the group-avatar,
+            // apart from that, the group-avatar is send along with various other messages
+            better_msg.get_or_insert(
+                if matches!(chat.typ, Chattype::InBroadcast | Chattype::OutBroadcast) {
+                    stock_str::msg_broadcast_img_changed(context)
+                } else {
+                    match avatar_action {
+                        AvatarAction::Delete => {
+                            stock_str::msg_grp_img_deleted(context, from_id).await
+                        }
+                        AvatarAction::Change(_) => {
+                            stock_str::msg_grp_img_changed(context, from_id).await
+                        }
                     }
-                }
-            },
-        );
+                },
+            );
+        } else {
+            // Attempt to change group avatar by non-member, trash it.
+            *better_msg = Some(String::new());
+        }
     }
 
-    if let Some(avatar_action) = &mime_parser.group_avatar {
-        info!(context, "Group-avatar change for {}.", chat.id);
-        if chat
+    if let Some(avatar_action) = &mime_parser.group_avatar
+        && is_from_in_chat
+        && chat
             .param
             .update_timestamp(Param::AvatarTimestamp, mime_parser.timestamp_sent)?
-        {
-            match avatar_action {
-                AvatarAction::Change(profile_image) => {
-                    chat.param.set(Param::ProfileImage, profile_image);
-                }
-                AvatarAction::Delete => {
-                    chat.param.remove(Param::ProfileImage);
-                }
-            };
-            chat.update_param(context).await?;
-            *send_event_chat_modified = true;
-        }
+    {
+        info!(context, "Group-avatar change for {}.", chat.id);
+        match avatar_action {
+            AvatarAction::Change(profile_image) => {
+                chat.param.set(Param::ProfileImage, profile_image);
+            }
+            AvatarAction::Delete => {
+                chat.param.remove(Param::ProfileImage);
+            }
+        };
+        chat.update_param(context).await?;
+        *send_event_chat_modified = true;
     }
 
     Ok(())
@@ -3755,10 +3778,12 @@ async fn apply_out_broadcast_changes(
     let mut added_removed_id: Option<ContactId> = None;
 
     if from_id == ContactId::SELF {
+        let is_from_in_chat = true;
         apply_chat_name_avatar_and_description_changes(
             context,
             mime_parser,
             from_id,
+            is_from_in_chat,
             chat,
             &mut send_event_chat_modified,
             &mut better_msg,
@@ -3847,10 +3872,12 @@ async fn apply_in_broadcast_changes(
     let mut send_event_chat_modified = false;
     let mut better_msg = None;
 
+    let is_from_in_chat = is_contact_in_chat(context, chat.id, from_id).await?;
     apply_chat_name_avatar_and_description_changes(
         context,
         mime_parser,
         from_id,
+        is_from_in_chat,
         chat,
         &mut send_event_chat_modified,
         &mut better_msg,

src/receive_imf/receive_imf_tests.rs

@@ -4378,39 +4378,42 @@ async fn test_recreate_member_list_on_missing_add_of_self() -> Result<()> {
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_keep_member_list_if_possibly_nomember() -> Result<()> {
-    let alice = TestContext::new_alice().await;
-    let bob = TestContext::new_bob().await;
-    let alice_chat_id = create_group(&alice, "Group").await?;
+    let mut tcm = TestContextManager::new();
+    let alice = &tcm.alice().await;
+    let bob = &tcm.bob().await;
+    let alice_chat_id = create_group(alice, "Group").await?;
     add_contact_to_chat(
-        &alice,
+        alice,
         alice_chat_id,
-        alice.add_or_lookup_contact_id(&bob).await,
+        alice.add_or_lookup_contact_id(bob).await,
     )
     .await?;
-    send_text_msg(&alice, alice_chat_id, "populate".to_string()).await?;
+    send_text_msg(alice, alice_chat_id, "populate".to_string()).await?;
     let bob_chat_id = bob.recv_msg(&alice.pop_sent_msg().await).await.chat_id;
 
-    let fiona = TestContext::new_fiona().await;
+    let fiona = &tcm.fiona().await;
     add_contact_to_chat(
-        &alice,
+        alice,
         alice_chat_id,
-        alice.add_or_lookup_contact_id(&fiona).await,
+        alice.add_or_lookup_contact_id(fiona).await,
     )
     .await?;
     let fiona_chat_id = fiona.recv_msg(&alice.pop_sent_msg().await).await.chat_id;
-    fiona_chat_id.accept(&fiona).await?;
+    fiona_chat_id.accept(fiona).await?;
 
     SystemTime::shift(Duration::from_secs(60));
-    chat::set_chat_name(&fiona, fiona_chat_id, "Renamed").await?;
-    bob.recv_msg(&fiona.pop_sent_msg().await).await;
+    chat::set_chat_name(fiona, fiona_chat_id, "Renamed").await?;
+
+    // Message about chat name change from non-member is trashed.
+    bob.recv_msg_trash(&fiona.pop_sent_msg().await).await;
 
     // Bob missed the message adding fiona, but mustn't recreate the member list or apply the group
     // name change.
-    assert_eq!(get_chat_contacts(&bob, bob_chat_id).await?.len(), 2);
-    assert!(is_contact_in_chat(&bob, bob_chat_id, ContactId::SELF).await?);
-    let bob_alice_contact = bob.add_or_lookup_contact_id(&alice).await;
-    assert!(is_contact_in_chat(&bob, bob_chat_id, bob_alice_contact).await?);
-    let chat = Chat::load_from_db(&bob, bob_chat_id).await?;
+    assert_eq!(get_chat_contacts(bob, bob_chat_id).await?.len(), 2);
+    assert!(is_contact_in_chat(bob, bob_chat_id, ContactId::SELF).await?);
+    let bob_alice_contact = bob.add_or_lookup_contact_id(alice).await;
+    assert!(is_contact_in_chat(bob, bob_chat_id, bob_alice_contact).await?);
+    let chat = Chat::load_from_db(bob, bob_chat_id).await?;
     assert_eq!(chat.get_name(), "Group");
     Ok(())
 }