Skip to content

Commit e452230

Browse files
committed
feat: Don't reset key-contact status if Chat-User-Avatar header is absent (#7002)
This prepares for sending self-status only together with self-avatar in encrypted messages. The idea is that self-status normally doesn't change frequently, so it's not a problem to re-send the whole profile. Self-status is rather a biography, it even goes to "NOTE:" in vCards, so it's not a contact status at a particular moment like "online" or "busy", and to see it one should go to the contact profile. Don't check for "Chat-Version" header though. So if a non- Delta Chat key-contact removes footer, its "status" remains, but this shouldn't be a problem. For unencrypted messages self-status will still be always attached except MDNs, reactions and SecureJoin messages, so that it's visible as the message footer in other MUAs.
1 parent ab9fd3d commit e452230

File tree

7 files changed

+82
-25
lines changed

7 files changed

+82
-25
lines changed

src/chat.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4008,12 +4008,12 @@ pub(crate) async fn add_contact_to_chat_ex(
40084008
Ok(true)
40094009
}
40104010

4011-
/// Returns true if an avatar should be attached in the given chat.
4011+
/// Returns whether profile data should be attached when sending to the given chat.
40124012
///
4013-
/// This function does not check if the avatar is set.
4013+
/// This function does not check if the avatar/status is set.
40144014
/// If avatar is not set and this function returns `true`,
40154015
/// a `Chat-User-Avatar: 0` header should be sent to reset the avatar.
4016-
pub(crate) async fn shall_attach_selfavatar(context: &Context, chat_id: ChatId) -> Result<bool> {
4016+
pub(crate) async fn should_attach_profile(context: &Context, chat_id: ChatId) -> Result<bool> {
40174017
let timestamp_some_days_ago = time() - DC_RESEND_USER_AVATAR_DAYS * 24 * 60 * 60;
40184018
let needs_attach = context
40194019
.sql
@@ -4028,8 +4028,8 @@ pub(crate) async fn shall_attach_selfavatar(context: &Context, chat_id: ChatId)
40284028
let mut needs_attach = false;
40294029
for row in rows {
40304030
let row = row?;
4031-
let selfavatar_sent = row?;
4032-
if selfavatar_sent < timestamp_some_days_ago {
4031+
let profile_sent = row?;
4032+
if profile_sent < timestamp_some_days_ago {
40334033
needs_attach = true;
40344034
}
40354035
}

src/chat/chat_tests.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1542,23 +1542,23 @@ async fn test_create_same_chat_twice() {
15421542
}
15431543

15441544
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
1545-
async fn test_shall_attach_selfavatar() -> Result<()> {
1545+
async fn test_should_attach_profile() -> Result<()> {
15461546
let mut tcm = TestContextManager::new();
15471547
let alice = &tcm.alice().await;
15481548
let bob = &tcm.bob().await;
15491549

15501550
let chat_id = create_group_chat(alice, ProtectionStatus::Unprotected, "foo").await?;
1551-
assert!(!shall_attach_selfavatar(alice, chat_id).await?);
1551+
assert!(!should_attach_profile(alice, chat_id).await?);
15521552

15531553
let contact_id = alice.add_or_lookup_contact_id(bob).await;
15541554
add_contact_to_chat(alice, chat_id, contact_id).await?;
1555-
assert!(shall_attach_selfavatar(alice, chat_id).await?);
1555+
assert!(should_attach_profile(alice, chat_id).await?);
15561556

15571557
chat_id.set_selfavatar_timestamp(alice, time()).await?;
1558-
assert!(!shall_attach_selfavatar(alice, chat_id).await?);
1558+
assert!(!should_attach_profile(alice, chat_id).await?);
15591559

15601560
alice.set_config(Config::Selfavatar, None).await?; // setting to None also forces re-sending
1561-
assert!(shall_attach_selfavatar(alice, chat_id).await?);
1561+
assert!(should_attach_profile(alice, chat_id).await?);
15621562
Ok(())
15631563
}
15641564

@@ -1582,7 +1582,7 @@ async fn test_profile_data_on_group_leave() -> Result<()> {
15821582
tokio::fs::write(&file, bytes).await?;
15831583
t.set_config(Config::Selfavatar, Some(file.to_str().unwrap()))
15841584
.await?;
1585-
assert!(shall_attach_selfavatar(t, chat_id).await?);
1585+
assert!(should_attach_profile(t, chat_id).await?);
15861586

15871587
remove_contact_from_chat(t, chat_id, ContactId::SELF).await?;
15881588
let sent_msg = t.pop_sent_msg().await;

src/config.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -744,6 +744,16 @@ impl Context {
744744
let better_value;
745745

746746
match key {
747+
Config::Selfstatus => {
748+
// Currently we send the self-status in every appropriate message, but in the future
749+
// (when most users upgrade to "feat: Don't reset key-contact status if
750+
// Chat-User-Avatar header is absent") we want to send it periodically together with
751+
// the self-avatar. This ensures the correct behavior after a possible Core upgrade.
752+
self.sql
753+
.execute("UPDATE contacts SET selfavatar_sent=0", ())
754+
.await?;
755+
self.sql.set_raw_config(key.as_ref(), value).await?;
756+
}
747757
Config::Selfavatar => {
748758
self.sql
749759
.execute("UPDATE contacts SET selfavatar_sent=0;", ())

src/headerdef.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,12 @@ pub enum HeaderDef {
6060
ChatGroupNameTimestamp,
6161
ChatVerified,
6262
ChatGroupAvatar,
63+
64+
/// If present, contact's avatar and status should be applied from the message.
65+
/// "Chat-User-Avatar: 0" means that the contact has no avatar. Contact's status is transferred
66+
/// in the message footer.
6367
ChatUserAvatar,
68+
6469
ChatVoiceMessage,
6570
ChatGroupMemberRemoved,
6671
ChatGroupMemberAdded,

src/mimefactory.rs

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ impl MimeFactory {
181181
pub async fn from_msg(context: &Context, msg: Message) -> Result<MimeFactory> {
182182
let now = time();
183183
let chat = Chat::load_from_db(context, msg.chat_id).await?;
184-
let attach_profile_data = Self::should_attach_profile_data(&msg);
184+
let can_transfer_profile = Self::can_transfer_profile(&msg);
185185
let undisclosed_recipients = chat.typ == Chattype::OutBroadcast;
186186

187187
let from_addr = context.get_primary_self_addr().await?;
@@ -193,7 +193,7 @@ impl MimeFactory {
193193
if let Some(override_name) = msg.param.get(Param::OverrideSenderDisplayname) {
194194
(override_name.to_string(), Some(config_displayname))
195195
} else {
196-
let name = match attach_profile_data {
196+
let name = match can_transfer_profile {
197197
true => config_displayname,
198198
false => "".to_string(),
199199
};
@@ -301,7 +301,7 @@ impl MimeFactory {
301301
} else {
302302
addr
303303
};
304-
let name = match attach_profile_data {
304+
let name = match can_transfer_profile {
305305
true => authname,
306306
false => "".to_string(),
307307
};
@@ -450,14 +450,18 @@ impl MimeFactory {
450450
.split_ascii_whitespace()
451451
.map(|s| s.trim_start_matches('<').trim_end_matches('>').to_string())
452452
.collect();
453-
let selfstatus = match attach_profile_data {
453+
let should_attach_profile = Self::should_attach_profile(context, &msg).await;
454+
// TODO: (2025-08) Attach self-status in every message for compatibility with older
455+
// versions. Should be replaced with
456+
// `should_attach_profile || !is_encrypted && can_transfer_profile`.
457+
let selfstatus = match can_transfer_profile {
454458
true => context
455459
.get_config(Config::Selfstatus)
456460
.await?
457461
.unwrap_or_default(),
458462
false => "".to_string(),
459463
};
460-
let attach_selfavatar = Self::should_attach_selfavatar(context, &msg).await;
464+
let attach_selfavatar = should_attach_profile;
461465

462466
ensure_and_debug_assert!(
463467
member_timestamps.is_empty()
@@ -550,7 +554,7 @@ impl MimeFactory {
550554
}
551555
}
552556

553-
fn should_attach_profile_data(msg: &Message) -> bool {
557+
fn can_transfer_profile(msg: &Message) -> bool {
554558
msg.param.get_cmd() != SystemMessage::SecurejoinMessage || {
555559
let step = msg.param.get(Param::Arg).unwrap_or_default();
556560
// Don't attach profile data at the earlier SecureJoin steps:
@@ -565,14 +569,14 @@ impl MimeFactory {
565569
}
566570
}
567571

568-
async fn should_attach_selfavatar(context: &Context, msg: &Message) -> bool {
569-
Self::should_attach_profile_data(msg)
570-
&& match chat::shall_attach_selfavatar(context, msg.chat_id).await {
572+
async fn should_attach_profile(context: &Context, msg: &Message) -> bool {
573+
Self::can_transfer_profile(msg)
574+
&& match chat::should_attach_profile(context, msg.chat_id).await {
571575
Ok(should) => should,
572576
Err(err) => {
573577
warn!(
574578
context,
575-
"should_attach_selfavatar: cannot get selfavatar state: {err:#}."
579+
"should_attach_profile: chat::should_attach_profile: {err:#}."
576580
);
577581
false
578582
}
@@ -637,7 +641,7 @@ impl MimeFactory {
637641
return Ok(format!("Re: {}", remove_subject_prefix(last_subject)));
638642
}
639643

640-
let self_name = match Self::should_attach_profile_data(msg) {
644+
let self_name = match Self::can_transfer_profile(msg) {
641645
true => context.get_config(Config::Displayname).await?,
642646
false => None,
643647
};

src/receive_imf.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -938,8 +938,11 @@ pub(crate) async fn receive_imf_inner(
938938
}
939939
}
940940

941-
// Ignore footers from mailinglists as they are often created or modified by the mailinglist software.
942-
if let Some(footer) = &mime_parser.footer {
941+
if let Some(footer) = mime_parser.footer.as_ref().filter(|footer| {
942+
!footer.is_empty() || mime_parser.user_avatar.is_some() || !mime_parser.was_encrypted()
943+
}) {
944+
// Ignore footers from mailinglists as they are often created or modified by the mailinglist
945+
// software.
943946
if !mime_parser.is_mailinglist_message()
944947
&& from_id != ContactId::UNDEFINED
945948
&& context
@@ -953,7 +956,7 @@ pub(crate) async fn receive_imf_inner(
953956
if let Err(err) = contact::set_status(
954957
context,
955958
from_id,
956-
footer.to_string(),
959+
footer.clone(),
957960
mime_parser.was_encrypted(),
958961
mime_parser.has_chat_version(),
959962
)

src/receive_imf/receive_imf_tests.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2254,6 +2254,41 @@ sig thursday",
22542254
Ok(())
22552255
}
22562256

2257+
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
2258+
async fn test_contact_status_from_encrypted_msg() -> Result<()> {
2259+
let mut tcm = TestContextManager::new();
2260+
let alice = &tcm.alice().await;
2261+
let bob = &tcm.bob().await;
2262+
2263+
let alice_chat_id = alice.create_chat(bob).await.id;
2264+
let sent = alice.send_text(alice_chat_id, "hi").await;
2265+
bob.recv_msg(&sent).await;
2266+
alice.set_config(Config::Selfstatus, Some("status")).await?;
2267+
let sent = alice.send_text(alice_chat_id, "I've set self-status").await;
2268+
bob.recv_msg(&sent).await;
2269+
let bob_alice = bob.add_or_lookup_contact(alice).await;
2270+
assert_eq!(bob_alice.get_status(), "status");
2271+
2272+
alice
2273+
.set_config(Config::Selfstatus, Some("status1"))
2274+
.await?;
2275+
alice
2276+
.send_text(alice_chat_id, "I changed self-status")
2277+
.await;
2278+
2279+
// Currently we send self-status in every appropriate message.
2280+
let sent = alice
2281+
.send_text(alice_chat_id, "This message also contains my status")
2282+
.await;
2283+
let parsed_msg = bob.parse_msg(&sent).await;
2284+
assert!(parsed_msg.was_encrypted());
2285+
assert!(parsed_msg.get_header(HeaderDef::ChatUserAvatar).is_none());
2286+
bob.recv_msg(&sent).await;
2287+
let bob_alice = bob.add_or_lookup_contact(alice).await;
2288+
assert_eq!(bob_alice.get_status(), "status1");
2289+
Ok(())
2290+
}
2291+
22572292
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
22582293
async fn test_chat_assignment_private_classical_reply() {
22592294
for outgoing_is_classical in &[true, false] {

0 commit comments

Comments
 (0)