Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
feat: Don't update self-{avatar,status} from received messages (#7002)
The normal way of synchronizing self-avatar and -status nowadays is sync messages.
  • Loading branch information
iequidoo committed Oct 25, 2025
commit 221526da639ee25101606899411cab8f6646a2f4
7 changes: 3 additions & 4 deletions src/config/config_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,6 @@ async fn test_sync() -> Result<()> {
Ok(())
}

/// Sync message mustn't be sent if self-{status,avatar} is changed by a self-sent message.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_no_sync_on_self_sent_msg() -> Result<()> {
let mut tcm = TestContextManager::new();
Expand All @@ -288,7 +287,7 @@ async fn test_no_sync_on_self_sent_msg() -> Result<()> {
a.set_config_bool(Config::SyncMsgs, true).await?;
}

let status = "Synced via usual message";
let status = "Sent via usual message";
alice0.set_config(Config::Selfstatus, Some(status)).await?;
alice0.send_sync_msg().await?;
alice0.pop_sent_sync_msg().await;
Expand All @@ -297,7 +296,7 @@ async fn test_no_sync_on_self_sent_msg() -> Result<()> {
tcm.send_recv(alice0, alice1, "hi Alice!").await;
assert_eq!(
alice1.get_config(Config::Selfstatus).await?,
Some(status.to_string())
Some(status1.to_string())
);
sync(alice1, alice0).await;
assert_eq!(
Expand Down Expand Up @@ -328,7 +327,7 @@ async fn test_no_sync_on_self_sent_msg() -> Result<()> {
alice1
.get_config(Config::Selfavatar)
.await?
.filter(|path| path.ends_with(".png"))
.filter(|path| path.ends_with(".jpg"))
.is_some()
);
sync(alice1, alice0).await;
Expand Down
46 changes: 13 additions & 33 deletions src/contact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,19 +383,15 @@ async fn import_vcard_contact(context: &Context, contact: &VcardContact) -> Resu
None => None,
};
if let Some(path) = path {
// Currently this value doesn't matter as we don't import the contact of self.
let was_encrypted = false;
if let Err(e) =
set_profile_image(context, id, &AvatarAction::Change(path), was_encrypted).await
{
if let Err(e) = set_profile_image(context, id, &AvatarAction::Change(path)).await {
warn!(
context,
"import_vcard_contact: Could not set avatar for {}: {e:#}.", contact.addr
);
}
}
if let Some(biography) = &contact.biography {
if let Err(e) = set_status(context, id, biography.to_owned(), false, false).await {
if let Err(e) = set_status(context, id, biography.to_owned()).await {
warn!(
context,
"import_vcard_contact: Could not set biography for {}: {e:#}.", contact.addr
Expand Down Expand Up @@ -1818,39 +1814,29 @@ WHERE type=? AND id IN (
/// The given profile image is expected to be already in the blob directory
/// as profile images can be set only by receiving messages, this should be always the case, however.
///
/// For contact SELF, the image is not saved in the contact-database but as Config::Selfavatar;
/// this typically happens if we see message with our own profile image.
/// For contact SELF, the image is not saved in the contact-database but as Config::Selfavatar.
pub(crate) async fn set_profile_image(
context: &Context,
contact_id: ContactId,
profile_image: &AvatarAction,
was_encrypted: bool,
) -> Result<()> {
let mut contact = Contact::get_by_id(context, contact_id).await?;
let changed = match profile_image {
AvatarAction::Change(profile_image) => {
if contact_id == ContactId::SELF {
if was_encrypted {
context
.set_config_ex(Nosync, Config::Selfavatar, Some(profile_image))
.await?;
} else {
info!(context, "Do not use unencrypted selfavatar.");
}
context
.set_config_ex(Nosync, Config::Selfavatar, Some(profile_image))
.await?;
} else {
contact.param.set(Param::ProfileImage, profile_image);
}
true
}
AvatarAction::Delete => {
if contact_id == ContactId::SELF {
if was_encrypted {
context
.set_config_ex(Nosync, Config::Selfavatar, None)
.await?;
} else {
info!(context, "Do not use unencrypted selfavatar deletion.");
}
context
.set_config_ex(Nosync, Config::Selfavatar, None)
.await?;
} else {
contact.param.remove(Param::ProfileImage);
}
Expand All @@ -1867,22 +1853,16 @@ pub(crate) async fn set_profile_image(

/// Sets contact status.
///
/// For contact SELF, the status is not saved in the contact table, but as Config::Selfstatus. This
/// is only done if message is sent from Delta Chat and it is encrypted, to synchronize signature
/// between Delta Chat devices.
/// For contact SELF, the status is not saved in the contact table, but as Config::Selfstatus.
pub(crate) async fn set_status(
context: &Context,
contact_id: ContactId,
status: String,
encrypted: bool,
has_chat_version: bool,
) -> Result<()> {
if contact_id == ContactId::SELF {
if encrypted && has_chat_version {
context
.set_config_ex(Nosync, Config::Selfstatus, Some(&status))
.await?;
}
context
.set_config_ex(Nosync, Config::Selfstatus, Some(&status))
.await?;
} else {
let mut contact = Contact::get_by_id(context, contact_id).await?;

Expand Down
44 changes: 8 additions & 36 deletions src/contact/contact_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::chat::{Chat, get_chat_contacts, send_text_msg};
use crate::chatlist::Chatlist;
use crate::receive_imf::receive_imf;
use crate::securejoin::get_securejoin_qr;
use crate::test_utils::{self, TestContext, TestContextManager, TimeShiftFalsePositiveNote};
use crate::test_utils::{self, TestContext, TestContextManager, TimeShiftFalsePositiveNote, sync};

#[test]
fn test_contact_id_values() {
Expand Down Expand Up @@ -846,8 +846,7 @@ CCCB 5AA9 F6E1 141C 9431
Ok(())
}

/// Tests that status is synchronized when sending encrypted BCC-self messages and not
/// synchronized when the message is not encrypted.
/// Tests that self-status is not synchronized from outgoing messages.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_synchronize_status() -> Result<()> {
let mut tcm = TestContextManager::new();
Expand All @@ -866,39 +865,22 @@ async fn test_synchronize_status() -> Result<()> {
.await?;
let chat = alice1.create_email_chat(bob).await;

// Alice sends a message to Bob from the first device.
// Alice sends an unencrypted message to Bob from the first device.
send_text_msg(alice1, chat.id, "Hello".to_string()).await?;
let sent_msg = alice1.pop_sent_msg().await;

// Message is not encrypted.
let message = sent_msg.load_from_db().await;
assert!(!message.get_showpadlock());

// Alice's second devices receives a copy of outgoing message.
alice2.recv_msg(&sent_msg).await;

// Bob receives message.
bob.recv_msg(&sent_msg).await;

// Message was not encrypted, so status is not copied.
assert_eq!(alice2.get_config(Config::Selfstatus).await?, default_status);

// Alice sends encrypted message.
let chat = alice1.create_chat(bob).await;
send_text_msg(alice1, chat.id, "Hello".to_string()).await?;
let sent_msg = alice1.pop_sent_msg().await;

// Second message is encrypted.
let message = sent_msg.load_from_db().await;
assert!(message.get_showpadlock());

// Alice's second devices receives a copy of second outgoing message.
alice2.recv_msg(&sent_msg).await;

assert_eq!(
alice2.get_config(Config::Selfstatus).await?,
Some("New status".to_string())
);
assert_eq!(alice2.get_config(Config::Selfstatus).await?, default_status);

Ok(())
}
Expand All @@ -911,9 +893,9 @@ async fn test_selfavatar_changed_event() -> Result<()> {
// Alice has two devices.
let alice1 = &tcm.alice().await;
let alice2 = &tcm.alice().await;

// Bob has one device.
let bob = &tcm.bob().await;
for a in [alice1, alice2] {
a.set_config_bool(Config::SyncMsgs, true).await?;
}

assert_eq!(alice1.get_config(Config::Selfavatar).await?, None);

Expand All @@ -929,17 +911,7 @@ async fn test_selfavatar_changed_event() -> Result<()> {
.get_matching(|e| matches!(e, EventType::SelfavatarChanged))
.await;

// Alice sends a message.
let alice1_chat_id = alice1.create_chat(bob).await.id;
send_text_msg(alice1, alice1_chat_id, "Hello".to_string()).await?;
let sent_msg = alice1.pop_sent_msg().await;

// The message is encrypted.
let message = sent_msg.load_from_db().await;
assert!(message.get_showpadlock());

// Alice's second device receives a copy of the outgoing message.
alice2.recv_msg(&sent_msg).await;
sync(alice1, alice2).await;

// Alice's second device applies the selfavatar.
assert!(alice2.get_config(Config::Selfavatar).await?.is_some());
Expand Down
36 changes: 0 additions & 36 deletions src/reaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -992,42 +992,6 @@ Content-Disposition: reaction\n\
Ok(())
}

/// Regression test for reaction resetting self-status.
///
/// Reactions do not contain the status,
/// but should not result in self-status being reset on other devices.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_reaction_status_multidevice() -> Result<()> {
let mut tcm = TestContextManager::new();
let alice1 = tcm.alice().await;
let alice2 = tcm.alice().await;

alice1
.set_config(Config::Selfstatus, Some("New status"))
.await?;

let alice2_msg = tcm.send_recv(&alice1, &alice2, "Hi!").await;
assert_eq!(
alice2.get_config(Config::Selfstatus).await?.as_deref(),
Some("New status")
);

// Alice reacts to own message from second device,
// first device receives rection.
{
send_reaction(&alice2, alice2_msg.id, "👍").await?;
let msg = alice2.pop_sent_msg().await;
alice1.recv_msg_hidden(&msg).await;
}

// Check that the status is still the same.
assert_eq!(
alice1.get_config(Config::Selfstatus).await?.as_deref(),
Some("New status")
);
Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_send_reaction_multidevice() -> Result<()> {
let mut tcm = TestContextManager::new();
Expand Down
23 changes: 4 additions & 19 deletions src/receive_imf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -886,7 +886,7 @@ pub(crate) async fn receive_imf_inner(
}

if let Some(avatar_action) = &mime_parser.user_avatar {
if from_id != ContactId::UNDEFINED
if !matches!(from_id, ContactId::UNDEFINED | ContactId::SELF)
&& context
.update_contacts_timestamp(
from_id,
Expand All @@ -895,14 +895,7 @@ pub(crate) async fn receive_imf_inner(
)
.await?
{
if let Err(err) = contact::set_profile_image(
context,
from_id,
avatar_action,
mime_parser.was_encrypted(),
)
.await
{
if let Err(err) = contact::set_profile_image(context, from_id, avatar_action).await {
warn!(context, "receive_imf cannot update profile image: {err:#}.");
};
}
Expand All @@ -914,7 +907,7 @@ pub(crate) async fn receive_imf_inner(
// Ignore footers from mailinglists as they are often created or modified by the mailinglist
// software.
if !mime_parser.is_mailinglist_message()
&& from_id != ContactId::UNDEFINED
&& !matches!(from_id, ContactId::UNDEFINED | ContactId::SELF)
&& context
.update_contacts_timestamp(
from_id,
Expand All @@ -923,15 +916,7 @@ pub(crate) async fn receive_imf_inner(
)
.await?
{
if let Err(err) = contact::set_status(
context,
from_id,
footer.clone(),
mime_parser.was_encrypted(),
mime_parser.has_chat_version(),
)
.await
{
if let Err(err) = contact::set_status(context, from_id, footer.clone()).await {
warn!(context, "Cannot update contact status: {err:#}.");
}
}
Expand Down