Skip to content

Commit 3e0a181

Browse files
committed
feat: Don't update self-{avatar,status} from received messages
The normal way of synchronizing self-avatar and -status nowadays is sync messages.
1 parent 14e5c35 commit 3e0a181

File tree

5 files changed

+28
-128
lines changed

5 files changed

+28
-128
lines changed

src/config/config_tests.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,6 @@ async fn test_sync() -> Result<()> {
278278
Ok(())
279279
}
280280

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

291-
let status = "Synced via usual message";
290+
let status = "Sent via usual message";
292291
alice0.set_config(Config::Selfstatus, Some(status)).await?;
293292
alice0.send_sync_msg().await?;
294293
alice0.pop_sent_sync_msg().await;
@@ -297,7 +296,7 @@ async fn test_no_sync_on_self_sent_msg() -> Result<()> {
297296
tcm.send_recv(alice0, alice1, "hi Alice!").await;
298297
assert_eq!(
299298
alice1.get_config(Config::Selfstatus).await?,
300-
Some(status.to_string())
299+
Some(status1.to_string())
301300
);
302301
sync(alice1, alice0).await;
303302
assert_eq!(
@@ -328,7 +327,7 @@ async fn test_no_sync_on_self_sent_msg() -> Result<()> {
328327
alice1
329328
.get_config(Config::Selfavatar)
330329
.await?
331-
.filter(|path| path.ends_with(".png"))
330+
.filter(|path| path.ends_with(".jpg"))
332331
.is_some()
333332
);
334333
sync(alice1, alice0).await;

src/contact.rs

Lines changed: 13 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -383,19 +383,15 @@ async fn import_vcard_contact(context: &Context, contact: &VcardContact) -> Resu
383383
None => None,
384384
};
385385
if let Some(path) = path {
386-
// Currently this value doesn't matter as we don't import the contact of self.
387-
let was_encrypted = false;
388-
if let Err(e) =
389-
set_profile_image(context, id, &AvatarAction::Change(path), was_encrypted).await
390-
{
386+
if let Err(e) = set_profile_image(context, id, &AvatarAction::Change(path)).await {
391387
warn!(
392388
context,
393389
"import_vcard_contact: Could not set avatar for {}: {e:#}.", contact.addr
394390
);
395391
}
396392
}
397393
if let Some(biography) = &contact.biography {
398-
if let Err(e) = set_status(context, id, biography.to_owned(), false, false).await {
394+
if let Err(e) = set_status(context, id, biography.to_owned()).await {
399395
warn!(
400396
context,
401397
"import_vcard_contact: Could not set biography for {}: {e:#}.", contact.addr
@@ -1835,39 +1831,29 @@ WHERE type=? AND id IN (
18351831
/// The given profile image is expected to be already in the blob directory
18361832
/// as profile images can be set only by receiving messages, this should be always the case, however.
18371833
///
1838-
/// For contact SELF, the image is not saved in the contact-database but as Config::Selfavatar;
1839-
/// this typically happens if we see message with our own profile image.
1834+
/// For contact SELF, the image is not saved in the contact-database but as Config::Selfavatar.
18401835
pub(crate) async fn set_profile_image(
18411836
context: &Context,
18421837
contact_id: ContactId,
18431838
profile_image: &AvatarAction,
1844-
was_encrypted: bool,
18451839
) -> Result<()> {
18461840
let mut contact = Contact::get_by_id(context, contact_id).await?;
18471841
let changed = match profile_image {
18481842
AvatarAction::Change(profile_image) => {
18491843
if contact_id == ContactId::SELF {
1850-
if was_encrypted {
1851-
context
1852-
.set_config_ex(Nosync, Config::Selfavatar, Some(profile_image))
1853-
.await?;
1854-
} else {
1855-
info!(context, "Do not use unencrypted selfavatar.");
1856-
}
1844+
context
1845+
.set_config_ex(Nosync, Config::Selfavatar, Some(profile_image))
1846+
.await?;
18571847
} else {
18581848
contact.param.set(Param::ProfileImage, profile_image);
18591849
}
18601850
true
18611851
}
18621852
AvatarAction::Delete => {
18631853
if contact_id == ContactId::SELF {
1864-
if was_encrypted {
1865-
context
1866-
.set_config_ex(Nosync, Config::Selfavatar, None)
1867-
.await?;
1868-
} else {
1869-
info!(context, "Do not use unencrypted selfavatar deletion.");
1870-
}
1854+
context
1855+
.set_config_ex(Nosync, Config::Selfavatar, None)
1856+
.await?;
18711857
} else {
18721858
contact.param.remove(Param::ProfileImage);
18731859
}
@@ -1884,22 +1870,16 @@ pub(crate) async fn set_profile_image(
18841870

18851871
/// Sets contact status.
18861872
///
1887-
/// For contact SELF, the status is not saved in the contact table, but as Config::Selfstatus. This
1888-
/// is only done if message is sent from Delta Chat and it is encrypted, to synchronize signature
1889-
/// between Delta Chat devices.
1873+
/// For contact SELF, the status is not saved in the contact table, but as Config::Selfstatus.
18901874
pub(crate) async fn set_status(
18911875
context: &Context,
18921876
contact_id: ContactId,
18931877
status: String,
1894-
encrypted: bool,
1895-
has_chat_version: bool,
18961878
) -> Result<()> {
18971879
if contact_id == ContactId::SELF {
1898-
if encrypted && has_chat_version {
1899-
context
1900-
.set_config_ex(Nosync, Config::Selfstatus, Some(&status))
1901-
.await?;
1902-
}
1880+
context
1881+
.set_config_ex(Nosync, Config::Selfstatus, Some(&status))
1882+
.await?;
19031883
} else {
19041884
let mut contact = Contact::get_by_id(context, contact_id).await?;
19051885

src/contact/contact_tests.rs

Lines changed: 8 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use super::*;
44
use crate::chat::{Chat, get_chat_contacts, send_text_msg};
55
use crate::chatlist::Chatlist;
66
use crate::receive_imf::receive_imf;
7-
use crate::test_utils::{self, TestContext, TestContextManager, TimeShiftFalsePositiveNote};
7+
use crate::test_utils::{self, TestContext, TestContextManager, TimeShiftFalsePositiveNote, sync};
88

99
#[test]
1010
fn test_contact_id_values() {
@@ -831,8 +831,7 @@ CCCB 5AA9 F6E1 141C 9431
831831
Ok(())
832832
}
833833

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

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

858-
// Message is not encrypted.
859-
let message = sent_msg.load_from_db().await;
860-
assert!(!message.get_showpadlock());
861-
862857
// Alice's second devices receives a copy of outgoing message.
863858
alice2.recv_msg(&sent_msg).await;
864-
865-
// Bob receives message.
866-
bob.recv_msg(&sent_msg).await;
867-
868-
// Message was not encrypted, so status is not copied.
869859
assert_eq!(alice2.get_config(Config::Selfstatus).await?, default_status);
870860

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

876-
// Second message is encrypted.
877-
let message = sent_msg.load_from_db().await;
878-
assert!(message.get_showpadlock());
879-
880866
// Alice's second devices receives a copy of second outgoing message.
881867
alice2.recv_msg(&sent_msg).await;
882-
883-
assert_eq!(
884-
alice2.get_config(Config::Selfstatus).await?,
885-
Some("New status".to_string())
886-
);
868+
assert_eq!(alice2.get_config(Config::Selfstatus).await?, default_status);
887869

888870
Ok(())
889871
}
@@ -896,9 +878,9 @@ async fn test_selfavatar_changed_event() -> Result<()> {
896878
// Alice has two devices.
897879
let alice1 = &tcm.alice().await;
898880
let alice2 = &tcm.alice().await;
899-
900-
// Bob has one device.
901-
let bob = &tcm.bob().await;
881+
for a in [alice1, alice2] {
882+
a.set_config_bool(Config::SyncMsgs, true).await?;
883+
}
902884

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

@@ -914,17 +896,7 @@ async fn test_selfavatar_changed_event() -> Result<()> {
914896
.get_matching(|e| matches!(e, EventType::SelfavatarChanged))
915897
.await;
916898

917-
// Alice sends a message.
918-
let alice1_chat_id = alice1.create_chat(bob).await.id;
919-
send_text_msg(alice1, alice1_chat_id, "Hello".to_string()).await?;
920-
let sent_msg = alice1.pop_sent_msg().await;
921-
922-
// The message is encrypted.
923-
let message = sent_msg.load_from_db().await;
924-
assert!(message.get_showpadlock());
925-
926-
// Alice's second device receives a copy of the outgoing message.
927-
alice2.recv_msg(&sent_msg).await;
899+
sync(alice1, alice2).await;
928900

929901
// Alice's second device applies the selfavatar.
930902
assert!(alice2.get_config(Config::Selfavatar).await?.is_some());

src/reaction.rs

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -962,42 +962,6 @@ Here's my footer -- [email protected]"
962962
Ok(())
963963
}
964964

965-
/// Regression test for reaction resetting self-status.
966-
///
967-
/// Reactions do not contain the status,
968-
/// but should not result in self-status being reset on other devices.
969-
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
970-
async fn test_reaction_status_multidevice() -> Result<()> {
971-
let mut tcm = TestContextManager::new();
972-
let alice1 = tcm.alice().await;
973-
let alice2 = tcm.alice().await;
974-
975-
alice1
976-
.set_config(Config::Selfstatus, Some("New status"))
977-
.await?;
978-
979-
let alice2_msg = tcm.send_recv(&alice1, &alice2, "Hi!").await;
980-
assert_eq!(
981-
alice2.get_config(Config::Selfstatus).await?.as_deref(),
982-
Some("New status")
983-
);
984-
985-
// Alice reacts to own message from second device,
986-
// first device receives rection.
987-
{
988-
send_reaction(&alice2, alice2_msg.id, "👍").await?;
989-
let msg = alice2.pop_sent_msg().await;
990-
alice1.recv_msg_hidden(&msg).await;
991-
}
992-
993-
// Check that the status is still the same.
994-
assert_eq!(
995-
alice1.get_config(Config::Selfstatus).await?.as_deref(),
996-
Some("New status")
997-
);
998-
Ok(())
999-
}
1000-
1001965
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
1002966
async fn test_send_reaction_multidevice() -> Result<()> {
1003967
let mut tcm = TestContextManager::new();

src/receive_imf.rs

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -918,7 +918,7 @@ pub(crate) async fn receive_imf_inner(
918918
}
919919

920920
if let Some(avatar_action) = &mime_parser.user_avatar {
921-
if from_id != ContactId::UNDEFINED
921+
if !matches!(from_id, ContactId::UNDEFINED | ContactId::SELF)
922922
&& context
923923
.update_contacts_timestamp(
924924
from_id,
@@ -927,14 +927,7 @@ pub(crate) async fn receive_imf_inner(
927927
)
928928
.await?
929929
{
930-
if let Err(err) = contact::set_profile_image(
931-
context,
932-
from_id,
933-
avatar_action,
934-
mime_parser.was_encrypted(),
935-
)
936-
.await
937-
{
930+
if let Err(err) = contact::set_profile_image(context, from_id, avatar_action).await {
938931
warn!(context, "receive_imf cannot update profile image: {err:#}.");
939932
};
940933
}
@@ -946,7 +939,7 @@ pub(crate) async fn receive_imf_inner(
946939
// Ignore footers from mailinglists as they are often created or modified by the mailinglist
947940
// software.
948941
if !mime_parser.is_mailinglist_message()
949-
&& from_id != ContactId::UNDEFINED
942+
&& !matches!(from_id, ContactId::UNDEFINED | ContactId::SELF)
950943
&& context
951944
.update_contacts_timestamp(
952945
from_id,
@@ -955,15 +948,7 @@ pub(crate) async fn receive_imf_inner(
955948
)
956949
.await?
957950
{
958-
if let Err(err) = contact::set_status(
959-
context,
960-
from_id,
961-
footer.clone(),
962-
mime_parser.was_encrypted(),
963-
mime_parser.has_chat_version(),
964-
)
965-
.await
966-
{
951+
if let Err(err) = contact::set_status(context, from_id, footer.clone()).await {
967952
warn!(context, "Cannot update contact status: {err:#}.");
968953
}
969954
}

0 commit comments

Comments
 (0)