diff --git a/deltachat-rpc-client/tests/test_securejoin.py b/deltachat-rpc-client/tests/test_securejoin.py index 6222a2a4e..780f9f419 100644 --- a/deltachat-rpc-client/tests/test_securejoin.py +++ b/deltachat-rpc-client/tests/test_securejoin.py @@ -122,7 +122,7 @@ def test_qr_securejoin_broadcast(acfactory, all_devices_online): bob2.start_io() logging.info("===================== Alice creates a broadcast =====================") - alice_chat = alice.create_broadcast("Broadcast channel for everyone!") + alice_chat = alice.create_broadcast("Broadcast channel!") snapshot = alice_chat.get_basic_snapshot() assert not snapshot.is_unpromoted # Broadcast channels are never unpromoted @@ -135,8 +135,8 @@ def test_qr_securejoin_broadcast(acfactory, all_devices_online): alice_chat.send_text("Hello everyone!") def get_broadcast(ac): - chat = ac.get_chatlist(query="Broadcast channel for everyone!")[0] - assert chat.get_basic_snapshot().name == "Broadcast channel for everyone!" + chat = ac.get_chatlist(query="Broadcast channel!")[0] + assert chat.get_basic_snapshot().name == "Broadcast channel!" return chat def wait_for_broadcast_messages(ac): @@ -184,7 +184,7 @@ def test_qr_securejoin_broadcast(acfactory, all_devices_online): chat_snapshot = chat.get_full_snapshot() assert chat_snapshot.is_encrypted - assert chat_snapshot.name == "Broadcast channel for everyone!" + assert chat_snapshot.name == "Broadcast channel!" if inviter_side: assert chat_snapshot.chat_type == ChatType.OUT_BROADCAST else: diff --git a/deltachat-rpc-client/tests/test_something.py b/deltachat-rpc-client/tests/test_something.py index 5bbc32c21..c82bf4fc6 100644 --- a/deltachat-rpc-client/tests/test_something.py +++ b/deltachat-rpc-client/tests/test_something.py @@ -940,7 +940,7 @@ def test_leave_broadcast(acfactory, all_devices_online): bob2.start_io() logging.info("===================== Alice creates a broadcast =====================") - alice_chat = alice.create_broadcast("Broadcast channel for everyone!") + alice_chat = alice.create_broadcast("Broadcast channel!") logging.info("===================== Bob joins the broadcast =====================") qr_code = alice_chat.get_qr_code() @@ -957,8 +957,8 @@ def test_leave_broadcast(acfactory, all_devices_online): assert member_added_msg.get_snapshot().text == "You joined the channel." def get_broadcast(ac): - chat = ac.get_chatlist(query="Broadcast channel for everyone!")[0] - assert chat.get_basic_snapshot().name == "Broadcast channel for everyone!" + chat = ac.get_chatlist(query="Broadcast channel!")[0] + assert chat.get_basic_snapshot().name == "Broadcast channel!" return chat def check_account(ac, contact, inviter_side, please_wait_info_msg=False): diff --git a/src/qr.rs b/src/qr.rs index 4f1476bbd..61816b76c 100644 --- a/src/qr.rs +++ b/src/qr.rs @@ -556,10 +556,14 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result { fn decode_name(param: &BTreeMap<&str, &str>, key: &str) -> Result> { if let Some(encoded_name) = param.get(key) { let encoded_name = encoded_name.replace('+', "%20"); // sometimes spaces are encoded as `+` - match percent_decode_str(&encoded_name).decode_utf8() { - Ok(name) => Ok(Some(name.to_string())), + let mut name = match percent_decode_str(&encoded_name).decode_utf8() { + Ok(name) => name.to_string(), Err(err) => bail!("Invalid QR param {key}: {err}"), + }; + if let Some(n) = name.strip_suffix('_') { + name = format!("{n}…"); } + Ok(Some(name)) } else { Ok(None) } diff --git a/src/securejoin.rs b/src/securejoin.rs index d2b9337dc..eabd2da92 100644 --- a/src/securejoin.rs +++ b/src/securejoin.rs @@ -2,7 +2,7 @@ use anyhow::{Context as _, Error, Result, bail, ensure}; use deltachat_contact_tools::ContactAddress; -use percent_encoding::{NON_ALPHANUMERIC, utf8_percent_encode}; +use percent_encoding::{AsciiSet, utf8_percent_encode}; use crate::chat::{self, Chat, ChatId, ChatIdBlocked, get_chat_id_by_grpid}; use crate::config::Config; @@ -42,6 +42,8 @@ use crate::token::Namespace; // when Delta Chat v2.22.0 is sufficiently rolled out const VERIFICATION_TIMEOUT_SECONDS: i64 = 7 * 24 * 3600; +const DISALLOWED_CHARACTERS: &AsciiSet = &NON_ALPHANUMERIC_WITHOUT_DOT.remove(b'_'); + fn inviter_progress( context: &Context, contact_id: ContactId, @@ -60,6 +62,17 @@ fn inviter_progress( Ok(()) } +/// Shorten name to max. 25 characters. +/// This is to not make QR codes or invite links arbitrary long. +fn shorten_name(name: &str) -> String { + if name.chars().count() > 25 { + // We use _ rather than ... to avoid dots at the end of the URL, which would confuse linkifiers + format!("{}_", &name.chars().take(24).collect::()) + } else { + name.to_string() + } +} + /// Generates a Secure Join QR code. /// /// With `chat` set to `None` this generates a setup-contact QR code, with `chat` set to a @@ -108,18 +121,10 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option) -> Resul let auth = create_id(); token::save(context, Namespace::Auth, grpid, &auth, time()).await?; + let fingerprint = get_self_fingerprint(context).await?.hex(); + let self_addr = context.get_primary_self_addr().await?; - let self_name = context - .get_config(Config::Displayname) - .await? - .unwrap_or_default(); - - let fingerprint = get_self_fingerprint(context).await?; - - let self_addr_urlencoded = - utf8_percent_encode(&self_addr, NON_ALPHANUMERIC_WITHOUT_DOT).to_string(); - let self_name_urlencoded = - utf8_percent_encode(&self_name, NON_ALPHANUMERIC_WITHOUT_DOT).to_string(); + let self_addr_urlencoded = utf8_percent_encode(&self_addr, DISALLOWED_CHARACTERS).to_string(); let qr = if let Some(chat) = chat { if sync_token { @@ -130,42 +135,36 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option) -> Resul } let chat_name = chat.get_name(); - let chat_name_urlencoded = utf8_percent_encode(chat_name, NON_ALPHANUMERIC).to_string(); + let chat_name_shortened = shorten_name(chat_name); + let chat_name_urlencoded = utf8_percent_encode(&chat_name_shortened, DISALLOWED_CHARACTERS) + .to_string() + .replace("%20", "+"); + let grpid = &chat.grpid; if chat.typ == Chattype::OutBroadcast { // For historic reansons, broadcasts currently use j instead of i for the invitenumber. format!( - "https://i.delta.chat/#{}&a={}&b={}&x={}&j={}&s={}", - fingerprint.hex(), - self_addr_urlencoded, - &chat_name_urlencoded, - &chat.grpid, - &invitenumber, - &auth, + "https://i.delta.chat/#{fingerprint}&x={grpid}&j={invitenumber}&s={auth}&a={self_addr_urlencoded}&b={chat_name_urlencoded}", ) } else { format!( - "https://i.delta.chat/#{}&a={}&g={}&x={}&i={}&s={}", - fingerprint.hex(), - self_addr_urlencoded, - &chat_name_urlencoded, - &chat.grpid, - &invitenumber, - &auth, + "https://i.delta.chat/#{fingerprint}&x={grpid}&i={invitenumber}&s={auth}&a={self_addr_urlencoded}&g={chat_name_urlencoded}", ) } } else { - // parameters used: a=n=i=s= + let self_name = context + .get_config(Config::Displayname) + .await? + .unwrap_or_default(); + let self_name_shortened = shorten_name(&self_name); + let self_name_urlencoded = utf8_percent_encode(&self_name_shortened, DISALLOWED_CHARACTERS) + .to_string() + .replace("%20", "+"); if sync_token { context.sync_qr_code_tokens(None).await?; context.scheduler.interrupt_inbox().await; } format!( - "https://i.delta.chat/#{}&a={}&n={}&i={}&s={}", - fingerprint.hex(), - self_addr_urlencoded, - self_name_urlencoded, - &invitenumber, - &auth, + "https://i.delta.chat/#{fingerprint}&i={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}", ) }; diff --git a/src/securejoin/securejoin_tests.rs b/src/securejoin/securejoin_tests.rs index 9b8197a04..537cea4bd 100644 --- a/src/securejoin/securejoin_tests.rs +++ b/src/securejoin/securejoin_tests.rs @@ -1070,3 +1070,46 @@ async fn test_rejoin_group() -> Result<()> { Ok(()) } + +/// To make invite links a little bit nicer, we put the readable part at the end and encode spaces by `+` instead of `%20`. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_get_securejoin_qr_name_is_last() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + alice + .set_config(Config::Displayname, Some("Alice Axe")) + .await?; + let qr = get_securejoin_qr(alice, None).await?; + assert!(qr.ends_with("Alice+Axe")); + + let alice_chat_id = chat::create_group(alice, "The Chat").await?; + let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await?; + assert!(qr.ends_with("The+Chat")); + + Ok(()) +} + +/// QR codes should not get arbitrary big because of long names. +/// The truncation, however, should not let the url end with a `.`, which is a call for trouble in linkfiers. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_get_securejoin_qr_name_is_truncated() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + alice + .set_config( + Config::Displayname, + Some("Alice Axe Has A Very Long Family Name Really Indeed"), + ) + .await?; + let qr = get_securejoin_qr(alice, None).await?; + assert!(qr.ends_with("Alice+Axe+Has+A+Very+Lon_")); + assert!(!qr.ends_with(".")); + + let alice_chat_id = + chat::create_group(alice, "The Chat With One Of The Longest Titles Around").await?; + let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await?; + assert!(qr.ends_with("The+Chat+With+One+Of+The_")); + assert!(!qr.ends_with(".")); + + Ok(()) +}