mirror of
https://github.com/chatmail/core.git
synced 2026-05-19 06:46:32 +03:00
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.
This commit is contained in:
10
src/chat.rs
10
src/chat.rs
@@ -3719,12 +3719,12 @@ pub(crate) async fn add_contact_to_chat_ex(
|
|||||||
Ok(true)
|
Ok(true)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Returns true if an avatar should be attached in the given chat.
|
/// Returns whether profile data should be attached when sending to the given chat.
|
||||||
///
|
///
|
||||||
/// This function does not check if the avatar is set.
|
/// This function does not check if the avatar/status is set.
|
||||||
/// If avatar is not set and this function returns `true`,
|
/// If avatar is not set and this function returns `true`,
|
||||||
/// a `Chat-User-Avatar: 0` header should be sent to reset the avatar.
|
/// a `Chat-User-Avatar: 0` header should be sent to reset the avatar.
|
||||||
pub(crate) async fn shall_attach_selfavatar(context: &Context, chat_id: ChatId) -> Result<bool> {
|
pub(crate) async fn should_attach_profile(context: &Context, chat_id: ChatId) -> Result<bool> {
|
||||||
let timestamp_some_days_ago = time() - DC_RESEND_USER_AVATAR_DAYS * 24 * 60 * 60;
|
let timestamp_some_days_ago = time() - DC_RESEND_USER_AVATAR_DAYS * 24 * 60 * 60;
|
||||||
let needs_attach = context
|
let needs_attach = context
|
||||||
.sql
|
.sql
|
||||||
@@ -3739,8 +3739,8 @@ pub(crate) async fn shall_attach_selfavatar(context: &Context, chat_id: ChatId)
|
|||||||
let mut needs_attach = false;
|
let mut needs_attach = false;
|
||||||
for row in rows {
|
for row in rows {
|
||||||
let row = row?;
|
let row = row?;
|
||||||
let selfavatar_sent = row?;
|
let profile_sent = row?;
|
||||||
if selfavatar_sent < timestamp_some_days_ago {
|
if profile_sent < timestamp_some_days_ago {
|
||||||
needs_attach = true;
|
needs_attach = true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1531,23 +1531,23 @@ async fn test_create_same_chat_twice() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
async fn test_shall_attach_selfavatar() -> Result<()> {
|
async fn test_should_attach_profile() -> Result<()> {
|
||||||
let mut tcm = TestContextManager::new();
|
let mut tcm = TestContextManager::new();
|
||||||
let alice = &tcm.alice().await;
|
let alice = &tcm.alice().await;
|
||||||
let bob = &tcm.bob().await;
|
let bob = &tcm.bob().await;
|
||||||
|
|
||||||
let chat_id = create_group(alice, "foo").await?;
|
let chat_id = create_group(alice, "foo").await?;
|
||||||
assert!(!shall_attach_selfavatar(alice, chat_id).await?);
|
assert!(!should_attach_profile(alice, chat_id).await?);
|
||||||
|
|
||||||
let contact_id = alice.add_or_lookup_contact_id(bob).await;
|
let contact_id = alice.add_or_lookup_contact_id(bob).await;
|
||||||
add_contact_to_chat(alice, chat_id, contact_id).await?;
|
add_contact_to_chat(alice, chat_id, contact_id).await?;
|
||||||
assert!(shall_attach_selfavatar(alice, chat_id).await?);
|
assert!(should_attach_profile(alice, chat_id).await?);
|
||||||
|
|
||||||
chat_id.set_selfavatar_timestamp(alice, time()).await?;
|
chat_id.set_selfavatar_timestamp(alice, time()).await?;
|
||||||
assert!(!shall_attach_selfavatar(alice, chat_id).await?);
|
assert!(!should_attach_profile(alice, chat_id).await?);
|
||||||
|
|
||||||
alice.set_config(Config::Selfavatar, None).await?; // setting to None also forces re-sending
|
alice.set_config(Config::Selfavatar, None).await?; // setting to None also forces re-sending
|
||||||
assert!(shall_attach_selfavatar(alice, chat_id).await?);
|
assert!(should_attach_profile(alice, chat_id).await?);
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1571,7 +1571,7 @@ async fn test_profile_data_on_group_leave() -> Result<()> {
|
|||||||
tokio::fs::write(&file, bytes).await?;
|
tokio::fs::write(&file, bytes).await?;
|
||||||
t.set_config(Config::Selfavatar, Some(file.to_str().unwrap()))
|
t.set_config(Config::Selfavatar, Some(file.to_str().unwrap()))
|
||||||
.await?;
|
.await?;
|
||||||
assert!(shall_attach_selfavatar(t, chat_id).await?);
|
assert!(should_attach_profile(t, chat_id).await?);
|
||||||
|
|
||||||
remove_contact_from_chat(t, chat_id, ContactId::SELF).await?;
|
remove_contact_from_chat(t, chat_id, ContactId::SELF).await?;
|
||||||
let sent_msg = t.pop_sent_msg().await;
|
let sent_msg = t.pop_sent_msg().await;
|
||||||
|
|||||||
@@ -758,6 +758,16 @@ impl Context {
|
|||||||
let better_value;
|
let better_value;
|
||||||
|
|
||||||
match key {
|
match key {
|
||||||
|
Config::Selfstatus => {
|
||||||
|
// Currently we send the self-status in every appropriate message, but in the future
|
||||||
|
// (when most users upgrade to "feat: Don't reset key-contact status if
|
||||||
|
// Chat-User-Avatar header is absent") we want to send it periodically together with
|
||||||
|
// the self-avatar. This ensures the correct behavior after a possible Core upgrade.
|
||||||
|
self.sql
|
||||||
|
.execute("UPDATE contacts SET selfavatar_sent=0", ())
|
||||||
|
.await?;
|
||||||
|
self.sql.set_raw_config(key.as_ref(), value).await?;
|
||||||
|
}
|
||||||
Config::Selfavatar => {
|
Config::Selfavatar => {
|
||||||
self.sql
|
self.sql
|
||||||
.execute("UPDATE contacts SET selfavatar_sent=0;", ())
|
.execute("UPDATE contacts SET selfavatar_sent=0;", ())
|
||||||
|
|||||||
@@ -60,7 +60,12 @@ pub enum HeaderDef {
|
|||||||
ChatGroupNameTimestamp,
|
ChatGroupNameTimestamp,
|
||||||
ChatVerified,
|
ChatVerified,
|
||||||
ChatGroupAvatar,
|
ChatGroupAvatar,
|
||||||
|
|
||||||
|
/// If present, contact's avatar and status should be applied from the message.
|
||||||
|
/// "Chat-User-Avatar: 0" means that the contact has no avatar. Contact's status is transferred
|
||||||
|
/// in the message footer.
|
||||||
ChatUserAvatar,
|
ChatUserAvatar,
|
||||||
|
|
||||||
ChatVoiceMessage,
|
ChatVoiceMessage,
|
||||||
ChatGroupMemberRemoved,
|
ChatGroupMemberRemoved,
|
||||||
ChatGroupMemberAdded,
|
ChatGroupMemberAdded,
|
||||||
|
|||||||
@@ -182,7 +182,7 @@ impl MimeFactory {
|
|||||||
pub async fn from_msg(context: &Context, msg: Message) -> Result<MimeFactory> {
|
pub async fn from_msg(context: &Context, msg: Message) -> Result<MimeFactory> {
|
||||||
let now = time();
|
let now = time();
|
||||||
let chat = Chat::load_from_db(context, msg.chat_id).await?;
|
let chat = Chat::load_from_db(context, msg.chat_id).await?;
|
||||||
let attach_profile_data = Self::should_attach_profile_data(&msg);
|
let can_transfer_profile = Self::can_transfer_profile(&msg);
|
||||||
let undisclosed_recipients = chat.typ == Chattype::OutBroadcast;
|
let undisclosed_recipients = chat.typ == Chattype::OutBroadcast;
|
||||||
|
|
||||||
let from_addr = context.get_primary_self_addr().await?;
|
let from_addr = context.get_primary_self_addr().await?;
|
||||||
@@ -194,7 +194,7 @@ impl MimeFactory {
|
|||||||
if let Some(override_name) = msg.param.get(Param::OverrideSenderDisplayname) {
|
if let Some(override_name) = msg.param.get(Param::OverrideSenderDisplayname) {
|
||||||
(override_name.to_string(), Some(config_displayname))
|
(override_name.to_string(), Some(config_displayname))
|
||||||
} else {
|
} else {
|
||||||
let name = match attach_profile_data {
|
let name = match can_transfer_profile {
|
||||||
true => config_displayname,
|
true => config_displayname,
|
||||||
false => "".to_string(),
|
false => "".to_string(),
|
||||||
};
|
};
|
||||||
@@ -302,7 +302,7 @@ impl MimeFactory {
|
|||||||
} else {
|
} else {
|
||||||
addr
|
addr
|
||||||
};
|
};
|
||||||
let name = match attach_profile_data {
|
let name = match can_transfer_profile {
|
||||||
true => authname,
|
true => authname,
|
||||||
false => "".to_string(),
|
false => "".to_string(),
|
||||||
};
|
};
|
||||||
@@ -451,14 +451,18 @@ impl MimeFactory {
|
|||||||
.split_ascii_whitespace()
|
.split_ascii_whitespace()
|
||||||
.map(|s| s.trim_start_matches('<').trim_end_matches('>').to_string())
|
.map(|s| s.trim_start_matches('<').trim_end_matches('>').to_string())
|
||||||
.collect();
|
.collect();
|
||||||
let selfstatus = match attach_profile_data {
|
let should_attach_profile = Self::should_attach_profile(context, &msg).await;
|
||||||
|
// TODO: (2025-08) Attach self-status in every message for compatibility with older
|
||||||
|
// versions. Should be replaced with
|
||||||
|
// `should_attach_profile || !is_encrypted && can_transfer_profile`.
|
||||||
|
let selfstatus = match can_transfer_profile {
|
||||||
true => context
|
true => context
|
||||||
.get_config(Config::Selfstatus)
|
.get_config(Config::Selfstatus)
|
||||||
.await?
|
.await?
|
||||||
.unwrap_or_default(),
|
.unwrap_or_default(),
|
||||||
false => "".to_string(),
|
false => "".to_string(),
|
||||||
};
|
};
|
||||||
let attach_selfavatar = Self::should_attach_selfavatar(context, &msg).await;
|
let attach_selfavatar = should_attach_profile;
|
||||||
|
|
||||||
ensure_and_debug_assert!(
|
ensure_and_debug_assert!(
|
||||||
member_timestamps.is_empty()
|
member_timestamps.is_empty()
|
||||||
@@ -551,7 +555,7 @@ impl MimeFactory {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn should_attach_profile_data(msg: &Message) -> bool {
|
fn can_transfer_profile(msg: &Message) -> bool {
|
||||||
msg.param.get_cmd() != SystemMessage::SecurejoinMessage || {
|
msg.param.get_cmd() != SystemMessage::SecurejoinMessage || {
|
||||||
let step = msg.param.get(Param::Arg).unwrap_or_default();
|
let step = msg.param.get(Param::Arg).unwrap_or_default();
|
||||||
// Don't attach profile data at the earlier SecureJoin steps:
|
// Don't attach profile data at the earlier SecureJoin steps:
|
||||||
@@ -566,14 +570,14 @@ impl MimeFactory {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn should_attach_selfavatar(context: &Context, msg: &Message) -> bool {
|
async fn should_attach_profile(context: &Context, msg: &Message) -> bool {
|
||||||
Self::should_attach_profile_data(msg)
|
Self::can_transfer_profile(msg)
|
||||||
&& match chat::shall_attach_selfavatar(context, msg.chat_id).await {
|
&& match chat::should_attach_profile(context, msg.chat_id).await {
|
||||||
Ok(should) => should,
|
Ok(should) => should,
|
||||||
Err(err) => {
|
Err(err) => {
|
||||||
warn!(
|
warn!(
|
||||||
context,
|
context,
|
||||||
"should_attach_selfavatar: cannot get selfavatar state: {err:#}."
|
"should_attach_profile: chat::should_attach_profile: {err:#}."
|
||||||
);
|
);
|
||||||
false
|
false
|
||||||
}
|
}
|
||||||
@@ -638,7 +642,7 @@ impl MimeFactory {
|
|||||||
return Ok(format!("Re: {}", remove_subject_prefix(last_subject)));
|
return Ok(format!("Re: {}", remove_subject_prefix(last_subject)));
|
||||||
}
|
}
|
||||||
|
|
||||||
let self_name = match Self::should_attach_profile_data(msg) {
|
let self_name = match Self::can_transfer_profile(msg) {
|
||||||
true => context.get_config(Config::Displayname).await?,
|
true => context.get_config(Config::Displayname).await?,
|
||||||
false => None,
|
false => None,
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -908,8 +908,11 @@ pub(crate) async fn receive_imf_inner(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Ignore footers from mailinglists as they are often created or modified by the mailinglist software.
|
if let Some(footer) = mime_parser.footer.as_ref().filter(|footer| {
|
||||||
if let Some(footer) = &mime_parser.footer {
|
!footer.is_empty() || mime_parser.user_avatar.is_some() || !mime_parser.was_encrypted()
|
||||||
|
}) {
|
||||||
|
// Ignore footers from mailinglists as they are often created or modified by the mailinglist
|
||||||
|
// software.
|
||||||
if !mime_parser.is_mailinglist_message()
|
if !mime_parser.is_mailinglist_message()
|
||||||
&& from_id != ContactId::UNDEFINED
|
&& from_id != ContactId::UNDEFINED
|
||||||
&& context
|
&& context
|
||||||
@@ -923,7 +926,7 @@ pub(crate) async fn receive_imf_inner(
|
|||||||
if let Err(err) = contact::set_status(
|
if let Err(err) = contact::set_status(
|
||||||
context,
|
context,
|
||||||
from_id,
|
from_id,
|
||||||
footer.to_string(),
|
footer.clone(),
|
||||||
mime_parser.was_encrypted(),
|
mime_parser.was_encrypted(),
|
||||||
mime_parser.has_chat_version(),
|
mime_parser.has_chat_version(),
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -2254,6 +2254,41 @@ sig thursday",
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
|
async fn test_contact_status_from_encrypted_msg() -> Result<()> {
|
||||||
|
let mut tcm = TestContextManager::new();
|
||||||
|
let alice = &tcm.alice().await;
|
||||||
|
let bob = &tcm.bob().await;
|
||||||
|
|
||||||
|
let alice_chat_id = alice.create_chat(bob).await.id;
|
||||||
|
let sent = alice.send_text(alice_chat_id, "hi").await;
|
||||||
|
bob.recv_msg(&sent).await;
|
||||||
|
alice.set_config(Config::Selfstatus, Some("status")).await?;
|
||||||
|
let sent = alice.send_text(alice_chat_id, "I've set self-status").await;
|
||||||
|
bob.recv_msg(&sent).await;
|
||||||
|
let bob_alice = bob.add_or_lookup_contact(alice).await;
|
||||||
|
assert_eq!(bob_alice.get_status(), "status");
|
||||||
|
|
||||||
|
alice
|
||||||
|
.set_config(Config::Selfstatus, Some("status1"))
|
||||||
|
.await?;
|
||||||
|
alice
|
||||||
|
.send_text(alice_chat_id, "I changed self-status")
|
||||||
|
.await;
|
||||||
|
|
||||||
|
// Currently we send self-status in every appropriate message.
|
||||||
|
let sent = alice
|
||||||
|
.send_text(alice_chat_id, "This message also contains my status")
|
||||||
|
.await;
|
||||||
|
let parsed_msg = bob.parse_msg(&sent).await;
|
||||||
|
assert!(parsed_msg.was_encrypted());
|
||||||
|
assert!(parsed_msg.get_header(HeaderDef::ChatUserAvatar).is_none());
|
||||||
|
bob.recv_msg(&sent).await;
|
||||||
|
let bob_alice = bob.add_or_lookup_contact(alice).await;
|
||||||
|
assert_eq!(bob_alice.get_status(), "status1");
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
async fn test_chat_assignment_private_classical_reply() {
|
async fn test_chat_assignment_private_classical_reply() {
|
||||||
for outgoing_is_classical in &[true, false] {
|
for outgoing_is_classical in &[true, false] {
|
||||||
|
|||||||
Reference in New Issue
Block a user