From 97ee78e10e6484ab1dbe8f1f13d1e6a9b520add9 Mon Sep 17 00:00:00 2001 From: link2xt Date: Sat, 7 Dec 2024 18:27:26 +0000 Subject: [PATCH 1/4] api!: remove `dc_prepare_msg` and `dc_msg_is_increation` --- deltachat-ffi/deltachat.h | 71 +---------- deltachat-ffi/src/lib.rs | 31 ----- node/lib/message.ts | 4 - node/src/module.c | 12 -- node/test/test.mjs | 1 - python/src/deltachat/chat.py | 34 +---- python/tests/test_1_online.py | 8 +- python/tests/test_2_increation.py | 107 ---------------- python/tests/test_3_offline.py | 42 +----- src/blob.rs | 35 ----- src/chat.rs | 204 +++++++++++------------------- src/ephemeral.rs | 3 - src/message.rs | 48 +------ src/mimefactory.rs | 20 +-- src/mimeparser.rs | 6 +- src/param.rs | 22 +--- src/sql.rs | 1 - 17 files changed, 101 insertions(+), 548 deletions(-) delete mode 100644 python/tests/test_2_increation.py diff --git a/deltachat-ffi/deltachat.h b/deltachat-ffi/deltachat.h index c062fef84e..82e937f82c 100644 --- a/deltachat-ffi/deltachat.h +++ b/deltachat-ffi/deltachat.h @@ -963,54 +963,6 @@ uint32_t dc_create_chat_by_contact_id (dc_context_t* context, uint32_t co uint32_t dc_get_chat_id_by_contact_id (dc_context_t* context, uint32_t contact_id); -/** - * Prepare a message for sending. - * - * Call this function if the file to be sent is still in creation. - * Once you're done with creating the file, call dc_send_msg() as usual - * and the message will really be sent. - * - * This is useful as the user can already send the next messages while - * e.g. the recoding of a video is not yet finished. Or the user can even forward - * the message with the file being still in creation to other groups. - * - * Files being sent with the increation-method must be placed in the - * blob directory, see dc_get_blobdir(). - * If the increation-method is not used - which is probably the normal case - - * dc_send_msg() copies the file to the blob directory if it is not yet there. - * To distinguish the two cases, msg->state must be set properly. The easiest - * way to ensure this is to reuse the same object for both calls. - * - * Example: - * ~~~ - * char* blobdir = dc_get_blobdir(context); - * char* file_to_send = mprintf("%s/%s", blobdir, "send.mp4") - * - * dc_msg_t* msg = dc_msg_new(context, DC_MSG_VIDEO); - * dc_msg_set_file(msg, file_to_send, NULL); - * dc_prepare_msg(context, chat_id, msg); - * - * // ... create the file ... - * - * dc_send_msg(context, chat_id, msg); - * - * dc_msg_unref(msg); - * free(file_to_send); - * dc_str_unref(file_to_send); - * ~~~ - * - * @memberof dc_context_t - * @param context The context object as returned from dc_context_new(). - * @param chat_id The chat ID to send the message to. - * @param msg The message object to send to the chat defined by the chat ID. - * On success, msg_id and state of the object are set up, - * The function does not take ownership of the object, - * so you have to free it using dc_msg_unref() as usual. - * @return The ID of the message that is being prepared. - */ -uint32_t dc_prepare_msg (dc_context_t* context, uint32_t chat_id, dc_msg_t* msg); - - /** * Send a message defined by a dc_msg_t object to a chat. * @@ -1035,13 +987,11 @@ uint32_t dc_prepare_msg (dc_context_t* context, uint32_t ch * If that fails, is not possible, or the image is already small enough, the image is sent as original. * If you want images to be always sent as the original file, use the #DC_MSG_FILE type. * - * Videos and other file types are currently not recoded by the library, - * with dc_prepare_msg(), however, you can do that from the UI. + * Videos and other file types are currently not recoded by the library. * * @memberof dc_context_t * @param context The context object as returned from dc_context_new(). * @param chat_id The chat ID to send the message to. - * If dc_prepare_msg() was called before, this parameter can be 0. * @param msg The message object to send to the chat defined by the chat ID. * On success, msg_id of the object is set up, * The function does not take ownership of the object, @@ -1058,7 +1008,6 @@ uint32_t dc_send_msg (dc_context_t* context, uint32_t ch * @memberof dc_context_t * @param context The context object as returned from dc_context_new(). * @param chat_id The chat ID to send the message to. - * If dc_prepare_msg() was called before, this parameter can be 0. * @param msg The message object to send to the chat defined by the chat ID. * On success, msg_id of the object is set up, * The function does not take ownership of the object, @@ -3985,7 +3934,7 @@ int dc_msg_get_viewtype (const dc_msg_t* msg); * * Outgoing message states: * - @ref DC_STATE_OUT_PREPARING - For files which need time to be prepared before they can be sent, - * the message enters this state before @ref DC_STATE_OUT_PENDING. + * the message enters this state before @ref DC_STATE_OUT_PENDING. Deprecated. * - @ref DC_STATE_OUT_DRAFT - Message saved as draft using dc_set_draft() * - @ref DC_STATE_OUT_PENDING - The user has pressed the "send" button but the * message is not yet sent and is pending in some way. Maybe we're offline (no checkmark). @@ -4535,20 +4484,6 @@ int dc_msg_get_info_type (const dc_msg_t* msg); */ char* dc_msg_get_webxdc_href (const dc_msg_t* msg); -/** - * Check if a message is still in creation. A message is in creation between - * the calls to dc_prepare_msg() and dc_send_msg(). - * - * Typically, this is used for videos that are recoded by the UI before - * they can be sent. - * - * @memberof dc_msg_t - * @param msg The message object. - * @return 1=message is still in creation (dc_send_msg() was not called yet), - * 0=message no longer in creation. - */ -int dc_msg_is_increation (const dc_msg_t* msg); - /** * Check if the message is an Autocrypt Setup Message. @@ -5562,6 +5497,8 @@ int64_t dc_lot_get_timestamp (const dc_lot_t* lot); /** * Outgoing message being prepared. See dc_msg_get_state() for details. + * + * @deprecated 2024-12-07 */ #define DC_STATE_OUT_PREPARING 18 diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 694654e365..c9c0cf8433 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -976,27 +976,6 @@ pub unsafe extern "C" fn dc_get_chat_id_by_contact_id( }) } -#[no_mangle] -pub unsafe extern "C" fn dc_prepare_msg( - context: *mut dc_context_t, - chat_id: u32, - msg: *mut dc_msg_t, -) -> u32 { - if context.is_null() || chat_id == 0 || msg.is_null() { - eprintln!("ignoring careless call to dc_prepare_msg()"); - return 0; - } - let ctx = &mut *context; - let ffi_msg: &mut MessageWrapper = &mut *msg; - - block_on(async move { - chat::prepare_msg(ctx, ChatId::new(chat_id), &mut ffi_msg.message) - .await - .unwrap_or_log_default(ctx, "Failed to prepare message") - }) - .to_u32() -} - #[no_mangle] pub unsafe extern "C" fn dc_send_msg( context: *mut dc_context_t, @@ -3713,16 +3692,6 @@ pub unsafe extern "C" fn dc_msg_get_webxdc_href(msg: *mut dc_msg_t) -> *mut libc ffi_msg.message.get_webxdc_href().strdup() } -#[no_mangle] -pub unsafe extern "C" fn dc_msg_is_increation(msg: *mut dc_msg_t) -> libc::c_int { - if msg.is_null() { - eprintln!("ignoring careless call to dc_msg_is_increation()"); - return 0; - } - let ffi_msg = &*msg; - ffi_msg.message.is_increation().into() -} - #[no_mangle] pub unsafe extern "C" fn dc_msg_is_setupmessage(msg: *mut dc_msg_t) -> libc::c_int { if msg.is_null() { diff --git a/node/lib/message.ts b/node/lib/message.ts index 5b2ad67d9d..c4036ad6b4 100644 --- a/node/lib/message.ts +++ b/node/lib/message.ts @@ -299,10 +299,6 @@ export class Message { return Boolean(binding.dcn_msg_is_forwarded(this.dc_msg)) } - isIncreation() { - return Boolean(binding.dcn_msg_is_increation(this.dc_msg)) - } - isInfo() { return Boolean(binding.dcn_msg_is_info(this.dc_msg)) } diff --git a/node/src/module.c b/node/src/module.c index 33492aeecb..02dda50c1b 100644 --- a/node/src/module.c +++ b/node/src/module.c @@ -2374,17 +2374,6 @@ NAPI_METHOD(dcn_msg_is_forwarded) { NAPI_RETURN_INT32(is_forwarded); } -NAPI_METHOD(dcn_msg_is_increation) { - NAPI_ARGV(1); - NAPI_DC_MSG(); - - //TRACE("calling.."); - int is_increation = dc_msg_is_increation(dc_msg); - //TRACE("result %d", is_increation); - - NAPI_RETURN_INT32(is_increation); -} - NAPI_METHOD(dcn_msg_is_info) { NAPI_ARGV(1); NAPI_DC_MSG(); @@ -3555,7 +3544,6 @@ NAPI_INIT() { NAPI_EXPORT_FUNCTION(dcn_msg_has_location); NAPI_EXPORT_FUNCTION(dcn_msg_has_html); NAPI_EXPORT_FUNCTION(dcn_msg_is_forwarded); - NAPI_EXPORT_FUNCTION(dcn_msg_is_increation); NAPI_EXPORT_FUNCTION(dcn_msg_is_info); NAPI_EXPORT_FUNCTION(dcn_msg_is_sent); NAPI_EXPORT_FUNCTION(dcn_msg_is_setupmessage); diff --git a/node/test/test.mjs b/node/test/test.mjs index 5c9d67ef78..a7ae737274 100644 --- a/node/test/test.mjs +++ b/node/test/test.mjs @@ -536,7 +536,6 @@ describe('Offline Tests with unconfigured account', function () { strictEqual(msg.getWidth(), 0, 'no message width') strictEqual(msg.isDeadDrop(), false, 'not deaddrop') strictEqual(msg.isForwarded(), false, 'not forwarded') - strictEqual(msg.isIncreation(), false, 'not in creation') strictEqual(msg.isInfo(), false, 'not an info message') strictEqual(msg.isSent(), false, 'messge is not sent') strictEqual(msg.isSetupmessage(), false, 'not an autocrypt setup message') diff --git a/python/src/deltachat/chat.py b/python/src/deltachat/chat.py index d275e09ccb..b5cfa9235f 100644 --- a/python/src/deltachat/chat.py +++ b/python/src/deltachat/chat.py @@ -271,8 +271,7 @@ def send_msg(self, msg: Message) -> Message: :param msg: a :class:`deltachat.message.Message` instance previously returned by - e.g. :meth:`deltachat.message.Message.new_empty` or - :meth:`prepare_file`. + e.g. :meth:`deltachat.message.Message.new_empty`. :raises ValueError: if message can not be sent. :returns: a :class:`deltachat.message.Message` instance as @@ -341,37 +340,6 @@ def send_image(self, path): raise ValueError("message could not be sent") return Message.from_db(self.account, sent_id) - def prepare_message(self, msg): - """prepare a message for sending. - - :param msg: the message to be prepared. - :returns: a :class:`deltachat.message.Message` instance. - This is the same object that was passed in, which - has been modified with the new state of the core. - """ - msg_id = lib.dc_prepare_msg(self.account._dc_context, self.id, msg._dc_msg) - if msg_id == 0: - raise ValueError("message could not be prepared") - # modify message in place to avoid bad state for the caller - msg._dc_msg = Message.from_db(self.account, msg_id)._dc_msg - return msg - - def prepare_message_file(self, path, mime_type=None, view_type="file"): - """prepare a message for sending and return the resulting Message instance. - - To actually send the message, call :meth:`send_prepared`. - The file must be inside the blob directory. - - :param path: path to the file. - :param mime_type: the mime-type of this file, defaults to auto-detection. - :param view_type: "text", "image", "gif", "audio", "video", "file" - :raises ValueError: if message can not be prepared/chat does not exist. - :returns: the resulting :class:`Message` instance - """ - msg = Message.new_empty(self.account, view_type) - msg.set_file(path, mime_type) - return self.prepare_message(msg) - def send_prepared(self, message): """send a previously prepared message. diff --git a/python/tests/test_1_online.py b/python/tests/test_1_online.py index d6bc3d6e9f..de7b38f6d9 100644 --- a/python/tests/test_1_online.py +++ b/python/tests/test_1_online.py @@ -1366,10 +1366,9 @@ def test_quote_encrypted(acfactory, lp): msg_draft.quote = quoted_msg chat.set_draft(msg_draft) - # Get the draft, prepare and send it. + # Get the draft and send it. msg_draft = chat.get_draft() - msg_out = chat.prepare_message(msg_draft) - chat.send_prepared(msg_out) + chat.send_msg(msg_draft) chat.set_draft(None) assert chat.get_draft() is None @@ -2291,9 +2290,8 @@ def test_group_quote(acfactory, lp): reply_msg = Message.new_empty(msg.chat.account, "text") reply_msg.set_text("reply") reply_msg.quote = msg - reply_msg = msg.chat.prepare_message(reply_msg) assert reply_msg.quoted_text == "hello" - msg.chat.send_prepared(reply_msg) + msg.chat.send_msg(reply_msg) lp.sec("ac3: receiving reply") received_reply = ac3._evtracker.wait_next_incoming_message() diff --git a/python/tests/test_2_increation.py b/python/tests/test_2_increation.py deleted file mode 100644 index 1fe85e02a7..0000000000 --- a/python/tests/test_2_increation.py +++ /dev/null @@ -1,107 +0,0 @@ -import os.path -import shutil -from filecmp import cmp - -import pytest - - -def wait_msg_delivered(account, msg_list): - """wait for one or more MSG_DELIVERED events to match msg_list contents.""" - msg_list = list(msg_list) - while msg_list: - ev = account._evtracker.get_matching("DC_EVENT_MSG_DELIVERED") - msg_list.remove((ev.data1, ev.data2)) - - -def wait_msgs_changed(account, msgs_list): - """wait for one or more MSGS_CHANGED events to match msgs_list contents.""" - account.log(f"waiting for msgs_list={msgs_list}") - msgs_list = list(msgs_list) - while msgs_list: - ev = account._evtracker.get_matching("DC_EVENT_MSGS_CHANGED") - for i, (data1, data2) in enumerate(msgs_list): - if ev.data1 == data1: - if data2 is None or ev.data2 == data2: - del msgs_list[i] - break - else: - account.log(f"waiting mismatch data1={data1} data2={data2}") - return ev.data2 - - -class TestOnlineInCreation: - def test_increation_not_blobdir(self, tmp_path, acfactory, lp): - ac1, ac2 = acfactory.get_online_accounts(2) - chat = ac1.create_chat(ac2) - - lp.sec("Creating in-creation file outside of blobdir") - assert str(tmp_path) != ac1.get_blobdir() - src = tmp_path / "file.txt" - src.touch() - with pytest.raises(Exception): - chat.prepare_message_file(str(src)) - - def test_no_increation_copies_to_blobdir(self, tmp_path, acfactory, lp): - ac1, ac2 = acfactory.get_online_accounts(2) - chat = ac1.create_chat(ac2) - - lp.sec("Creating file outside of blobdir") - assert str(tmp_path) != ac1.get_blobdir() - src = tmp_path / "file.txt" - src.write_text("hello there\n") - msg = chat.send_file(str(src)) - assert msg.filename.startswith(os.path.join(ac1.get_blobdir(), "file")) - assert msg.filename.endswith(".txt") - - def test_forward_increation(self, acfactory, data, lp): - ac1, ac2 = acfactory.get_online_accounts(2) - - chat = ac1.create_chat(ac2) - wait_msgs_changed(ac1, [(0, 0)]) # why no chat id? - - lp.sec("create a message with a file in creation") - orig = data.get_path("d.png") - path = os.path.join(ac1.get_blobdir(), "d.png") - with open(path, "x") as fp: - fp.write("preparing") - prepared_original = chat.prepare_message_file(path) - assert prepared_original.is_out_preparing() - wait_msgs_changed(ac1, [(chat.id, prepared_original.id)]) - - lp.sec("create a new group") - chat2 = ac1.create_group_chat("newgroup") - wait_msgs_changed(ac1, [(0, 0)]) - - lp.sec("add a contact to new group") - chat2.add_contact(ac2) - wait_msgs_changed(ac1, [(chat2.id, None)]) - - lp.sec("forward the message while still in creation") - ac1.forward_messages([prepared_original], chat2) - forwarded_id = wait_msgs_changed(ac1, [(chat2.id, None)]) - forwarded_msg = ac1.get_message_by_id(forwarded_id) - assert forwarded_msg.is_out_preparing() - - lp.sec("finish creating the file and send it") - assert prepared_original.is_out_preparing() - shutil.copyfile(orig, path) - chat.send_prepared(prepared_original) - assert prepared_original.is_out_pending() or prepared_original.is_out_delivered() - - lp.sec("check that both forwarded and original message are proper.") - wait_msgs_changed(ac1, [(chat2.id, forwarded_id), (chat.id, prepared_original.id)]) - - fwd_msg = ac1.get_message_by_id(forwarded_id) - assert fwd_msg.is_out_pending() or fwd_msg.is_out_delivered() - - lp.sec("wait for both messages to be delivered to SMTP") - wait_msg_delivered(ac1, [(chat2.id, forwarded_id), (chat.id, prepared_original.id)]) - - lp.sec("wait1 for original or forwarded messages to arrive") - received_original = ac2._evtracker.wait_next_incoming_message() - assert cmp(received_original.filename, orig, shallow=False) - - lp.sec("wait2 for original or forwarded messages to arrive") - received_copy = ac2._evtracker.wait_next_incoming_message() - assert received_copy.id != received_original.id - assert cmp(received_copy.filename, orig, shallow=False) diff --git a/python/tests/test_3_offline.py b/python/tests/test_3_offline.py index dcd752d551..cb5d2ac77f 100644 --- a/python/tests/test_3_offline.py +++ b/python/tests/test_3_offline.py @@ -378,30 +378,6 @@ def test_delete_and_send_fails(self, ac1, chat1): with pytest.raises(ValueError): chat1.send_text("msg1") - def test_prepare_message_and_send(self, ac1, chat1): - msg = chat1.prepare_message(Message.new_empty(chat1.account, "text")) - msg.set_text("hello world") - assert msg.text == "hello world" - assert msg.id > 0 - chat1.send_prepared(msg) - assert "Sent" in msg.get_message_info() - str(msg) - repr(msg) - assert msg == ac1.get_message_by_id(msg.id) - - def test_prepare_file(self, ac1, chat1): - blobdir = ac1.get_blobdir() - p = os.path.join(blobdir, "somedata.txt") - with open(p, "w") as f: - f.write("some data") - message = chat1.prepare_message_file(p) - assert message.id > 0 - message.set_text("hello world") - assert message.is_out_preparing() - assert message.text == "hello world" - chat1.send_prepared(message) - assert "Sent" in message.get_message_info() - def test_message_eq_contains(self, chat1): msg = chat1.send_text("msg1") msg2 = None @@ -691,8 +667,7 @@ def test_import_encrypted_bak_into_encrypted_acct(self, acfactory, tmp_path): assert os.path.exists(messages[1].filename) def test_set_get_draft(self, chat1): - msg = Message.new_empty(chat1.account, "text") - msg1 = chat1.prepare_message(msg) + msg1 = Message.new_empty(chat1.account, "text") msg1.set_text("hello") chat1.set_draft(msg1) msg1.set_text("obsolete") @@ -711,21 +686,6 @@ def test_qr_setup_contact(self, acfactory): assert not res.is_ask_verifygroup() assert res.contact_id == 10 - def test_quote(self, chat1): - """Offline quoting test""" - msg = Message.new_empty(chat1.account, "text") - msg.set_text("Multi\nline\nmessage") - assert msg.quoted_text is None - - # Prepare message to assign it a Message-Id. - # Messages without Message-Id cannot be quoted. - msg = chat1.prepare_message(msg) - - reply_msg = Message.new_empty(chat1.account, "text") - reply_msg.set_text("reply") - reply_msg.quote = msg - assert reply_msg.quoted_text == "Multi\nline\nmessage" - def test_group_chat_many_members_add_remove(self, ac1, lp): lp.sec("ac1: creating group chat with 10 other members") chat = ac1.create_group_chat(name="title1") diff --git a/src/blob.rs b/src/blob.rs index 43a42850f6..63c2350562 100644 --- a/src/blob.rs +++ b/src/blob.rs @@ -763,7 +763,6 @@ mod tests { use fs::File; use super::*; - use crate::chat::{self, create_group_chat, ProtectionStatus}; use crate::message::{Message, Viewtype}; use crate::test_utils::{self, TestContext}; @@ -1456,38 +1455,4 @@ mod tests { check_image_size(file_saved, width, height); Ok(()) } - - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn test_increation_in_blobdir() -> Result<()> { - let t = TestContext::new_alice().await; - let chat_id = create_group_chat(&t, ProtectionStatus::Unprotected, "abc").await?; - - let file = t.get_blobdir().join("anyfile.dat"); - fs::write(&file, b"bla").await?; - let mut msg = Message::new(Viewtype::File); - msg.set_file(file.to_str().unwrap(), None); - let prepared_id = chat::prepare_msg(&t, chat_id, &mut msg).await?; - assert_eq!(prepared_id, msg.id); - assert!(msg.is_increation()); - - let msg = Message::load_from_db(&t, prepared_id).await?; - assert!(msg.is_increation()); - - Ok(()) - } - - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn test_increation_not_blobdir() -> Result<()> { - let t = TestContext::new_alice().await; - let chat_id = create_group_chat(&t, ProtectionStatus::Unprotected, "abc").await?; - assert_ne!(t.get_blobdir().to_str(), t.dir.path().to_str()); - - let file = t.dir.path().join("anyfile.dat"); - fs::write(&file, b"bla").await?; - let mut msg = Message::new(Viewtype::File); - msg.set_file(file.to_str().unwrap(), None); - assert!(chat::prepare_msg(&t, chat_id, &mut msg).await.is_err()); - - Ok(()) - } } diff --git a/src/chat.rs b/src/chat.rs index d05ce56f7e..9ad4d87628 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -888,7 +888,7 @@ impl ChatId { _ => { let blob = msg .param - .get_blob(Param::File, context, !msg.is_increation()) + .get_blob(Param::File, context) .await? .context("no file stored in params")?; msg.param.set(Param::File, blob.as_name()); @@ -2677,26 +2677,13 @@ impl ChatIdBlocked { } } -/// Prepares a message for sending. -pub async fn prepare_msg(context: &Context, chat_id: ChatId, msg: &mut Message) -> Result { - ensure!( - !chat_id.is_special(), - "Cannot prepare message for special chat" - ); - - let msg_id = prepare_msg_common(context, chat_id, msg, MessageState::OutPreparing).await?; - context.emit_msgs_changed(msg.chat_id, msg.id); - - Ok(msg_id) -} - async fn prepare_msg_blob(context: &Context, msg: &mut Message) -> Result<()> { if msg.viewtype == Viewtype::Text || msg.viewtype == Viewtype::VideochatInvitation { // the caller should check if the message text is empty } else if msg.viewtype.has_file() { let mut blob = msg .param - .get_blob(Param::File, context, !msg.is_increation()) + .get_blob(Param::File, context) .await? .with_context(|| format!("attachment missing for message of type #{}", msg.viewtype))?; let send_as_is = msg.viewtype == Viewtype::File; @@ -2771,75 +2758,6 @@ async fn prepare_msg_blob(context: &Context, msg: &mut Message) -> Result<()> { Ok(()) } -/// Prepares a message to be sent out. -async fn prepare_msg_common( - context: &Context, - chat_id: ChatId, - msg: &mut Message, - change_state_to: MessageState, -) -> Result { - let mut chat = Chat::load_from_db(context, chat_id).await?; - - // Check if the chat can be sent to. - if let Some(reason) = chat.why_cant_send(context).await? { - if matches!( - reason, - CantSendReason::ProtectionBroken - | CantSendReason::ContactRequest - | CantSendReason::SecurejoinWait - ) && msg.param.get_cmd() == SystemMessage::SecurejoinMessage - { - // Send out the message, the securejoin message is supposed to repair the verification. - // If the chat is a contact request, let the user accept it later. - } else { - bail!("cannot send to {chat_id}: {reason}"); - } - } - - // Check a quote reply is not leaking data from other chats. - // This is meant as a last line of defence, the UI should check that before as well. - // (We allow Chattype::Single in general for "Reply Privately"; - // checking for exact contact_id will produce false positives when ppl just left the group) - if chat.typ != Chattype::Single && !context.get_config_bool(Config::Bot).await? { - if let Some(quoted_message) = msg.quoted_message(context).await? { - if quoted_message.chat_id != chat_id { - bail!("Bad quote reply"); - } - } - } - - // check current MessageState for drafts (to keep msg_id) ... - let update_msg_id = if msg.state == MessageState::OutDraft { - msg.hidden = false; - if !msg.id.is_special() && msg.chat_id == chat_id { - Some(msg.id) - } else { - None - } - } else { - None - }; - - // ... then change the MessageState in the message object - msg.state = change_state_to; - - prepare_msg_blob(context, msg).await?; - if !msg.hidden { - chat_id.unarchive_if_not_muted(context, msg.state).await?; - } - msg.id = chat - .prepare_msg_raw( - context, - msg, - update_msg_id, - create_smeared_timestamp(context), - ) - .await?; - msg.chat_id = chat_id; - - Ok(msg.id) -} - /// Returns whether a contact is in a chat or not. pub async fn is_contact_in_chat( context: &Context, @@ -2935,27 +2853,73 @@ async fn send_msg_inner(context: &Context, chat_id: ChatId, msg: &mut Message) - Ok(msg.id) } +/// Prepares a message to be sent out. +/// /// Returns row ids of the `smtp` table. async fn prepare_send_msg( context: &Context, chat_id: ChatId, msg: &mut Message, ) -> Result> { - // prepare_msg() leaves the message state to OutPreparing, we - // only have to change the state to OutPending in this case. - // Otherwise we still have to prepare the message, which will set - // the state to OutPending. - if msg.state != MessageState::OutPreparing { - // automatically prepare normal messages - prepare_msg_common(context, chat_id, msg, MessageState::OutPending).await?; + let mut chat = Chat::load_from_db(context, chat_id).await?; + + // Check if the chat can be sent to. + if let Some(reason) = chat.why_cant_send(context).await? { + if matches!( + reason, + CantSendReason::ProtectionBroken + | CantSendReason::ContactRequest + | CantSendReason::SecurejoinWait + ) && msg.param.get_cmd() == SystemMessage::SecurejoinMessage + { + // Send out the message, the securejoin message is supposed to repair the verification. + // If the chat is a contact request, let the user accept it later. + } else { + bail!("cannot send to {chat_id}: {reason}"); + } + } + + // Check a quote reply is not leaking data from other chats. + // This is meant as a last line of defence, the UI should check that before as well. + // (We allow Chattype::Single in general for "Reply Privately"; + // checking for exact contact_id will produce false positives when ppl just left the group) + if chat.typ != Chattype::Single && !context.get_config_bool(Config::Bot).await? { + if let Some(quoted_message) = msg.quoted_message(context).await? { + if quoted_message.chat_id != chat_id { + bail!("Bad quote reply"); + } + } + } + + // check current MessageState for drafts (to keep msg_id) ... + let update_msg_id = if msg.state == MessageState::OutDraft { + msg.hidden = false; + if !msg.id.is_special() && msg.chat_id == chat_id { + Some(msg.id) + } else { + None + } } else { - // update message state of separately prepared messages - ensure!( - chat_id.is_unset() || chat_id == msg.chat_id, - "Inconsistent chat ID" - ); - message::update_msg_state(context, msg.id, MessageState::OutPending).await?; + None + }; + + // ... then change the MessageState in the message object + msg.state = MessageState::OutPending; + + prepare_msg_blob(context, msg).await?; + if !msg.hidden { + chat_id.unarchive_if_not_muted(context, msg.state).await?; } + msg.id = chat + .prepare_msg_raw( + context, + msg, + update_msg_id, + create_smeared_timestamp(context), + ) + .await?; + msg.chat_id = chat_id; + let row_ids = create_send_msg_jobs(context, msg) .await .context("Failed to create send jobs")?; @@ -4173,8 +4137,6 @@ pub async fn forward_msgs(context: &Context, msg_ids: &[MsgId], chat_id: ChatId) bail!("cannot forward drafts."); } - let original_param = msg.param.clone(); - // we tested a sort of broadcast // by not marking own forwarded messages as such, // however, this turned out to be to confusing and unclear. @@ -4197,33 +4159,13 @@ pub async fn forward_msgs(context: &Context, msg_ids: &[MsgId], chat_id: ChatId) // do not leak data as group names; a default subject is generated by mimefactory msg.subject = "".to_string(); - let new_msg_id: MsgId; - if msg.state == MessageState::OutPreparing { - new_msg_id = chat - .prepare_msg_raw(context, &mut msg, None, curr_timestamp) - .await?; - curr_timestamp += 1; - msg.param = original_param; - msg.id = src_msg_id; - - if let Some(old_fwd) = msg.param.get(Param::PrepForwards) { - let new_fwd = format!("{} {}", old_fwd, new_msg_id.to_u32()); - msg.param.set(Param::PrepForwards, new_fwd); - } else { - msg.param - .set(Param::PrepForwards, new_msg_id.to_u32().to_string()); - } - - msg.update_param(context).await?; - } else { - msg.state = MessageState::OutPending; - new_msg_id = chat - .prepare_msg_raw(context, &mut msg, None, curr_timestamp) - .await?; - curr_timestamp += 1; - if !create_send_msg_jobs(context, &mut msg).await?.is_empty() { - context.scheduler.interrupt_smtp().await; - } + msg.state = MessageState::OutPending; + let new_msg_id = chat + .prepare_msg_raw(context, &mut msg, None, curr_timestamp) + .await?; + curr_timestamp += 1; + if !create_send_msg_jobs(context, &mut msg).await?.is_empty() { + context.scheduler.interrupt_smtp().await; } created_chats.push(chat_id); created_msgs.push(new_msg_id); @@ -4866,15 +4808,12 @@ mod tests { assert_eq!(test.text, "hello2".to_string()); assert_eq!(test.state, MessageState::OutDraft); - let id_after_prepare = prepare_msg(&t, *chat_id, &mut msg).await?; - assert_eq!(id_after_prepare, id_after_1st_set); - let test = Message::load_from_db(&t, id_after_prepare).await?; - assert_eq!(test.state, MessageState::OutPreparing); - assert!(!test.hidden); // sent draft must no longer be hidden - let id_after_send = send_msg(&t, *chat_id, &mut msg).await?; assert_eq!(id_after_send, id_after_1st_set); + let test = Message::load_from_db(&t, id_after_send).await?; + assert!(!test.hidden); // sent draft must no longer be hidden + Ok(()) } @@ -5626,7 +5565,6 @@ mod tests { let mut msg = Message::new_text("message text".to_string()); assert!(send_msg(&t, device_chat_id, &mut msg).await.is_err()); - assert!(prepare_msg(&t, device_chat_id, &mut msg).await.is_err()); let msg_id = add_device_msg(&t, None, Some(&mut msg)).await.unwrap(); assert!(forward_msgs(&t, &[msg_id], device_chat_id).await.is_err()); diff --git a/src/ephemeral.rs b/src/ephemeral.rs index 276119028f..f8ff056dab 100644 --- a/src/ephemeral.rs +++ b/src/ephemeral.rs @@ -930,7 +930,6 @@ mod tests { // Alice sends a text message. let mut msg = Message::new(Viewtype::Text); - chat::prepare_msg(&alice.ctx, chat_alice, &mut msg).await?; chat::send_msg(&alice.ctx, chat_alice, &mut msg).await?; let sent = alice.pop_sent_msg().await; @@ -957,14 +956,12 @@ mod tests { // Alice sends message to Bob let mut msg = Message::new(Viewtype::Text); - chat::prepare_msg(&alice.ctx, chat_alice, &mut msg).await?; chat::send_msg(&alice.ctx, chat_alice, &mut msg).await?; let sent = alice.pop_sent_msg().await; bob.recv_msg(&sent).await; // Alice sends second message to Bob, with no timer let mut msg = Message::new(Viewtype::Text); - chat::prepare_msg(&alice.ctx, chat_alice, &mut msg).await?; chat::send_msg(&alice.ctx, chat_alice, &mut msg).await?; let sent = alice.pop_sent_msg().await; diff --git a/src/message.rs b/src/message.rs index 75f3813cd8..f31df1a097 100644 --- a/src/message.rs +++ b/src/message.rs @@ -953,18 +953,6 @@ impl Message { cmd != SystemMessage::Unknown } - /// Whether the message is still being created. - /// - /// Messages with attachments might be created before the - /// attachment is ready. In this case some more restrictions on - /// the attachment apply, e.g. if the file to be attached is still - /// being written to or otherwise will still change it can not be - /// copied to the blobdir. Thus those attachments need to be - /// created immediately in the blobdir with a valid filename. - pub fn is_increation(&self) -> bool { - self.viewtype.has_file() && self.state == MessageState::OutPreparing - } - /// Returns true if the message is an Autocrypt Setup Message. pub fn is_setupmessage(&self) -> bool { if self.viewtype != Viewtype::File { @@ -2206,38 +2194,6 @@ mod tests { ); } - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn test_prepare_message_and_send() { - let d = test::TestContext::new().await; - let ctx = &d.ctx; - - ctx.set_config(Config::ConfiguredAddr, Some("self@example.com")) - .await - .unwrap(); - - let chat = d.create_chat_with_contact("", "dest@example.com").await; - - let mut msg = Message::new(Viewtype::Text); - - let msg_id = chat::prepare_msg(ctx, chat.id, &mut msg).await.unwrap(); - - let _msg2 = Message::load_from_db(ctx, msg_id).await.unwrap(); - assert_eq!(_msg2.get_filemime(), None); - } - - /// Tests that message can be prepared even if account has no configured address. - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn test_prepare_not_configured() { - let d = test::TestContext::new().await; - let ctx = &d.ctx; - - let chat = d.create_chat_with_contact("", "dest@example.com").await; - - let mut msg = Message::new(Viewtype::Text); - - assert!(chat::prepare_msg(ctx, chat.id, &mut msg).await.is_ok()); - } - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_parse_webrtc_instance() { let (webrtc_type, url) = Message::parse_webrtc_instance("basicwebrtc:https://foo/bar"); @@ -2357,9 +2313,9 @@ mod tests { let mut msg = Message::new_text("Quoted message".to_string()); - // Prepare message for sending, so it gets a Message-Id. + // Send message, so it gets a Message-Id. assert!(msg.rfc724_mid.is_empty()); - let msg_id = chat::prepare_msg(ctx, chat.id, &mut msg).await.unwrap(); + let msg_id = chat::send_msg(ctx, chat.id, &mut msg).await.unwrap(); let msg = Message::load_from_db(ctx, msg_id).await.unwrap(); assert!(!msg.rfc724_mid.is_empty()); diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 52820a1437..8f5f66f015 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -1516,7 +1516,7 @@ async fn build_body_file( ) -> Result<(PartBuilder, String)> { let blob = msg .param - .get_blob(Param::File, context, true) + .get_blob(Param::File, context) .await? .context("msg has no file")?; let suffix = blob.suffix().unwrap_or("dat"); @@ -1905,7 +1905,7 @@ mod tests { ) .await .unwrap(); - let new_msg = incoming_msg_to_reply_msg( + let mut new_msg = incoming_msg_to_reply_msg( b"Received: (Postfix, from userid 1000); Mon, 4 Dec 2006 14:51:39 +0100 (CET)\n\ From: bob@example.com\n\ To: alice@example.org\n\ @@ -1931,6 +1931,9 @@ mod tests { Original-Message-ID: <2893@example.com>\n\ Disposition: manual-action/MDN-sent-automatically; displayed\n\ \n", &t).await; + chat::send_msg(&t, new_msg.chat_id, &mut new_msg) + .await + .unwrap(); let mf = MimeFactory::from_msg(&t, new_msg).await.unwrap(); // The subject string should not be "Re: message opened" assert_eq!("Re: Hello, Bob", mf.subject_str(&t).await.unwrap()); @@ -2077,7 +2080,7 @@ mod tests { let mut new_msg = Message::new_text("Hi".to_string()); new_msg.chat_id = chat_id; - chat::prepare_msg(&t, chat_id, &mut new_msg).await.unwrap(); + chat::send_msg(&t, chat_id, &mut new_msg).await.unwrap(); let mf = MimeFactory::from_msg(&t, new_msg).await.unwrap(); @@ -2134,7 +2137,7 @@ mod tests { ) -> String { let t = TestContext::new_alice().await; let mut new_msg = incoming_msg_to_reply_msg(imf_raw, &t).await; - let incoming_msg = get_chat_msg(&t, new_msg.chat_id, 0, 2).await; + let incoming_msg = get_chat_msg(&t, new_msg.chat_id, 0, 1).await; if delete_original_msg { incoming_msg.id.trash(&t, false).await.unwrap(); @@ -2164,6 +2167,9 @@ mod tests { new_msg.set_quote(&t, Some(&incoming_msg)).await.unwrap(); } + chat::send_msg(&t, new_msg.chat_id, &mut new_msg) + .await + .unwrap(); let mf = MimeFactory::from_msg(&t, new_msg).await.unwrap(); mf.subject_str(&t).await.unwrap() } @@ -2184,9 +2190,6 @@ mod tests { let mut new_msg = Message::new_text("Hi".to_string()); new_msg.chat_id = chat_id; - chat::prepare_msg(context, chat_id, &mut new_msg) - .await - .unwrap(); new_msg } @@ -2197,7 +2200,7 @@ mod tests { let t = TestContext::new_alice().await; let context = &t; - let msg = incoming_msg_to_reply_msg( + let mut msg = incoming_msg_to_reply_msg( b"Received: (Postfix, from userid 1000); Mon, 4 Dec 2006 14:51:39 +0100 (CET)\n\ From: Charlie \n\ To: alice@example.org\n\ @@ -2210,6 +2213,7 @@ mod tests { context, ) .await; + chat::send_msg(&t, msg.chat_id, &mut msg).await.unwrap(); let mimefactory = MimeFactory::from_msg(&t, msg).await.unwrap(); diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 6fcc9603c7..caae972b81 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -3151,11 +3151,7 @@ MDYyMDYxNTE1RTlDOEE4Cj4+CnN0YXJ0eHJlZgo4Mjc4CiUlRU9GCg== // Make sure the file is there even though the html is wrong: let param = &message.parts[0].param; - let blob: BlobObject = param - .get_blob(Param::File, &t, false) - .await - .unwrap() - .unwrap(); + let blob: BlobObject = param.get_blob(Param::File, &t).await.unwrap().unwrap(); let f = tokio::fs::File::open(blob.to_abs_path()).await.unwrap(); let size = f.metadata().await.unwrap().len(); assert_eq!(size, 154); diff --git a/src/param.rs b/src/param.rs index b65cfd1d39..d83beef6b7 100644 --- a/src/param.rs +++ b/src/param.rs @@ -366,20 +366,16 @@ impl Params { /// /// This parses the parameter value as a [ParamsFile] and than /// tries to return a [BlobObject] for that file. If the file is - /// not yet a valid blob, one will be created by copying the file - /// only if `create` is set to `true`, otherwise an error is - /// returned. + /// not yet a valid blob, one will be created by copying the file. /// /// Note that in the [ParamsFile::FsPath] case the blob can be /// created without copying if the path already refers to a valid - /// blob. If so a [BlobObject] will be returned regardless of the - /// `create` argument. + /// blob. If so a [BlobObject] will be returned. #[allow(clippy::needless_lifetimes)] pub async fn get_blob<'a>( &self, key: Param, context: &'a Context, - create: bool, ) -> Result>> { let val = match self.get(key) { Some(val) => val, @@ -387,10 +383,7 @@ impl Params { }; let file = ParamsFile::from_param(context, val)?; let blob = match file { - ParamsFile::FsPath(path) => match create { - true => BlobObject::new_from_path(context, &path).await?, - false => BlobObject::from_path(context, &path)?, - }, + ParamsFile::FsPath(path) => BlobObject::new_from_path(context, &path).await?, ParamsFile::Blob(blob) => blob, }; Ok(Some(blob)) @@ -546,23 +539,20 @@ mod tests { let path: PathBuf = p.get_path(Param::File, &t).unwrap().unwrap(); assert_eq!(path, fname); - // Blob does not exist yet, expect error. - assert!(p.get_blob(Param::File, &t, false).await.is_err()); - fs::write(fname, b"boo").await.unwrap(); - let blob = p.get_blob(Param::File, &t, true).await.unwrap().unwrap(); + let blob = p.get_blob(Param::File, &t).await.unwrap().unwrap(); assert!(blob.as_file_name().starts_with("foo")); // Blob in blobdir, expect blob. let bar_path = t.get_blobdir().join("bar"); p.set(Param::File, bar_path.to_str().unwrap()); - let blob = p.get_blob(Param::File, &t, false).await.unwrap().unwrap(); + let blob = p.get_blob(Param::File, &t).await.unwrap().unwrap(); assert_eq!(blob, BlobObject::from_name(&t, "bar".to_string()).unwrap()); p.remove(Param::File); assert!(p.get_file(Param::File, &t).unwrap().is_none()); assert!(p.get_path(Param::File, &t).unwrap().is_none()); - assert!(p.get_blob(Param::File, &t, false).await.unwrap().is_none()); + assert!(p.get_blob(Param::File, &t).await.unwrap().is_none()); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] diff --git a/src/sql.rs b/src/sql.rs index 47604778d3..c1c46e2fb8 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -864,7 +864,6 @@ pub async fn remove_unused_files(context: &Context) -> Result<()> { if p == blobdir && (is_file_in_use(&files_in_use, None, &name_s) - || is_file_in_use(&files_in_use, Some(".increation"), &name_s) || is_file_in_use(&files_in_use, Some(".waveform"), &name_s) || is_file_in_use(&files_in_use, Some("-preview.jpg"), &name_s)) { From 2f7969b5ce766483312964f3ab80fc39499b8431 Mon Sep 17 00:00:00 2001 From: link2xt Date: Mon, 9 Dec 2024 06:11:15 +0000 Subject: [PATCH 2/4] do not allow special chat_id in send_msg --- src/chat.rs | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index 9ad4d87628..d809447477 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -2785,25 +2785,8 @@ pub async fn is_contact_in_chat( /// However, this does not imply, the message really reached the recipient - /// sending may be delayed eg. due to network problems. However, from your /// view, you're done with the message. Sooner or later it will find its way. -// TODO: Do not allow ChatId to be 0, if prepare_msg had been called -// the caller can get it from msg.chat_id. Forwards would need to -// be fixed for this somehow too. pub async fn send_msg(context: &Context, chat_id: ChatId, msg: &mut Message) -> Result { - if chat_id.is_unset() { - let forwards = msg.param.get(Param::PrepForwards); - if let Some(forwards) = forwards { - for forward in forwards.split(' ') { - if let Ok(msg_id) = forward.parse::().map(MsgId::new) { - if let Ok(mut msg) = Message::load_from_db(context, msg_id).await { - send_msg_inner(context, chat_id, &mut msg).await?; - }; - } - } - msg.param.remove(Param::PrepForwards); - msg.update_param(context).await?; - } - return send_msg_inner(context, chat_id, msg).await; - } + ensure!(!chat_id.is_special(), "chat_id cannot be a special chat: {chat_id}"); if msg.state != MessageState::Undefined && msg.state != MessageState::OutPreparing { msg.param.remove(Param::GuaranteeE2ee); From dd053115db2af7ef81e1157c665c01863cbfb900 Mon Sep 17 00:00:00 2001 From: link2xt Date: Mon, 9 Dec 2024 06:15:25 +0000 Subject: [PATCH 3/4] merge send_msg_inner into send_msg --- src/chat.rs | 41 +++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index d809447477..606703084e 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -2793,29 +2793,7 @@ pub async fn send_msg(context: &Context, chat_id: ChatId, msg: &mut Message) -> msg.param.remove(Param::ForcePlaintext); msg.update_param(context).await?; } - send_msg_inner(context, chat_id, msg).await -} - -/// Tries to send a message synchronously. -/// -/// Creates jobs in the `smtp` table, then drectly opens an SMTP connection and sends the -/// message. If this fails, the jobs remain in the database for later sending. -pub async fn send_msg_sync(context: &Context, chat_id: ChatId, msg: &mut Message) -> Result { - let rowids = prepare_send_msg(context, chat_id, msg).await?; - if rowids.is_empty() { - return Ok(msg.id); - } - let mut smtp = crate::smtp::Smtp::new(); - for rowid in rowids { - send_msg_to_smtp(context, &mut smtp, rowid) - .await - .context("failed to send message, queued for later sending")?; - } - context.emit_msgs_changed(msg.chat_id, msg.id); - Ok(msg.id) -} -async fn send_msg_inner(context: &Context, chat_id: ChatId, msg: &mut Message) -> Result { // protect all system messages against RTLO attacks if msg.is_system_message() { msg.text = sanitize_bidi_characters(&msg.text); @@ -2836,6 +2814,25 @@ async fn send_msg_inner(context: &Context, chat_id: ChatId, msg: &mut Message) - Ok(msg.id) } +/// Tries to send a message synchronously. +/// +/// Creates jobs in the `smtp` table, then drectly opens an SMTP connection and sends the +/// message. If this fails, the jobs remain in the database for later sending. +pub async fn send_msg_sync(context: &Context, chat_id: ChatId, msg: &mut Message) -> Result { + let rowids = prepare_send_msg(context, chat_id, msg).await?; + if rowids.is_empty() { + return Ok(msg.id); + } + let mut smtp = crate::smtp::Smtp::new(); + for rowid in rowids { + send_msg_to_smtp(context, &mut smtp, rowid) + .await + .context("failed to send message, queued for later sending")?; + } + context.emit_msgs_changed(msg.chat_id, msg.id); + Ok(msg.id) +} + /// Prepares a message to be sent out. /// /// Returns row ids of the `smtp` table. From 3dfb2a23fec7fecca7eb679f4a9d44562289e4a7 Mon Sep 17 00:00:00 2001 From: link2xt Date: Mon, 9 Dec 2024 06:16:05 +0000 Subject: [PATCH 4/4] rustfmt --- src/chat.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/chat.rs b/src/chat.rs index 606703084e..437dbff4ef 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -2786,7 +2786,10 @@ pub async fn is_contact_in_chat( /// sending may be delayed eg. due to network problems. However, from your /// view, you're done with the message. Sooner or later it will find its way. pub async fn send_msg(context: &Context, chat_id: ChatId, msg: &mut Message) -> Result { - ensure!(!chat_id.is_special(), "chat_id cannot be a special chat: {chat_id}"); + ensure!( + !chat_id.is_special(), + "chat_id cannot be a special chat: {chat_id}" + ); if msg.state != MessageState::Undefined && msg.state != MessageState::OutPreparing { msg.param.remove(Param::GuaranteeE2ee);