Skip to content

Commit ee6b907

Browse files
r10sHocuri
andauthored
slightly nicer and shorter QR and invite codes (#7390)
- sort garbage to the beginning, readable text to the end - instead of `%20`, make use of `+` to encode spaces - shorter invite links and smaller QR codes by truncation of the names the truncation of the name uses chars() which does not respect grapheme clusters, so that last character may be wrong. not sure if there is a nice and easy alternative, but maybe it's good engoug - the real, full name will come over the wire (exiting truncate() truncates on word boundaries, which is maybe too soft here - names may be long, depending on the language, and not contain any space) moreover, this resolves the "name too long" issue from #7015 --------- Co-authored-by: Hocuri <hocuri@gmx.de>
1 parent 9c2a13b commit ee6b907

File tree

5 files changed

+89
-43
lines changed

5 files changed

+89
-43
lines changed

deltachat-rpc-client/tests/test_securejoin.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ def test_qr_securejoin_broadcast(acfactory, all_devices_online):
122122
bob2.start_io()
123123

124124
logging.info("===================== Alice creates a broadcast =====================")
125-
alice_chat = alice.create_broadcast("Broadcast channel for everyone!")
125+
alice_chat = alice.create_broadcast("Broadcast channel!")
126126
snapshot = alice_chat.get_basic_snapshot()
127127
assert not snapshot.is_unpromoted # Broadcast channels are never unpromoted
128128

@@ -135,8 +135,8 @@ def test_qr_securejoin_broadcast(acfactory, all_devices_online):
135135
alice_chat.send_text("Hello everyone!")
136136

137137
def get_broadcast(ac):
138-
chat = ac.get_chatlist(query="Broadcast channel for everyone!")[0]
139-
assert chat.get_basic_snapshot().name == "Broadcast channel for everyone!"
138+
chat = ac.get_chatlist(query="Broadcast channel!")[0]
139+
assert chat.get_basic_snapshot().name == "Broadcast channel!"
140140
return chat
141141

142142
def wait_for_broadcast_messages(ac):
@@ -184,7 +184,7 @@ def check_account(ac, contact, inviter_side, please_wait_info_msg=False):
184184

185185
chat_snapshot = chat.get_full_snapshot()
186186
assert chat_snapshot.is_encrypted
187-
assert chat_snapshot.name == "Broadcast channel for everyone!"
187+
assert chat_snapshot.name == "Broadcast channel!"
188188
if inviter_side:
189189
assert chat_snapshot.chat_type == ChatType.OUT_BROADCAST
190190
else:

deltachat-rpc-client/tests/test_something.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -940,7 +940,7 @@ def test_leave_broadcast(acfactory, all_devices_online):
940940
bob2.start_io()
941941

942942
logging.info("===================== Alice creates a broadcast =====================")
943-
alice_chat = alice.create_broadcast("Broadcast channel for everyone!")
943+
alice_chat = alice.create_broadcast("Broadcast channel!")
944944

945945
logging.info("===================== Bob joins the broadcast =====================")
946946
qr_code = alice_chat.get_qr_code()
@@ -957,8 +957,8 @@ def test_leave_broadcast(acfactory, all_devices_online):
957957
assert member_added_msg.get_snapshot().text == "You joined the channel."
958958

959959
def get_broadcast(ac):
960-
chat = ac.get_chatlist(query="Broadcast channel for everyone!")[0]
961-
assert chat.get_basic_snapshot().name == "Broadcast channel for everyone!"
960+
chat = ac.get_chatlist(query="Broadcast channel!")[0]
961+
assert chat.get_basic_snapshot().name == "Broadcast channel!"
962962
return chat
963963

964964
def check_account(ac, contact, inviter_side, please_wait_info_msg=False):

src/qr.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -556,10 +556,14 @@ async fn decode_openpgp(context: &Context, qr: &str) -> Result<Qr> {
556556
fn decode_name(param: &BTreeMap<&str, &str>, key: &str) -> Result<Option<String>> {
557557
if let Some(encoded_name) = param.get(key) {
558558
let encoded_name = encoded_name.replace('+', "%20"); // sometimes spaces are encoded as `+`
559-
match percent_decode_str(&encoded_name).decode_utf8() {
560-
Ok(name) => Ok(Some(name.to_string())),
559+
let mut name = match percent_decode_str(&encoded_name).decode_utf8() {
560+
Ok(name) => name.to_string(),
561561
Err(err) => bail!("Invalid QR param {key}: {err}"),
562+
};
563+
if let Some(n) = name.strip_suffix('_') {
564+
name = format!("{n}…");
562565
}
566+
Ok(Some(name))
563567
} else {
564568
Ok(None)
565569
}

src/securejoin.rs

Lines changed: 33 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
33
use anyhow::{Context as _, Error, Result, bail, ensure};
44
use deltachat_contact_tools::ContactAddress;
5-
use percent_encoding::{NON_ALPHANUMERIC, utf8_percent_encode};
5+
use percent_encoding::{AsciiSet, utf8_percent_encode};
66

77
use crate::chat::{self, Chat, ChatId, ChatIdBlocked, get_chat_id_by_grpid};
88
use crate::config::Config;
@@ -42,6 +42,8 @@ use crate::token::Namespace;
4242
// when Delta Chat v2.22.0 is sufficiently rolled out
4343
const VERIFICATION_TIMEOUT_SECONDS: i64 = 7 * 24 * 3600;
4444

45+
const DISALLOWED_CHARACTERS: &AsciiSet = &NON_ALPHANUMERIC_WITHOUT_DOT.remove(b'_');
46+
4547
fn inviter_progress(
4648
context: &Context,
4749
contact_id: ContactId,
@@ -60,6 +62,17 @@ fn inviter_progress(
6062
Ok(())
6163
}
6264

65+
/// Shorten name to max. 25 characters.
66+
/// This is to not make QR codes or invite links arbitrary long.
67+
fn shorten_name(name: &str) -> String {
68+
if name.chars().count() > 25 {
69+
// We use _ rather than ... to avoid dots at the end of the URL, which would confuse linkifiers
70+
format!("{}_", &name.chars().take(24).collect::<String>())
71+
} else {
72+
name.to_string()
73+
}
74+
}
75+
6376
/// Generates a Secure Join QR code.
6477
///
6578
/// 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<ChatId>) -> Resul
108121
let auth = create_id();
109122
token::save(context, Namespace::Auth, grpid, &auth, time()).await?;
110123

111-
let self_addr = context.get_primary_self_addr().await?;
112-
let self_name = context
113-
.get_config(Config::Displayname)
114-
.await?
115-
.unwrap_or_default();
116-
117-
let fingerprint = get_self_fingerprint(context).await?;
124+
let fingerprint = get_self_fingerprint(context).await?.hex();
118125

119-
let self_addr_urlencoded =
120-
utf8_percent_encode(&self_addr, NON_ALPHANUMERIC_WITHOUT_DOT).to_string();
121-
let self_name_urlencoded =
122-
utf8_percent_encode(&self_name, NON_ALPHANUMERIC_WITHOUT_DOT).to_string();
126+
let self_addr = context.get_primary_self_addr().await?;
127+
let self_addr_urlencoded = utf8_percent_encode(&self_addr, DISALLOWED_CHARACTERS).to_string();
123128

124129
let qr = if let Some(chat) = chat {
125130
if sync_token {
@@ -130,42 +135,36 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option<ChatId>) -> Resul
130135
}
131136

132137
let chat_name = chat.get_name();
133-
let chat_name_urlencoded = utf8_percent_encode(chat_name, NON_ALPHANUMERIC).to_string();
138+
let chat_name_shortened = shorten_name(chat_name);
139+
let chat_name_urlencoded = utf8_percent_encode(&chat_name_shortened, DISALLOWED_CHARACTERS)
140+
.to_string()
141+
.replace("%20", "+");
142+
let grpid = &chat.grpid;
134143
if chat.typ == Chattype::OutBroadcast {
135144
// For historic reansons, broadcasts currently use j instead of i for the invitenumber.
136145
format!(
137-
"https://i.delta.chat/#{}&a={}&b={}&x={}&j={}&s={}",
138-
fingerprint.hex(),
139-
self_addr_urlencoded,
140-
&chat_name_urlencoded,
141-
&chat.grpid,
142-
&invitenumber,
143-
&auth,
146+
"https://i.delta.chat/#{fingerprint}&x={grpid}&j={invitenumber}&s={auth}&a={self_addr_urlencoded}&b={chat_name_urlencoded}",
144147
)
145148
} else {
146149
format!(
147-
"https://i.delta.chat/#{}&a={}&g={}&x={}&i={}&s={}",
148-
fingerprint.hex(),
149-
self_addr_urlencoded,
150-
&chat_name_urlencoded,
151-
&chat.grpid,
152-
&invitenumber,
153-
&auth,
150+
"https://i.delta.chat/#{fingerprint}&x={grpid}&i={invitenumber}&s={auth}&a={self_addr_urlencoded}&g={chat_name_urlencoded}",
154151
)
155152
}
156153
} else {
157-
// parameters used: a=n=i=s=
154+
let self_name = context
155+
.get_config(Config::Displayname)
156+
.await?
157+
.unwrap_or_default();
158+
let self_name_shortened = shorten_name(&self_name);
159+
let self_name_urlencoded = utf8_percent_encode(&self_name_shortened, DISALLOWED_CHARACTERS)
160+
.to_string()
161+
.replace("%20", "+");
158162
if sync_token {
159163
context.sync_qr_code_tokens(None).await?;
160164
context.scheduler.interrupt_inbox().await;
161165
}
162166
format!(
163-
"https://i.delta.chat/#{}&a={}&n={}&i={}&s={}",
164-
fingerprint.hex(),
165-
self_addr_urlencoded,
166-
self_name_urlencoded,
167-
&invitenumber,
168-
&auth,
167+
"https://i.delta.chat/#{fingerprint}&i={invitenumber}&s={auth}&a={self_addr_urlencoded}&n={self_name_urlencoded}",
169168
)
170169
};
171170

src/securejoin/securejoin_tests.rs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1070,3 +1070,46 @@ async fn test_rejoin_group() -> Result<()> {
10701070

10711071
Ok(())
10721072
}
1073+
1074+
/// To make invite links a little bit nicer, we put the readable part at the end and encode spaces by `+` instead of `%20`.
1075+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
1076+
async fn test_get_securejoin_qr_name_is_last() -> Result<()> {
1077+
let mut tcm = TestContextManager::new();
1078+
let alice = &tcm.alice().await;
1079+
alice
1080+
.set_config(Config::Displayname, Some("Alice Axe"))
1081+
.await?;
1082+
let qr = get_securejoin_qr(alice, None).await?;
1083+
assert!(qr.ends_with("Alice+Axe"));
1084+
1085+
let alice_chat_id = chat::create_group(alice, "The Chat").await?;
1086+
let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await?;
1087+
assert!(qr.ends_with("The+Chat"));
1088+
1089+
Ok(())
1090+
}
1091+
1092+
/// QR codes should not get arbitrary big because of long names.
1093+
/// The truncation, however, should not let the url end with a `.`, which is a call for trouble in linkfiers.
1094+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
1095+
async fn test_get_securejoin_qr_name_is_truncated() -> Result<()> {
1096+
let mut tcm = TestContextManager::new();
1097+
let alice = &tcm.alice().await;
1098+
alice
1099+
.set_config(
1100+
Config::Displayname,
1101+
Some("Alice Axe Has A Very Long Family Name Really Indeed"),
1102+
)
1103+
.await?;
1104+
let qr = get_securejoin_qr(alice, None).await?;
1105+
assert!(qr.ends_with("Alice+Axe+Has+A+Very+Lon_"));
1106+
assert!(!qr.ends_with("."));
1107+
1108+
let alice_chat_id =
1109+
chat::create_group(alice, "The Chat With One Of The Longest Titles Around").await?;
1110+
let qr = get_securejoin_qr(alice, Some(alice_chat_id)).await?;
1111+
assert!(qr.ends_with("The+Chat+With+One+Of+The_"));
1112+
assert!(!qr.ends_with("."));
1113+
1114+
Ok(())
1115+
}

0 commit comments

Comments
 (0)