fix: do not URL-encode proxy hostnames

src/net/proxy.rs

@@ -19,7 +19,6 @@ 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;
@@ -93,13 +92,12 @@ 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}@{host}:{}", self.port)
+            format!("{scheme}://{user}:{password}@{}:{}", self.host, self.port)
         } else {
-            format!("{scheme}://{host}:{}", self.port)
+            format!("{scheme}://{}:{}", self.host, self.port)
         }
     }
 }
@@ -143,13 +141,12 @@ 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}@{host}:{}", self.port)
+            format!("socks5://{user}:{password}@{}:{}", self.host, self.port)
         } else {
-            format!("socks5://{host}:{}", self.port)
+            format!("socks5://{}:{}", self.host, self.port)
         }
     }
 }
@@ -565,6 +562,20 @@ 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]
@@ -598,6 +609,20 @@ 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]
@@ -631,6 +656,20 @@ 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,6 +892,32 @@ 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,9 +14,7 @@ use mailparse::SingleInfo;
 use num_traits::FromPrimitive;
 use regex::Regex;
 
-use crate::chat::{
-    self, Chat, ChatId, ChatIdBlocked, ChatVisibility, is_contact_in_chat, save_broadcast_secret,
-};
+use crate::chat::{self, Chat, ChatId, ChatIdBlocked, ChatVisibility, 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};
@@ -3123,18 +3121,17 @@ async fn apply_group_changes(
         }
     }
 
-    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 {
+        apply_chat_name_avatar_and_description_changes(
+            context,
+            mime_parser,
+            from_id,
+            chat,
+            &mut send_event_chat_modified,
+            &mut better_msg,
+        )
+        .await?;
+
         // 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?
@@ -3308,7 +3305,6 @@ 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>,
@@ -3337,8 +3333,7 @@ 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 is_from_in_chat
-            && (chat_group_name_timestamp, grpname) < (group_name_timestamp, &chat.name)
+        if (chat_group_name_timestamp, grpname) < (group_name_timestamp, &chat.name)
             && chat
                 .id
                 .update_timestamp(context, Param::GroupNameTimestamp, group_name_timestamp)
@@ -3359,19 +3354,14 @@ async fn apply_chat_name_avatar_and_description_changes(
             .get_header(HeaderDef::ChatGroupNameChanged)
             .is_some()
         {
-            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());
-            }
+            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
+                },
+            );
         }
     }
 
@@ -3394,8 +3384,7 @@ 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 is_from_in_chat
-            && (old_timestamp, &old_description) < (new_timestamp, &new_description)
+        if (old_timestamp, &old_description) < (new_timestamp, &new_description)
             && chat
                 .id
                 .update_timestamp(context, Param::GroupDescriptionTimestamp, new_timestamp)
@@ -3416,13 +3405,8 @@ async fn apply_chat_name_avatar_and_description_changes(
             .get_header(HeaderDef::ChatGroupDescriptionChanged)
             .is_some()
         {
-            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());
-            }
+            better_msg
+                .get_or_insert(stock_str::msg_chat_description_changed(context, from_id).await);
         }
     }
 
@@ -3432,46 +3416,39 @@ async fn apply_chat_name_avatar_and_description_changes(
         && value == "group-avatar-changed"
         && let Some(avatar_action) = &mime_parser.group_avatar
     {
-        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
-                        }
+        // 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
-        && is_from_in_chat
-        && chat
+    if let Some(avatar_action) = &mime_parser.group_avatar {
+        info!(context, "Group-avatar change for {}.", chat.id);
+        if chat
             .param
             .update_timestamp(Param::AvatarTimestamp, mime_parser.timestamp_sent)?
-    {
-        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;
+        {
+            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(())
@@ -3778,12 +3755,10 @@ 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,
@@ -3872,12 +3847,10 @@ 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,42 +4378,39 @@ 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 mut tcm = TestContextManager::new();
-    let alice = &tcm.alice().await;
-    let bob = &tcm.bob().await;
-    let alice_chat_id = create_group(alice, "Group").await?;
+    let alice = TestContext::new_alice().await;
+    let bob = TestContext::new_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 = &tcm.fiona().await;
+    let fiona = TestContext::new_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?;
-
-    // Message about chat name change from non-member is trashed.
-    bob.recv_msg_trash(&fiona.pop_sent_msg().await).await;
+    chat::set_chat_name(&fiona, fiona_chat_id, "Renamed").await?;
+    bob.recv_msg(&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(())
 }