From f290fe08713ce834e0cd713ad9557997563682e8 Mon Sep 17 00:00:00 2001 From: link2xt Date: Sun, 24 Sep 2023 15:38:42 +0000 Subject: [PATCH 01/17] fix: wrap base64-encoded parts to 76 characters This is an RFC 2045 requirement for base64-encoded MIME parts. Previously referenced RFC 5322 requirement is a general Internet Message Format requirement and is more generous. --- src/mimefactory.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 724340348..996ea3fd7 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -1363,15 +1363,16 @@ impl<'a> MimeFactory<'a> { } } -/// Returns base64-encoded buffer `buf` split into 78-bytes long +/// Returns base64-encoded buffer `buf` split into 76-bytes long /// chunks separated by CRLF. /// -/// This line length limit is an -/// [RFC5322 requirement](https://tools.ietf.org/html/rfc5322#section-2.1.1). +/// [RFC2045 specification of base64 Content-Transfer-Encoding](https://datatracker.ietf.org/doc/html/rfc2045#section-6.8) +/// says that "The encoded output stream must be represented in lines of no more than 76 characters each." +/// Longer lines trigger `BASE64_LENGTH_78_79` rule of SpamAssassin. pub(crate) fn wrapped_base64_encode(buf: &[u8]) -> String { let base64 = base64::engine::general_purpose::STANDARD.encode(buf); let mut chars = base64.chars(); - std::iter::repeat_with(|| chars.by_ref().take(78).collect::()) + std::iter::repeat_with(|| chars.by_ref().take(76).collect::()) .take_while(|s| !s.is_empty()) .collect::>() .join("\r\n") @@ -1611,8 +1612,8 @@ mod tests { fn test_wrapped_base64_encode() { let input = b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; let output = - "QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQU\r\n\ - FBQUFBQUFBQQ=="; + "QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFB\r\n\ + QUFBQUFBQUFBQQ=="; assert_eq!(wrapped_base64_encode(input), output); } From a1345f2542679ff44846d76739d8abe5864e1eb5 Mon Sep 17 00:00:00 2001 From: link2xt Date: Mon, 25 Sep 2023 09:30:00 +0000 Subject: [PATCH 02/17] refactor: flatten `lookup_chat_by_reply` --- src/receive_imf.rs | 89 ++++++++++++++++++++++------------------------ 1 file changed, 43 insertions(+), 46 deletions(-) diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 889bb1175..d7b8bfe55 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -1408,56 +1408,53 @@ async fn lookup_chat_by_reply( ) -> Result> { // Try to assign message to the same chat as the parent message. - if let Some(parent) = parent { - let parent_chat = Chat::load_from_db(context, parent.chat_id).await?; + let Some(parent) = parent else { + return Ok(None); + }; - if parent.download_state != DownloadState::Done - // TODO (2023-09-12): Added for backward compatibility with versions that did not have - // `DownloadState::Undecipherable`. Remove eventually with the comment in - // `MimeMessage::from_bytes()`. - || parent - .error - .as_ref() - .filter(|e| e.starts_with("Decrypting failed:")) - .is_some() - { - // If the parent msg is not fully downloaded or undecipherable, it may have been - // assigned to the wrong chat (they often get assigned to the 1:1 chat with the sender). - return Ok(None); - } + let parent_chat = Chat::load_from_db(context, parent.chat_id).await?; - if parent_chat.id == DC_CHAT_ID_TRASH { - return Ok(None); - } - - // If this was a private message just to self, it was probably a private reply. - // It should not go into the group then, but into the private chat. - if is_probably_private_reply(context, to_ids, from_id, mime_parser, parent_chat.id).await? { - return Ok(None); - } - - // If the parent chat is a 1:1 chat, and the sender is a classical MUA and added - // a new person to TO/CC, then the message should not go to the 1:1 chat, but to a - // newly created ad-hoc group. - if parent_chat.typ == Chattype::Single - && !mime_parser.has_chat_version() - && to_ids.len() > 1 - { - let mut chat_contacts = chat::get_chat_contacts(context, parent_chat.id).await?; - chat_contacts.push(ContactId::SELF); - if to_ids.iter().any(|id| !chat_contacts.contains(id)) { - return Ok(None); - } - } - - info!( - context, - "Assigning message to {} as it's a reply to {}.", parent_chat.id, parent.rfc724_mid - ); - return Ok(Some((parent_chat.id, parent_chat.blocked))); + if parent.download_state != DownloadState::Done + // TODO (2023-09-12): Added for backward compatibility with versions that did not have + // `DownloadState::Undecipherable`. Remove eventually with the comment in + // `MimeMessage::from_bytes()`. + || parent + .error + .as_ref() + .filter(|e| e.starts_with("Decrypting failed:")) + .is_some() + { + // If the parent msg is not fully downloaded or undecipherable, it may have been + // assigned to the wrong chat (they often get assigned to the 1:1 chat with the sender). + return Ok(None); } - Ok(None) + if parent_chat.id == DC_CHAT_ID_TRASH { + return Ok(None); + } + + // If this was a private message just to self, it was probably a private reply. + // It should not go into the group then, but into the private chat. + if is_probably_private_reply(context, to_ids, from_id, mime_parser, parent_chat.id).await? { + return Ok(None); + } + + // If the parent chat is a 1:1 chat, and the sender is a classical MUA and added + // a new person to TO/CC, then the message should not go to the 1:1 chat, but to a + // newly created ad-hoc group. + if parent_chat.typ == Chattype::Single && !mime_parser.has_chat_version() && to_ids.len() > 1 { + let mut chat_contacts = chat::get_chat_contacts(context, parent_chat.id).await?; + chat_contacts.push(ContactId::SELF); + if to_ids.iter().any(|id| !chat_contacts.contains(id)) { + return Ok(None); + } + } + + info!( + context, + "Assigning message to {} as it's a reply to {}.", parent_chat.id, parent.rfc724_mid + ); + Ok(Some((parent_chat.id, parent_chat.blocked))) } /// If this method returns true, the message shall be assigned to the 1:1 chat with the sender. From d0ee21e6dcf0ffe9bf6120b5e7dd6c4d4beb0d59 Mon Sep 17 00:00:00 2001 From: link2xt Date: Mon, 25 Sep 2023 09:30:36 +0000 Subject: [PATCH 03/17] refactor: flatten `GENERATED_PREFIX` check in `receive_imf_inner` --- src/receive_imf.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/receive_imf.rs b/src/receive_imf.rs index d7b8bfe55..2011f39b2 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -113,21 +113,20 @@ pub(crate) async fn receive_imf_inner( { Err(err) => { warn!(context, "receive_imf: can't parse MIME: {err:#}."); - let msg_ids; - if !rfc724_mid.starts_with(GENERATED_PREFIX) { - let row_id = context - .sql - .execute( - "INSERT INTO msgs(rfc724_mid, chat_id) VALUES (?,?)", - (rfc724_mid, DC_CHAT_ID_TRASH), - ) - .await?; - msg_ids = vec![MsgId::new(u32::try_from(row_id)?)]; - } else { - return Ok(None); + if rfc724_mid.starts_with(GENERATED_PREFIX) { // We don't have an rfc724_mid, there's no point in adding a trash entry + return Ok(None); } + let row_id = context + .sql + .execute( + "INSERT INTO msgs(rfc724_mid, chat_id) VALUES (?,?)", + (rfc724_mid, DC_CHAT_ID_TRASH), + ) + .await?; + let msg_ids = vec![MsgId::new(u32::try_from(row_id)?)]; + return Ok(Some(ReceivedMsg { chat_id: DC_CHAT_ID_TRASH, state: MessageState::Undefined, From 968cc653236a47e529f8f76521390f28707f21cc Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Mon, 25 Sep 2023 15:25:39 +0200 Subject: [PATCH 04/17] test that update.document is not forwarded the test is failing currently --- src/webxdc.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/webxdc.rs b/src/webxdc.rs index 4697f025b..3341446e4 100644 --- a/src/webxdc.rs +++ b/src/webxdc.rs @@ -1012,7 +1012,7 @@ mod tests { let instance = send_webxdc_instance(&t, chat_id).await?; t.send_webxdc_status_update( instance.id, - r#"{"info": "foo", "summary":"bar", "payload": 42}"#, + r#"{"info": "foo", "summary":"bar", "document":"doc", "payload": 42}"#, "descr", ) .await?; @@ -1020,7 +1020,7 @@ mod tests { assert_eq!( t.get_webxdc_status_updates(instance.id, StatusUpdateSerial(0)) .await?, - r#"[{"payload":42,"info":"foo","summary":"bar","serial":1,"max_serial":1}]"# + r#"[{"payload":42,"info":"foo","document":"doc","summary":"bar","serial":1,"max_serial":1}]"# ); assert_eq!(chat_id.get_msg_cnt(&t).await?, 2); // instance and info let info = Message::load_from_db(&t, instance.id) @@ -1028,6 +1028,7 @@ mod tests { .get_webxdc_info(&t) .await?; assert_eq!(info.summary, "bar".to_string()); + assert_eq!(info.document, "doc".to_string()); // forwarding an instance creates a fresh instance; updates etc. are not forwarded forward_msgs(&t, &[instance.get_id()], chat_id).await?; @@ -1044,6 +1045,7 @@ mod tests { .get_webxdc_info(&t) .await?; assert_eq!(info.summary, "".to_string()); + assert_eq!(info.document, "".to_string()); Ok(()) } From 56b2361f01e9de9f3bfa262865a8fdd134e22f7a Mon Sep 17 00:00:00 2001 From: "B. Petersen" Date: Mon, 25 Sep 2023 15:27:02 +0200 Subject: [PATCH 05/17] reset document.update on forwarding this fixes the test test_forward_webxdc_instance() --- src/chat.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/chat.rs b/src/chat.rs index 5f4fcfe44..378456819 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -3528,6 +3528,8 @@ pub async fn forward_msgs(context: &Context, msg_ids: &[MsgId], chat_id: ChatId) msg.param.remove(Param::ForcePlaintext); msg.param.remove(Param::Cmd); msg.param.remove(Param::OverrideSenderDisplayname); + msg.param.remove(Param::WebxdcDocument); + msg.param.remove(Param::WebxdcDocumentTimestamp); msg.param.remove(Param::WebxdcSummary); msg.param.remove(Param::WebxdcSummaryTimestamp); msg.in_reply_to = None; From 4e5b41f150c32e1f8ff1e0346bde44defd98dfc2 Mon Sep 17 00:00:00 2001 From: link2xt Date: Mon, 25 Sep 2023 14:03:09 +0000 Subject: [PATCH 06/17] fix: require valid email addresses in dc_provider_new_from_email[_with_dns]() --- deltachat-ffi/src/lib.rs | 16 +++++++++++++--- src/provider.rs | 36 ++++++++++++++++++++++++++++-------- 2 files changed, 41 insertions(+), 11 deletions(-) diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index ba33b9637..8025620ef 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -4531,7 +4531,14 @@ pub unsafe extern "C" fn dc_provider_new_from_email( let ctx = &*context; - match block_on(provider::get_provider_info(ctx, addr.as_str(), true)) { + match block_on(provider::get_provider_info_by_addr( + ctx, + addr.as_str(), + true, + )) + .log_err(ctx) + .unwrap_or_default() + { Some(provider) => provider, None => ptr::null_mut(), } @@ -4558,11 +4565,14 @@ pub unsafe extern "C" fn dc_provider_new_from_email_with_dns( match socks5_enabled { Ok(socks5_enabled) => { - match block_on(provider::get_provider_info( + match block_on(provider::get_provider_info_by_addr( ctx, addr.as_str(), socks5_enabled, - )) { + )) + .log_err(ctx) + .unwrap_or_default() + { Some(provider) => provider, None => ptr::null_mut(), } diff --git a/src/provider.rs b/src/provider.rs index 9a8a438ca..fe7073ebe 100644 --- a/src/provider.rs +++ b/src/provider.rs @@ -8,6 +8,7 @@ use trust_dns_resolver::{config, AsyncResolver, TokioAsyncResolver}; use crate::config::Config; use crate::context::Context; use crate::provider::data::{PROVIDER_DATA, PROVIDER_IDS}; +use crate::tools::EmailAddress; /// Provider status according to manual testing. #[derive(Debug, Display, Copy, Clone, PartialEq, Eq, FromPrimitive, ToPrimitive)] @@ -175,21 +176,30 @@ fn get_resolver() -> Result { Ok(resolver) } +/// Returns provider for the given an e-mail address. +/// +/// Returns an error if provided address is not valid. +pub async fn get_provider_info_by_addr( + context: &Context, + addr: &str, + skip_mx: bool, +) -> Result> { + let addr = EmailAddress::new(addr)?; + + let provider = get_provider_info(context, &addr.domain, skip_mx).await; + Ok(provider) +} + /// Returns provider for the given domain. /// /// This function looks up domain in offline database first. If not /// found, it queries MX record for the domain and looks up offline /// database for MX domains. -/// -/// For compatibility, email address can be passed to this function -/// instead of the domain. pub async fn get_provider_info( context: &Context, domain: &str, skip_mx: bool, ) -> Option<&'static Provider> { - let domain = domain.rsplit('@').next()?; - if let Some(provider) = get_provider_by_domain(domain) { return Some(provider); } @@ -314,15 +324,25 @@ mod tests { let t = TestContext::new().await; assert!(get_provider_info(&t, "", false).await.is_none()); assert!(get_provider_info(&t, "google.com", false).await.unwrap().id == "gmail"); + assert!(get_provider_info(&t, "example@google.com", false) + .await + .is_none()); + } - // get_provider_info() accepts email addresses for backwards compatibility + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_get_provider_info_by_addr() -> Result<()> { + let t = TestContext::new().await; + assert!(get_provider_info_by_addr(&t, "google.com", false) + .await + .is_err()); assert!( - get_provider_info(&t, "example@google.com", false) - .await + get_provider_info_by_addr(&t, "example@google.com", false) + .await? .unwrap() .id == "gmail" ); + Ok(()) } #[test] From b1d517398d46220eec934f62ae6799866d9cc34f Mon Sep 17 00:00:00 2001 From: WofWca Date: Sat, 23 Sep 2023 20:46:17 +0400 Subject: [PATCH 07/17] refactor: improve comment about `Ratelimit` A few people got the impression that if you send 6 messages in a burst you'll only be able to send the next one in 60 seconds. Hopefully this can resolve it. --- src/context.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/context.rs b/src/context.rs index 78c0ed47f..d1300f7fa 100644 --- a/src/context.rs +++ b/src/context.rs @@ -382,7 +382,7 @@ impl Context { translated_stockstrings: stockstrings, events, scheduler: SchedulerState::new(), - ratelimit: RwLock::new(Ratelimit::new(Duration::new(60, 0), 6.0)), // Allow to send 6 messages immediately, no more than once every 10 seconds. + ratelimit: RwLock::new(Ratelimit::new(Duration::new(60, 0), 6.0)), // Allow at least 1 message every 10 seconds + a burst of 6. quota: RwLock::new(None), quota_update_request: AtomicBool::new(false), resync_request: AtomicBool::new(false), From 88bba83383a6704eb9f597d744008c60413eccd4 Mon Sep 17 00:00:00 2001 From: link2xt Date: Tue, 26 Sep 2023 16:02:14 +0000 Subject: [PATCH 08/17] refactor: flatten process_report() --- src/mimeparser.rs | 53 +++++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/src/mimeparser.rs b/src/mimeparser.rs index fa5bfef92..0c73d2b6f 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -1442,33 +1442,36 @@ impl MimeMessage { let (report_fields, _) = mailparse::parse_headers(&report_body)?; // must be present - if let Some(_disposition) = report_fields.get_header_value(HeaderDef::Disposition) { - let original_message_id = report_fields - .get_header_value(HeaderDef::OriginalMessageId) - // MS Exchange doesn't add an Original-Message-Id header. Instead, they put - // the original message id into the In-Reply-To header: - .or_else(|| report.headers.get_header_value(HeaderDef::InReplyTo)) - .and_then(|v| parse_message_id(&v).ok()); - let additional_message_ids = report_fields - .get_header_value(HeaderDef::AdditionalMessageIds) - .map_or_else(Vec::new, |v| { - v.split(' ') - .filter_map(|s| parse_message_id(s).ok()) - .collect() - }); + if report_fields + .get_header_value(HeaderDef::Disposition) + .is_none() + { + warn!( + context, + "Ignoring unknown disposition-notification, Message-Id: {:?}.", + report_fields.get_header_value(HeaderDef::MessageId) + ); + return Ok(None); + }; - return Ok(Some(Report { - original_message_id, - additional_message_ids, - })); - } - warn!( - context, - "ignoring unknown disposition-notification, Message-Id: {:?}", - report_fields.get_header_value(HeaderDef::MessageId) - ); + let original_message_id = report_fields + .get_header_value(HeaderDef::OriginalMessageId) + // MS Exchange doesn't add an Original-Message-Id header. Instead, they put + // the original message id into the In-Reply-To header: + .or_else(|| report.headers.get_header_value(HeaderDef::InReplyTo)) + .and_then(|v| parse_message_id(&v).ok()); + let additional_message_ids = report_fields + .get_header_value(HeaderDef::AdditionalMessageIds) + .map_or_else(Vec::new, |v| { + v.split(' ') + .filter_map(|s| parse_message_id(s).ok()) + .collect() + }); - Ok(None) + Ok(Some(Report { + original_message_id, + additional_message_ids, + })) } fn process_delivery_status( From 815c1b9c491a24eac56a547c2fbe9ee45dc8c0ea Mon Sep 17 00:00:00 2001 From: link2xt Date: Wed, 27 Sep 2023 17:00:39 +0000 Subject: [PATCH 09/17] refactor: resultify location::set() --- deltachat-ffi/src/lib.rs | 7 ++++- deltachat-repl/src/cmdline.rs | 2 +- src/location.rs | 54 ++++++++++++++++------------------- 3 files changed, 32 insertions(+), 31 deletions(-) diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 8025620ef..d9aba5b96 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -2567,7 +2567,12 @@ pub unsafe extern "C" fn dc_set_location( } let ctx = &*context; - block_on(location::set(ctx, latitude, longitude, accuracy)) as _ + block_on(async move { + location::set(ctx, latitude, longitude, accuracy) + .await + .log_err(ctx) + .unwrap_or_default() + }) as libc::c_int } #[no_mangle] diff --git a/deltachat-repl/src/cmdline.rs b/deltachat-repl/src/cmdline.rs index 240998a95..e0168a28f 100644 --- a/deltachat-repl/src/cmdline.rs +++ b/deltachat-repl/src/cmdline.rs @@ -894,7 +894,7 @@ pub async fn cmdline(context: Context, line: &str, chat_id: &mut ChatId) -> Resu let latitude = arg1.parse()?; let longitude = arg2.parse()?; - let continue_streaming = location::set(&context, latitude, longitude, 0.).await; + let continue_streaming = location::set(&context, latitude, longitude, 0.).await?; if continue_streaming { println!("Success, streaming should be continued."); } else { diff --git a/src/location.rs b/src/location.rs index 099ea3da9..ab3549516 100644 --- a/src/location.rs +++ b/src/location.rs @@ -328,13 +328,13 @@ pub async fn is_sending_locations_to_chat( } /// Sets current location of the user device. -pub async fn set(context: &Context, latitude: f64, longitude: f64, accuracy: f64) -> bool { +pub async fn set(context: &Context, latitude: f64, longitude: f64, accuracy: f64) -> Result { if latitude == 0.0 && longitude == 0.0 { - return true; + return Ok(true); } let mut continue_streaming = false; - if let Ok(chats) = context + let chats = context .sql .query_map( "SELECT id FROM chats WHERE locations_send_until>?;", @@ -346,33 +346,29 @@ pub async fn set(context: &Context, latitude: f64, longitude: f64, accuracy: f64 .map_err(Into::into) }, ) - .await - { - for chat_id in chats { - if let Err(err) = context.sql.execute( - "INSERT INTO locations \ - (latitude, longitude, accuracy, timestamp, chat_id, from_id) VALUES (?,?,?,?,?,?);", - ( - latitude, - longitude, - accuracy, - time(), - chat_id, - ContactId::SELF, - ) - ).await { - warn!(context, "failed to store location {:#}", err); - } else { - info!(context, "stored location for chat {}", chat_id); - continue_streaming = true; - } - } - if continue_streaming { - context.emit_event(EventType::LocationChanged(Some(ContactId::SELF))); - }; - } + .await?; - continue_streaming + for chat_id in chats { + context.sql.execute( + "INSERT INTO locations \ + (latitude, longitude, accuracy, timestamp, chat_id, from_id) VALUES (?,?,?,?,?,?);", + ( + latitude, + longitude, + accuracy, + time(), + chat_id, + ContactId::SELF, + )).await.context("Failed to store location")?; + + info!(context, "Stored location for chat {chat_id}."); + continue_streaming = true; + } + if continue_streaming { + context.emit_event(EventType::LocationChanged(Some(ContactId::SELF))); + }; + + Ok(continue_streaming) } /// Searches for locations in the given time range, optionally filtering by chat and contact IDs. From a7cf51868b252318b4b6793fc9939bb678e94c8c Mon Sep 17 00:00:00 2001 From: link2xt Date: Wed, 27 Sep 2023 15:36:10 +0000 Subject: [PATCH 10/17] test: test send_location --- src/location.rs | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/location.rs b/src/location.rs index ab3549516..dc07181ec 100644 --- a/src/location.rs +++ b/src/location.rs @@ -924,4 +924,38 @@ Content-Disposition: attachment; filename="location.kml" assert_eq!(locations.len(), 1); Ok(()) } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_send_locations_to_chat() -> Result<()> { + let alice = TestContext::new_alice().await; + let bob = TestContext::new_bob().await; + + let alice_chat = alice.create_chat(&bob).await; + send_locations_to_chat(&alice, alice_chat.id, 1000).await?; + let sent = alice.pop_sent_msg().await; + let msg = bob.recv_msg(&sent).await; + assert_eq!(msg.text, "Location streaming enabled by alice@example.org."); + let bob_chat_id = msg.chat_id; + + assert_eq!(set(&alice, 10.0, 20.0, 1.0).await?, true); + + // Send image without text. + let file_name = "image.png"; + let bytes = include_bytes!("../test-data/image/logo.png"); + let file = alice.get_blobdir().join(file_name); + tokio::fs::write(&file, bytes).await?; + let mut msg = Message::new(Viewtype::Image); + msg.set_file(file.to_str().unwrap(), None); + let sent = alice.send_msg(alice_chat.id, &mut msg).await; + + let msg = bob.recv_msg_opt(&sent).await.unwrap(); + assert!(msg.chat_id == bob_chat_id); + assert_eq!(msg.msg_ids.len(), 1); + + let bob_msg = Message::load_from_db(&bob, *msg.msg_ids.get(0).unwrap()).await?; + assert_eq!(bob_msg.chat_id, bob_chat_id); + assert_eq!(bob_msg.viewtype, Viewtype::Image); + + Ok(()) + } } From 6990312051efddbb59a221c6adca0ec538eaed26 Mon Sep 17 00:00:00 2001 From: link2xt Date: Wed, 27 Sep 2023 09:27:09 +0000 Subject: [PATCH 11/17] fix: trash only empty *text* parts when location.kml is attached If the message contains other attachment parts such as images, they should not go into trash. --- src/receive_imf.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 2011f39b2..3ebddf129 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -1128,7 +1128,8 @@ async fn add_parts( (&part.msg, part.typ) }; - let part_is_empty = part.msg.is_empty() && part.param.get(Param::Quote).is_none(); + let part_is_empty = + typ == Viewtype::Text && msg.is_empty() && part.param.get(Param::Quote).is_none(); let mime_modified = save_mime_modified && !part_is_empty; if mime_modified { // Avoid setting mime_modified for more than one part. @@ -1153,7 +1154,8 @@ async fn add_parts( // If you change which information is skipped if the message is trashed, // also change `MsgId::trash()` and `delete_expired_messages()` - let trash = chat_id.is_trash() || (is_location_kml && msg.is_empty()); + let trash = + chat_id.is_trash() || (is_location_kml && msg.is_empty() && typ == Viewtype::Text); let row_id = context .sql From 38d5743c06993452e542332aaf16e9a98d502651 Mon Sep 17 00:00:00 2001 From: link2xt Date: Thu, 28 Sep 2023 15:19:33 +0000 Subject: [PATCH 12/17] refactor: do not ignore errors in get_kml() This removes unnecessary warning "mimefactory: could not send location: No locations processed" when there are no locations to send. --- src/location.rs | 10 ++++++---- src/mimefactory.rs | 19 ++++++++++--------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/location.rs b/src/location.rs index dc07181ec..9084e752f 100644 --- a/src/location.rs +++ b/src/location.rs @@ -460,7 +460,7 @@ pub async fn delete_all(context: &Context) -> Result<()> { } /// Returns `location.kml` contents. -pub async fn get_kml(context: &Context, chat_id: ChatId) -> Result<(String, u32)> { +pub async fn get_kml(context: &Context, chat_id: ChatId) -> Result> { let mut last_added_location_id = 0; let self_addr = context.get_primary_self_addr().await?; @@ -530,9 +530,11 @@ pub async fn get_kml(context: &Context, chat_id: ChatId) -> Result<(String, u32) ret += "\n"; } - ensure!(location_count > 0, "No locations processed"); - - Ok((ret, last_added_location_id)) + if location_count > 0 { + Ok(Some((ret, last_added_location_id))) + } else { + Ok(None) + } } fn get_kml_timestamp(utc: i64) -> String { diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 996ea3fd7..0f55423cc 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -860,9 +860,13 @@ impl<'a> MimeFactory<'a> { } /// Returns MIME part with a `location.kml` attachment. - async fn get_location_kml_part(&mut self, context: &Context) -> Result { - let (kml_content, last_added_location_id) = - location::get_kml(context, self.msg.chat_id).await?; + async fn get_location_kml_part(&mut self, context: &Context) -> Result> { + let Some((kml_content, last_added_location_id)) = + location::get_kml(context, self.msg.chat_id).await? + else { + return Ok(None); + }; + let part = PartBuilder::new() .content_type( &"application/vnd.google-earth.kml+xml" @@ -878,7 +882,7 @@ impl<'a> MimeFactory<'a> { // otherwise, the independent location is already filed self.last_added_location_id = last_added_location_id; } - Ok(part) + Ok(Some(part)) } #[allow(clippy::cognitive_complexity)] @@ -1230,11 +1234,8 @@ impl<'a> MimeFactory<'a> { } if location::is_sending_locations_to_chat(context, Some(self.msg.chat_id)).await? { - match self.get_location_kml_part(context).await { - Ok(part) => parts.push(part), - Err(err) => { - warn!(context, "mimefactory: could not send location: {}", err); - } + if let Some(part) = self.get_location_kml_part(context).await? { + parts.push(part); } } From b463a0566e47182f640137238d5f2f2662ba1c54 Mon Sep 17 00:00:00 2001 From: link2xt Date: Thu, 28 Sep 2023 15:20:51 +0000 Subject: [PATCH 13/17] refactor: flatten create_or_lookup_mailinglist() --- src/receive_imf.rs | 57 +++++++++++++++++++++++----------------------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 3ebddf129..220174157 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -2007,39 +2007,40 @@ async fn apply_mailinglist_changes( mime_parser: &MimeMessage, chat_id: ChatId, ) -> Result<()> { - if let Some(list_post) = &mime_parser.list_post { - let mut chat = Chat::load_from_db(context, chat_id).await?; - if chat.typ != Chattype::Mailinglist { + let Some(list_post) = &mime_parser.list_post else { + return Ok(()); + }; + + let mut chat = Chat::load_from_db(context, chat_id).await?; + if chat.typ != Chattype::Mailinglist { + return Ok(()); + } + let listid = &chat.grpid; + + let list_post = match ContactAddress::new(list_post) { + Ok(list_post) => list_post, + Err(err) => { + warn!(context, "Invalid List-Post: {:#}.", err); return Ok(()); } - let listid = &chat.grpid; + }; + let (contact_id, _) = Contact::add_or_lookup(context, "", list_post, Origin::Hidden).await?; + let mut contact = Contact::get_by_id(context, contact_id).await?; + if contact.param.get(Param::ListId) != Some(listid) { + contact.param.set(Param::ListId, listid); + contact.update_param(context).await?; + } - let list_post = match ContactAddress::new(list_post) { - Ok(list_post) => list_post, - Err(err) => { - warn!(context, "Invalid List-Post: {:#}.", err); - return Ok(()); - } - }; - let (contact_id, _) = - Contact::add_or_lookup(context, "", list_post, Origin::Hidden).await?; - let mut contact = Contact::get_by_id(context, contact_id).await?; - if contact.param.get(Param::ListId) != Some(listid) { - contact.param.set(Param::ListId, listid); - contact.update_param(context).await?; - } - - if let Some(old_list_post) = chat.param.get(Param::ListPost) { - if list_post.as_ref() != old_list_post { - // Apparently the mailing list is using a different List-Post header in each message. - // Make the mailing list read-only because we wouldn't know which message the user wants to reply to. - chat.param.remove(Param::ListPost); - chat.update_param(context).await?; - } - } else { - chat.param.set(Param::ListPost, list_post); + if let Some(old_list_post) = chat.param.get(Param::ListPost) { + if list_post.as_ref() != old_list_post { + // Apparently the mailing list is using a different List-Post header in each message. + // Make the mailing list read-only because we wouldn't know which message the user wants to reply to. + chat.param.remove(Param::ListPost); chat.update_param(context).await?; } + } else { + chat.param.set(Param::ListPost, list_post); + chat.update_param(context).await?; } Ok(()) From f23023961ea311e398e73b9b49efa04d8c45b44d Mon Sep 17 00:00:00 2001 From: link2xt Date: Thu, 28 Sep 2023 16:09:15 +0000 Subject: [PATCH 14/17] api!: return DC_CONTACT_ID_SELF from dc_contact_get_verifier_id() for directly verified contacts --- deltachat-ffi/deltachat.h | 3 ++- src/contact.rs | 21 ++++++++++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index 9bca4b3b2..e35f7710b 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -5047,6 +5047,7 @@ int dc_contact_is_verified (dc_contact_t* contact); * A string containing the verifiers address. If it is the same address as the contact itself, * we verified the contact ourself. If it is an empty string, we don't have verifier * information or the contact is not verified. + * @deprecated 2023-09-28, use dc_contact_get_verifier_id instead */ char* dc_contact_get_verifier_addr (dc_contact_t* contact); @@ -5059,7 +5060,7 @@ char* dc_contact_get_verifier_addr (dc_contact_t* contact); * @memberof dc_contact_t * @param contact The contact object. * @return - * The `ContactId` of the verifiers address. If it is the same address as the contact itself, + * The contact ID of the verifier. If it is DC_CONTACT_ID_SELF, * we verified the contact ourself. If it is 0, we don't have verifier information or * the contact is not verified. */ diff --git a/src/contact.rs b/src/contact.rs index ab8522bd0..2af80c147 100644 --- a/src/contact.rs +++ b/src/contact.rs @@ -1252,11 +1252,22 @@ impl Contact { /// Returns the ContactId that verified the contact. pub async fn get_verifier_id(&self, context: &Context) -> Result> { - let verifier_addr = self.get_verifier_addr(context).await?; - if let Some(addr) = verifier_addr { - Ok(Contact::lookup_id_by_addr(context, &addr, Origin::AddressBook).await?) - } else { - Ok(None) + let Some(verifier_addr) = self.get_verifier_addr(context).await? else { + return Ok(None); + }; + + if verifier_addr == self.addr { + // Contact is directly verified via QR code. + return Ok(Some(ContactId::SELF)); + } + + match Contact::lookup_id_by_addr(context, &verifier_addr, Origin::AddressBook).await? { + Some(contact_id) => Ok(Some(contact_id)), + None => { + let addr = &self.addr; + warn!(context, "Could not lookup contact with address {verifier_addr} which introduced {addr}."); + Ok(None) + } } } From a19811f37983183c8c7451bd7c136fc1bac38ecc Mon Sep 17 00:00:00 2001 From: link2xt Date: Fri, 29 Sep 2023 13:07:49 +0000 Subject: [PATCH 15/17] chore(cargo): update `tungstenite` to fix RUSTSEC-2023-0065 Used `cargo update -p axum`. --- Cargo.lock | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a6b055826..608db7795 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -312,9 +312,9 @@ checksum = "d468802bab17cbc0cc575e9b053f41e72aa36bfa6b7f55e3529ffa43161b97fa" [[package]] name = "axum" -version = "0.6.18" +version = "0.6.20" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f8175979259124331c1d7bf6586ee7e0da434155e4b2d48ec2c8386281d8df39" +checksum = "3b829e4e32b91e643de6eafe82b1d90675f5874230191a4ffbc1b336dec4d6bf" dependencies = [ "async-trait", "axum-core", @@ -4947,9 +4947,9 @@ dependencies = [ [[package]] name = "tokio-tungstenite" -version = "0.18.0" +version = "0.20.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "54319c93411147bced34cb5609a80e0a8e44c5999c93903a81cd866630ec0bfd" +checksum = "212d5dcb2a1ce06d81107c3d0ffa3121fe974b73f068c8282cb1c32328113b6c" dependencies = [ "futures-util", "log", @@ -5159,13 +5159,13 @@ checksum = "3528ecfd12c466c6f163363caf2d02a71161dd5e1cc6ae7b34207ea2d42d81ed" [[package]] name = "tungstenite" -version = "0.18.0" +version = "0.20.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "30ee6ab729cd4cf0fd55218530c4522ed30b7b6081752839b68fcec8d0960788" +checksum = "9e3dac10fd62eaf6617d3a904ae222845979aec67c615d1c842b4002c7666fb9" dependencies = [ - "base64 0.13.1", "byteorder", "bytes", + "data-encoding", "http", "httparse", "log", From 33a203d56e6e7f4b3798a5e1276a6a0bdf96e5bd Mon Sep 17 00:00:00 2001 From: link2xt Date: Wed, 27 Sep 2023 19:30:19 +0000 Subject: [PATCH 16/17] fix: initialise last_msg_id to the highest known row id Otherwise existing bots migrating to get_next_msgs() are trying to process all the messages they have in the database. --- src/context.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/context.rs b/src/context.rs index d1300f7fa..d7aac82d1 100644 --- a/src/context.rs +++ b/src/context.rs @@ -814,7 +814,22 @@ impl Context { pub async fn get_next_msgs(&self) -> Result> { let last_msg_id = match self.get_config(Config::LastMsgId).await? { Some(s) => MsgId::new(s.parse()?), - None => MsgId::new_unset(), + None => { + // If `last_msg_id` is not set yet, + // subtract 1 from the last id, + // so a single message is returned and can + // be marked as seen. + self.sql + .query_row( + "SELECT IFNULL((SELECT MAX(id) - 1 FROM msgs), 0)", + (), + |row| { + let msg_id: MsgId = row.get(0)?; + Ok(msg_id) + }, + ) + .await? + } }; let list = self From 6d2ac304614272e8356637ec7e9be2d2f85d9e32 Mon Sep 17 00:00:00 2001 From: link2xt Date: Fri, 29 Sep 2023 14:52:33 +0000 Subject: [PATCH 17/17] fix: do not put the status footer into reaction MIME parts --- src/mimefactory.rs | 7 +++++-- src/reaction.rs | 10 ++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 0f55423cc..d9d6ea1ee 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -1172,7 +1172,10 @@ impl<'a> MimeFactory<'a> { } let flowed_text = format_flowed(final_text); - let footer = &self.selfstatus; + let is_reaction = self.msg.param.get_int(Param::Reaction).unwrap_or_default() != 0; + + let footer = if is_reaction { "" } else { &self.selfstatus }; + let message_text = format!( "{}{}{}{}{}{}", fwdhint.unwrap_or_default(), @@ -1195,7 +1198,7 @@ impl<'a> MimeFactory<'a> { )) .body(message_text); - if self.msg.param.get_int(Param::Reaction).unwrap_or_default() != 0 { + if is_reaction { main_part = main_part.header(("Content-Disposition", "reaction")); } diff --git a/src/reaction.rs b/src/reaction.rs index c349ed312..d8cb4450c 100644 --- a/src/reaction.rs +++ b/src/reaction.rs @@ -464,6 +464,16 @@ Content-Disposition: reaction\n\ let alice = TestContext::new_alice().await; let bob = TestContext::new_bob().await; + // Test that the status does not get mixed up into reactions. + alice + .set_config( + Config::Selfstatus, + Some("Buy Delta Chat today and make this banner go away!"), + ) + .await?; + bob.set_config(Config::Selfstatus, Some("Sent from my Delta Chat Pro. 👍")) + .await?; + let chat_alice = alice.create_chat(&bob).await; let alice_msg = alice.send_text(chat_alice.id, "Hi!").await; let bob_msg = bob.recv_msg(&alice_msg).await;