Don't set fake msg_id in resent messages

Setting the msg_id to `u32::MAX` is hacky, and may just as well break
things as it may fix things, because some code may use the msg.id to
load information from the database, like `get_iroh_topic_for_msg()`.

From reading the code, I couldn't find any problem with leaving the
correct `msg_id`, and if there is one, then we should add a function
parameter `is_resending` that is checked in the corresponding places.
This commit is contained in:
Hocuri
2026-04-21 16:15:21 +02:00
parent 524db830f1
commit e7d0687d90
4 changed files with 14 additions and 45 deletions

View File

@@ -2754,7 +2754,7 @@ async fn prepare_send_msg(
}
chat.prepare_msg_raw(context, msg, update_msg_id).await?;
let row_ids = create_send_msg_jobs(context, msg, None)
let row_ids = create_send_msg_jobs(context, msg)
.await
.context("Failed to create send jobs")?;
if !row_ids.is_empty() {
@@ -2824,14 +2824,7 @@ async fn render_mime_message_and_pre_message(
/// Returns row ids if `smtp` table jobs were created or an empty `Vec` otherwise.
///
/// The caller has to interrupt SMTP loop or otherwise process new rows.
///
/// * `row_id` - Actual Message ID, if `Some`. This is to avoid updating the `msgs` row, in which
/// case `msg.id` is fake (`u32::MAX`);
pub(crate) async fn create_send_msg_jobs(
context: &Context,
msg: &mut Message,
row_id: Option<MsgId>,
) -> Result<Vec<i64>> {
pub(crate) async fn create_send_msg_jobs(context: &Context, msg: &mut Message) -> Result<Vec<i64>> {
let cmd = msg.param.get_cmd();
if cmd == SystemMessage::GroupNameChanged || cmd == SystemMessage::GroupDescriptionChanged {
msg.chat_id
@@ -2848,7 +2841,7 @@ pub(crate) async fn create_send_msg_jobs(
}
let needs_encryption = msg.param.get_bool(Param::GuaranteeE2ee).unwrap_or_default();
let mimefactory = match MimeFactory::from_msg(context, msg.clone(), row_id).await {
let mimefactory = match MimeFactory::from_msg(context, msg.clone()).await {
Ok(mf) => mf,
Err(err) => {
// Mark message as failed
@@ -4028,11 +4021,7 @@ pub(crate) async fn add_contact_to_chat_ex(
Ok(true)
}
async fn resend_last_msgs(
context: &Context,
chat: &Chat,
to_contact: &Contact,
) -> std::result::Result<(), anyhow::Error> {
async fn resend_last_msgs(context: &Context, chat: &Chat, to_contact: &Contact) -> Result<()> {
let chat_id = chat.id;
let msgs: Vec<MsgId> = context
.sql
@@ -4621,10 +4610,7 @@ pub async fn forward_msgs_2ctx(
chat.prepare_msg_raw(ctx_dst, &mut msg, None).await?;
curr_timestamp += 1;
if !create_send_msg_jobs(ctx_dst, &mut msg, None)
.await?
.is_empty()
{
if !create_send_msg_jobs(ctx_dst, &mut msg).await?.is_empty() {
ctx_dst.scheduler.interrupt_smtp().await;
}
created_msgs.push(msg.id);
@@ -4779,17 +4765,10 @@ pub(crate) async fn resend_msgs_ex(
}
msg_state => bail!("Unexpected message state {msg_state}"),
}
let mut row_id = None;
if let Some(to_fingerprint) = &to_fingerprint {
msg.param.set(Param::Arg4, to_fingerprint.clone());
// Avoid updating the `msgs` row.
row_id = Some(msg.id);
msg.id = MsgId::new(u32::MAX);
}
if create_send_msg_jobs(context, &mut msg, row_id)
.await?
.is_empty()
{
if create_send_msg_jobs(context, &mut msg).await?.is_empty() {
continue;
}

View File

@@ -195,15 +195,8 @@ fn new_address_with_name(name: &str, address: String) -> Address<'static> {
impl MimeFactory {
/// Returns `MimeFactory` for rendering `msg`.
///
/// * `row_id` - Actual Message ID, if `Some`. This is to avoid updating the `msgs` row, in
/// which case `msg.id` is fake (`u32::MAX`);
#[expect(clippy::arithmetic_side_effects)]
pub async fn from_msg(
context: &Context,
msg: Message,
row_id: Option<MsgId>,
) -> Result<MimeFactory> {
pub async fn from_msg(context: &Context, msg: Message) -> Result<MimeFactory> {
let now = time();
let chat = Chat::load_from_db(context, msg.chat_id).await?;
let attach_profile_data = Self::should_attach_profile_data(&msg);
@@ -513,13 +506,12 @@ impl MimeFactory {
};
}
let msg_id = row_id.unwrap_or(msg.id);
let (in_reply_to, references) = context
.sql
.query_row(
"SELECT mime_in_reply_to, IFNULL(mime_references, '')
FROM msgs WHERE id=?",
(msg_id,),
(msg.id,),
|row| {
let in_reply_to: String = row.get(0)?;
let references: String = row.get(1)?;

View File

@@ -275,7 +275,7 @@ async fn test_subject_mdn() {
chat::send_msg(&t, new_msg.chat_id, &mut new_msg)
.await
.unwrap();
let mf = MimeFactory::from_msg(&t, new_msg, None).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());
}
@@ -412,7 +412,7 @@ async fn first_subject_str(t: TestContext) -> String {
new_msg.chat_id = chat_id;
chat::send_msg(&t, chat_id, &mut new_msg).await.unwrap();
let mf = MimeFactory::from_msg(&t, new_msg, None).await.unwrap();
let mf = MimeFactory::from_msg(&t, new_msg).await.unwrap();
mf.subject_str(&t).await.unwrap()
}
@@ -500,7 +500,7 @@ async fn msg_to_subject_str_inner(
chat::send_msg(&t, new_msg.chat_id, &mut new_msg)
.await
.unwrap();
let mf = MimeFactory::from_msg(&t, new_msg, None).await.unwrap();
let mf = MimeFactory::from_msg(&t, new_msg).await.unwrap();
mf.subject_str(&t).await.unwrap()
}
@@ -545,7 +545,7 @@ async fn test_render_reply() {
.await;
chat::send_msg(&t, msg.chat_id, &mut msg).await.unwrap();
let mimefactory = MimeFactory::from_msg(&t, msg, None).await.unwrap();
let mimefactory = MimeFactory::from_msg(&t, msg).await.unwrap();
let recipients = mimefactory.recipients();
assert_eq!(recipients, vec!["charlie@example.com"]);