diff --git a/python/src/deltachat/chat.py b/python/src/deltachat/chat.py index 4924fee15..70dfd4dcc 100644 --- a/python/src/deltachat/chat.py +++ b/python/src/deltachat/chat.py @@ -3,7 +3,7 @@ import mimetypes import calendar import json -from datetime import datetime +from datetime import datetime, timezone import os from .cutil import as_dc_charpointer, from_dc_charpointer, iter_array from .capi import lib, ffi @@ -512,8 +512,9 @@ class Chat(object): latitude=lib.dc_array_get_latitude(dc_array, i), longitude=lib.dc_array_get_longitude(dc_array, i), accuracy=lib.dc_array_get_accuracy(dc_array, i), - timestamp=datetime.utcfromtimestamp( - lib.dc_array_get_timestamp(dc_array, i) + timestamp=datetime.fromtimestamp( + lib.dc_array_get_timestamp(dc_array, i), + timezone.utc ), marker=from_dc_charpointer(lib.dc_array_get_marker(dc_array, i)), ) diff --git a/python/src/deltachat/cutil.py b/python/src/deltachat/cutil.py index b230276b9..4ef53e036 100644 --- a/python/src/deltachat/cutil.py +++ b/python/src/deltachat/cutil.py @@ -1,6 +1,6 @@ from .capi import lib from .capi import ffi -from datetime import datetime +from datetime import datetime, timezone def as_dc_charpointer(obj): @@ -44,4 +44,4 @@ class DCLot: ts = lib.dc_lot_get_timestamp(self._dc_lot) if ts == 0: return None - return datetime.utcfromtimestamp(ts) + return datetime.fromtimestamp(ts, timezone.utc) diff --git a/python/src/deltachat/message.py b/python/src/deltachat/message.py index ff82677c1..8bb06252a 100644 --- a/python/src/deltachat/message.py +++ b/python/src/deltachat/message.py @@ -6,7 +6,7 @@ from . import props from .cutil import from_dc_charpointer, as_dc_charpointer from .capi import lib, ffi from . import const -from datetime import datetime +from datetime import datetime, timezone class Message(object): @@ -170,7 +170,7 @@ class Message(object): :returns: naive datetime.datetime() object. """ ts = lib.dc_msg_get_timestamp(self._dc_msg) - return datetime.utcfromtimestamp(ts) + return datetime.fromtimestamp(ts, timezone.utc) @props.with_doc def time_received(self): @@ -180,7 +180,7 @@ class Message(object): """ ts = lib.dc_msg_get_received_timestamp(self._dc_msg) if ts: - return datetime.utcfromtimestamp(ts) + return datetime.fromtimestamp(ts, timezone.utc) @props.with_doc def ephemeral_timer(self): @@ -200,7 +200,7 @@ class Message(object): """ ts = lib.dc_msg_get_ephemeral_timestamp(self._dc_msg) if ts: - return datetime.utcfromtimestamp(ts) + return datetime.fromtimestamp(ts, timezone.utc) @property def quoted_text(self): diff --git a/python/tests/test_account.py b/python/tests/test_account.py index a0fd4d224..e409e73a9 100644 --- a/python/tests/test_account.py +++ b/python/tests/test_account.py @@ -10,7 +10,7 @@ from deltachat.tracker import ImexTracker from deltachat.hookspec import account_hookimpl from deltachat.capi import ffi, lib from deltachat.cutil import iter_array -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone @pytest.mark.parametrize("msgtext,res", [ @@ -447,7 +447,7 @@ class TestOfflineChat: contact1.create_chat().send_text("hello") def test_chat_message_distinctions(self, ac1, chat1): - past1s = datetime.utcnow() - timedelta(seconds=1) + past1s = datetime.now(timezone.utc) - timedelta(seconds=1) msg = chat1.send_text("msg1") ts = msg.time_sent assert msg.time_received is None @@ -1221,6 +1221,40 @@ class TestOnlineAccount: assert not msg.is_encrypted() ac1._evtracker.get_matching("DC_EVENT_SMTP_MESSAGE_SENT") + def test_gossip_optimization(self, acfactory, lp): + """Test that gossip timestamp is updated when someone else sends gossip, + so we don't have to send gossip ourselves. + """ + ac1, ac2, ac3 = acfactory.get_many_online_accounts(3) + + acfactory.introduce_each_other([ac1, ac2]) + acfactory.introduce_each_other([ac2, ac3]) + + lp.sec("ac1 creates a group chat with ac2") + group_chat = ac1.create_group_chat("hello") + group_chat.add_contact(ac2) + msg = group_chat.send_text("hi") + + # No Autocrypt gossip was sent yet. + gossiped_timestamp = msg.chat.get_summary()["gossiped_timestamp"] + assert gossiped_timestamp == 0 + + msg = ac2._evtracker.wait_next_incoming_message() + assert msg.is_encrypted() + assert msg.text == "hi" + + lp.sec("ac2 adds ac3 to the group") + msg.chat.add_contact(ac3) + + lp.sec("ac1 receives message from ac2 and updates gossip timestamp") + msg = ac1._evtracker.wait_next_incoming_message() + assert msg.is_encrypted() + + # ac1 has updated the gossip timestamp even though no gossip was sent by ac1. + # ac1 does not need to send gossip because ac2 already did it. + gossiped_timestamp = msg.chat.get_summary()["gossiped_timestamp"] + assert gossiped_timestamp == int(msg.time_sent.timestamp()) + def test_gossip_encryption_preference(self, acfactory, lp): """Test that encryption preference of group members is gossiped to new members. This is a Delta Chat extension to Autocrypt 1.1.0, which Autocrypt-Gossip headers @@ -2163,7 +2197,7 @@ class TestOnlineAccount: break # DC is done with reading messages def test_send_receive_locations(self, acfactory, lp): - now = datetime.utcnow() + now = datetime.now(timezone.utc) ac1, ac2 = acfactory.get_two_online_accounts() lp.sec("ac1: create chat with ac2") diff --git a/src/chat.rs b/src/chat.rs index 5779db426..5764b9ab0 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -397,7 +397,7 @@ impl ChatId { context.emit_event(EventType::ChatModified(self)); // make sure, the receivers will get all keys - reset_gossiped_timestamp(context, self).await?; + self.reset_gossiped_timestamp(context).await?; Ok(()) } @@ -857,6 +857,48 @@ impl ChatId { pub fn to_u32(self) -> u32 { self.0 } + + pub(crate) async fn reset_gossiped_timestamp(self, context: &Context) -> Result<()> { + self.set_gossiped_timestamp(context, 0).await + } + + /// Get timestamp of the last gossip sent in the chat. + /// Zero return value means that gossip was never sent. + pub async fn get_gossiped_timestamp(self, context: &Context) -> Result { + let timestamp: Option = context + .sql + .query_get_value( + "SELECT gossiped_timestamp FROM chats WHERE id=?;", + paramsv![self], + ) + .await?; + Ok(timestamp.unwrap_or_default()) + } + + pub(crate) async fn set_gossiped_timestamp( + self, + context: &Context, + timestamp: i64, + ) -> Result<()> { + ensure!( + !self.is_special(), + "can not set gossiped timestamp for special chats" + ); + info!( + context, + "set gossiped_timestamp for chat {} to {}.", self, timestamp, + ); + + context + .sql + .execute( + "UPDATE chats SET gossiped_timestamp=? WHERE id=?;", + paramsv![timestamp, self], + ) + .await?; + + Ok(()) + } } impl std::fmt::Display for ChatId { @@ -1057,10 +1099,6 @@ impl Chat { Ok(None) } - pub async fn get_gossiped_timestamp(&self, context: &Context) -> Result { - get_gossiped_timestamp(context, self.id).await - } - pub async fn get_color(&self, context: &Context) -> Result { let mut color = 0; @@ -1093,7 +1131,7 @@ impl Chat { name: self.name.clone(), archived: self.visibility == ChatVisibility::Archived, param: self.param.to_string(), - gossiped_timestamp: self.get_gossiped_timestamp(context).await?, + gossiped_timestamp: self.id.get_gossiped_timestamp(context).await?, is_sending_locations: self.is_sending_locations, color: self.get_color(context).await?, profile_image: self @@ -2371,7 +2409,7 @@ pub(crate) async fn add_contact_to_chat_ex( let contact = Contact::get_by_id(context, contact_id).await?; let mut msg = Message::default(); - reset_gossiped_timestamp(context, chat_id).await?; + chat_id.reset_gossiped_timestamp(context).await?; /*this also makes sure, not contacts are added to special or normal chats*/ let mut chat = Chat::load_from_db(context, chat_id).await?; @@ -2453,45 +2491,6 @@ pub(crate) async fn add_contact_to_chat_ex( Ok(true) } -pub(crate) async fn reset_gossiped_timestamp(context: &Context, chat_id: ChatId) -> Result<()> { - set_gossiped_timestamp(context, chat_id, 0).await -} - -/// Get timestamp of the last gossip sent in the chat. -/// Zero return value means that gossip was never sent. -pub async fn get_gossiped_timestamp(context: &Context, chat_id: ChatId) -> Result { - let timestamp: Option = context - .sql - .query_get_value( - "SELECT gossiped_timestamp FROM chats WHERE id=?;", - paramsv![chat_id], - ) - .await?; - Ok(timestamp.unwrap_or_default()) -} - -pub(crate) async fn set_gossiped_timestamp( - context: &Context, - chat_id: ChatId, - timestamp: i64, -) -> Result<()> { - ensure!(!chat_id.is_special(), "can not add member to special chats"); - info!( - context, - "set gossiped_timestamp for chat #{} to {}.", chat_id, timestamp, - ); - - context - .sql - .execute( - "UPDATE chats SET gossiped_timestamp=? WHERE id=?;", - paramsv![timestamp, chat_id], - ) - .await?; - - Ok(()) -} - pub(crate) async fn shall_attach_selfavatar(context: &Context, chat_id: ChatId) -> Result { // versions before 12/2019 already allowed to set selfavatar, however, it was never sent to others. // to avoid sending out previously set selfavatars unexpectedly we added this additional check. diff --git a/src/dc_receive_imf.rs b/src/dc_receive_imf.rs index 480292e33..4a40771e2 100644 --- a/src/dc_receive_imf.rs +++ b/src/dc_receive_imf.rs @@ -212,6 +212,26 @@ pub(crate) async fn dc_receive_imf_inner( .await .map_err(|err| err.context("add_parts error"))?; + // Update gossiped timestamp for the chat if someone else or our other device sent + // Autocrypt-Gossip for all recipients in the chat to avoid sending Autocrypt-Gossip ourselves + // and waste traffic. + if !chat_id.is_special() + && mime_parser + .recipients + .iter() + .all(|recipient| mime_parser.gossiped_addr.contains(&recipient.addr)) + { + info!( + context, + "Received message contains Autocrypt-Gossip for all members, updating timestamp." + ); + if chat_id.get_gossiped_timestamp(context).await? < sent_timestamp { + chat_id + .set_gossiped_timestamp(context, sent_timestamp) + .await?; + } + } + let insert_msg_id = if let Some((_chat_id, msg_id)) = created_db_entries.last() { *msg_id } else { @@ -2044,7 +2064,7 @@ async fn check_verified_properties( let peerstate = Peerstate::from_addr(context, &to_addr).await?; // mark gossiped keys (if any) as verified - if mimeparser.gossipped_addr.contains(&to_addr) { + if mimeparser.gossiped_addr.contains(&to_addr) { if let Some(mut peerstate) = peerstate { // if we're here, we know the gossip key is verified: // - use the gossip-key as verified-key if there is no verified-key diff --git a/src/job.rs b/src/job.rs index 1dbc44a3d..0e579813e 100644 --- a/src/job.rs +++ b/src/job.rs @@ -991,7 +991,7 @@ pub async fn send_msg_job(context: &Context, msg_id: MsgId) -> Result MimeFactory<'a> { match &self.loaded { Loaded::Message { chat } => { // beside key- and member-changes, force re-gossip every 48 hours - let gossiped_timestamp = chat.get_gossiped_timestamp(context).await?; + let gossiped_timestamp = chat.id.get_gossiped_timestamp(context).await?; if time() > gossiped_timestamp + (2 * 24 * 60 * 60) { Ok(true) } else { diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 377235c0f..aec43b48a 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -57,7 +57,9 @@ pub struct MimeMessage { /// this set is empty. pub signatures: HashSet, - pub gossipped_addr: HashSet, + /// The set of mail recipient addresses for which gossip headers were applied, regardless of + /// whether they modified any peerstates. + pub gossiped_addr: HashSet, pub is_forwarded: bool, pub is_system_message: SystemMessage, pub location_kml: Option, @@ -198,7 +200,7 @@ impl MimeMessage { // Memory location for a possible decrypted message. let mut mail_raw = Vec::new(); - let mut gossipped_addr = Default::default(); + let mut gossiped_addr = Default::default(); let (mail, signatures, warn_empty_signature) = match e2ee::try_decrypt(context, &mail, message_time).await { @@ -221,7 +223,7 @@ impl MimeMessage { if !signatures.is_empty() { let gossip_headers = decrypted_mail.headers.get_all_values("Autocrypt-Gossip"); - gossipped_addr = update_gossip_peerstates( + gossiped_addr = update_gossip_peerstates( context, message_time, &mail, @@ -279,7 +281,7 @@ impl MimeMessage { // only non-empty if it was a valid autocrypt message signatures, - gossipped_addr, + gossiped_addr, is_forwarded: false, mdn_reports: Vec::new(), is_system_message: SystemMessage::Unknown, @@ -1380,6 +1382,9 @@ impl MimeMessage { } } +/// Parses `Autocrypt-Gossip` headers from the email and applies them to peerstates. +/// +/// Returns the set of mail recipient addresses for which valid gossip headers were found. async fn update_gossip_peerstates( context: &Context, message_time: i64, @@ -1387,42 +1392,46 @@ async fn update_gossip_peerstates( gossip_headers: Vec, ) -> Result> { // XXX split the parsing from the modification part - let mut gossipped_addr: HashSet = Default::default(); + let mut gossiped_addr: HashSet = Default::default(); for value in &gossip_headers { - let gossip_header = value.parse::(); - - if let Ok(ref header) = gossip_header { - if get_recipients(&mail.headers) - .iter() - .any(|info| info.addr == header.addr.to_lowercase()) - { - let mut peerstate = Peerstate::from_addr(context, &header.addr).await?; - if let Some(ref mut peerstate) = peerstate { - peerstate.apply_gossip(header, message_time); - peerstate.save_to_db(&context.sql, false).await?; - } else { - let p = Peerstate::from_gossip(header, message_time); - p.save_to_db(&context.sql, true).await?; - peerstate = Some(p); - } - if let Some(peerstate) = peerstate { - peerstate - .handle_fingerprint_change(context, message_time) - .await?; - } - - gossipped_addr.insert(header.addr.clone()); - } else { - warn!( - context, - "Ignoring gossipped \"{}\" as the address is not in To/Cc list.", &header.addr, - ); + let header = match value.parse::() { + Ok(header) => header, + Err(err) => { + warn!(context, "Failed parsing Autocrypt-Gossip header: {}", err); + continue; } + }; + + if !get_recipients(&mail.headers) + .iter() + .any(|info| info.addr == header.addr.to_lowercase()) + { + warn!( + context, + "Ignoring gossiped \"{}\" as the address is not in To/Cc list.", &header.addr, + ); + continue; } + + let peerstate; + if let Some(mut p) = Peerstate::from_addr(context, &header.addr).await? { + p.apply_gossip(&header, message_time); + p.save_to_db(&context.sql, false).await?; + peerstate = p; + } else { + let p = Peerstate::from_gossip(&header, message_time); + p.save_to_db(&context.sql, true).await?; + peerstate = p; + }; + peerstate + .handle_fingerprint_change(context, message_time) + .await?; + + gossiped_addr.insert(header.addr.clone()); } - Ok(gossipped_addr) + Ok(gossiped_addr) } #[derive(Debug)]