+
Skip to content

Do not fail to receive_imf call accepted/ended messages referring to non-call Message-ID #7292

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+205 −27
Open

Do not fail to receive_imf call accepted/ended messages referring to non-call Message-ID #7292

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3a4ae42
fix: do not fail to receive call accepted/ended messages referring to…
link2xt Oct 10, 2025
aa8a242
fix: do not try to process calls from partial messages
link2xt Oct 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions deltachat-jsonrpc/src/api/types/calls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use anyhow::Result;
use anyhow::{Context as _, Result};

use deltachat::calls::{call_state, sdp_has_video, CallState};
use deltachat::context::Context;
Expand Down Expand Up @@ -26,7 +26,9 @@ pub struct JsonrpcCallInfo {

impl JsonrpcCallInfo {
pub async fn from_msg_id(context: &Context, msg_id: MsgId) -> Result<JsonrpcCallInfo> {
let call_info = context.load_call_by_id(msg_id).await?;
let call_info = context.load_call_by_id(msg_id).await?.with_context(|| {
format!("Attempting to get call state of non-call message {msg_id}")
})?;
let sdp_offer = call_info.place_call_info.clone();
let has_video = sdp_has_video(&sdp_offer).unwrap_or_default();
let state = JsonrpcCallState::from_msg_id(context, msg_id).await?;
Expand Down
61 changes: 49 additions & 12 deletions src/calls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,9 @@ impl Context {
call_id: MsgId,
accept_call_info: String,
) -> Result<()> {
let mut call: CallInfo = self.load_call_by_id(call_id).await?;
let mut call: CallInfo = self.load_call_by_id(call_id).await?.with_context(|| {
format!("accept_incoming_call is called with {call_id} which does not refer to a call")
})?;
ensure!(call.is_incoming());
if call.is_accepted() || call.is_ended() {
info!(self, "Call already accepted/ended");
Expand Down Expand Up @@ -248,7 +250,9 @@ impl Context {

/// Cancel, decline or hangup an incoming or outgoing call.
pub async fn end_call(&self, call_id: MsgId) -> Result<()> {
let mut call: CallInfo = self.load_call_by_id(call_id).await?;
let mut call: CallInfo = self.load_call_by_id(call_id).await?.with_context(|| {
format!("end_call is called with {call_id} which does not refer to a call")
})?;
if call.is_ended() {
info!(self, "Call already ended");
return Ok(());
Expand Down Expand Up @@ -291,7 +295,13 @@ impl Context {
call_id: MsgId,
) -> Result<()> {
sleep(Duration::from_secs(wait)).await;
let mut call = context.load_call_by_id(call_id).await?;
let Some(mut call) = context.load_call_by_id(call_id).await? else {
warn!(
context,
"emit_end_call_if_unaccepted is called with {call_id} which does not refer to a call."
);
return Ok(());
};
if !call.is_accepted() && !call.is_ended() {
if call.is_incoming() {
call.mark_as_canceled(&context).await?;
Expand All @@ -316,7 +326,10 @@ impl Context {
from_id: ContactId,
) -> Result<()> {
if mime_message.is_call() {
let call = self.load_call_by_id(call_id).await?;
let Some(call) = self.load_call_by_id(call_id).await? else {
warn!(self, "{call_id} does not refer to a call message");
return Ok(());
};

if call.is_incoming() {
if call.is_stale() {
Expand Down Expand Up @@ -352,7 +365,11 @@ impl Context {
} else {
match mime_message.is_system_message {
SystemMessage::CallAccepted => {
let mut call = self.load_call_by_id(call_id).await?;
let Some(mut call) = self.load_call_by_id(call_id).await? else {
warn!(self, "{call_id} does not refer to a call message");
return Ok(());
};

if call.is_ended() || call.is_accepted() {
info!(self, "CallAccepted received for accepted/ended call");
return Ok(());
Expand All @@ -377,7 +394,11 @@ impl Context {
}
}
SystemMessage::CallEnded => {
let mut call = self.load_call_by_id(call_id).await?;
let Some(mut call) = self.load_call_by_id(call_id).await? else {
warn!(self, "{call_id} does not refer to a call message");
return Ok(());
};

if call.is_ended() {
// may happen eg. if a a message is missed
info!(self, "CallEnded received for ended call");
Expand Down Expand Up @@ -421,15 +442,26 @@ impl Context {
}

/// Loads information about the call given its ID.
pub async fn load_call_by_id(&self, call_id: MsgId) -> Result<CallInfo> {
///
/// If the message referred to by ID is
/// not a call message, returns `None`.
pub async fn load_call_by_id(&self, call_id: MsgId) -> Result<Option<CallInfo>> {
let call = Message::load_from_db(self, call_id).await?;
self.load_call_by_message(call)
Ok(self.load_call_by_message(call))
}

fn load_call_by_message(&self, call: Message) -> Result<CallInfo> {
ensure!(call.viewtype == Viewtype::Call);
// Loads information about the call given the `Message`.
//
// If the `Message` is not a call message, returns `None`
fn load_call_by_message(&self, call: Message) -> Option<CallInfo> {
if call.viewtype != Viewtype::Call {
// This can happen e.g. if a "call accepted"
// or "call ended" message is received
// with `In-Reply-To` referring to non-call message.
return None;
}

Ok(CallInfo {
Some(CallInfo {
place_call_info: call
.param
.get(Param::WebrtcRoom)
Expand Down Expand Up @@ -497,8 +529,13 @@ pub enum CallState {
}

/// Returns call state given the message ID.
///
/// Returns an error if the message is not a call message.
pub async fn call_state(context: &Context, msg_id: MsgId) -> Result<CallState> {
let call = context.load_call_by_id(msg_id).await?;
let call = context
.load_call_by_id(msg_id)
.await?
.with_context(|| format!("{msg_id} is not a call message"))?;
let state = if call.is_incoming() {
if call.is_accepted() {
if call.is_ended() {
Expand Down
153 changes: 145 additions & 8 deletions src/calls/calls_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use super::*;
use crate::chat::forward_msgs;
use crate::config::Config;
use crate::constants::DC_CHAT_ID_TRASH;
use crate::receive_imf::{receive_imf, receive_imf_from_inbox};
use crate::test_utils::{TestContext, TestContextManager};

struct CallSetup {
Expand Down Expand Up @@ -53,7 +55,10 @@ async fn setup_call() -> Result<CallSetup> {
for (t, m) in [(&alice, &alice_call), (&alice2, &alice2_call)] {
assert!(!m.is_info());
assert_eq!(m.viewtype, Viewtype::Call);
let info = t.load_call_by_id(m.id).await?;
let info = t
.load_call_by_id(m.id)
.await?
.expect("m should be a call message");
assert!(!info.is_incoming());
assert!(!info.is_accepted());
assert_eq!(info.place_call_info, PLACE_INFO);
Expand All @@ -71,7 +76,10 @@ async fn setup_call() -> Result<CallSetup> {
t.evtracker
.get_matching(|evt| matches!(evt, EventType::IncomingCall { .. }))
.await;
let info = t.load_call_by_id(m.id).await?;
let info = t
.load_call_by_id(m.id)
.await?
.expect("IncomingCall event should refer to a call message");
assert!(info.is_incoming());
assert!(!info.is_accepted());
assert_eq!(info.place_call_info, PLACE_INFO);
Expand Down Expand Up @@ -111,7 +119,10 @@ async fn accept_call() -> Result<CallSetup> {
.get_matching(|evt| matches!(evt, EventType::IncomingCallAccepted { .. }))
.await;
let sent2 = bob.pop_sent_msg().await;
let info = bob.load_call_by_id(bob_call.id).await?;
let info = bob
.load_call_by_id(bob_call.id)
.await?
.expect("bob_call should be a call message");
assert!(info.is_accepted());
assert_eq!(info.place_call_info, PLACE_INFO);
assert_eq!(call_state(&bob, bob_call.id).await?, CallState::Active);
Expand All @@ -121,7 +132,10 @@ async fn accept_call() -> Result<CallSetup> {
bob2.evtracker
.get_matching(|evt| matches!(evt, EventType::IncomingCallAccepted { .. }))
.await;
let info = bob2.load_call_by_id(bob2_call.id).await?;
let info = bob2
.load_call_by_id(bob2_call.id)
.await?
.expect("bob2_call should be a call message");
assert!(info.is_accepted());
assert_eq!(call_state(&bob2, bob2_call.id).await?, CallState::Active);

Expand All @@ -140,7 +154,10 @@ async fn accept_call() -> Result<CallSetup> {
accept_call_info: ACCEPT_INFO.to_string()
}
);
let info = alice.load_call_by_id(alice_call.id).await?;
let info = alice
.load_call_by_id(alice_call.id)
.await?
.expect("alice_call should be a call message");
assert!(info.is_accepted());
assert_eq!(info.place_call_info, PLACE_INFO);
assert_eq!(call_state(&alice, alice_call.id).await?, CallState::Active);
Expand Down Expand Up @@ -460,14 +477,20 @@ async fn test_mark_calls() -> Result<()> {
alice, alice_call, ..
} = setup_call().await?;

let mut call_info: CallInfo = alice.load_call_by_id(alice_call.id).await?;
let mut call_info: CallInfo = alice
.load_call_by_id(alice_call.id)
.await?
.expect("alice_call should be a call message");
assert!(!call_info.is_accepted());
assert!(!call_info.is_ended());
call_info.mark_as_accepted(&alice).await?;
assert!(call_info.is_accepted());
assert!(!call_info.is_ended());

let mut call_info: CallInfo = alice.load_call_by_id(alice_call.id).await?;
let mut call_info: CallInfo = alice
.load_call_by_id(alice_call.id)
.await?
.expect("alice_call should be a call message");
assert!(call_info.is_accepted());
assert!(!call_info.is_ended());

Expand All @@ -484,7 +507,10 @@ async fn test_update_call_text() -> Result<()> {
alice, alice_call, ..
} = setup_call().await?;

let call_info = alice.load_call_by_id(alice_call.id).await?;
let call_info = alice
.load_call_by_id(alice_call.id)
.await?
.expect("alice_call should be a call message");
call_info.update_text(&alice, "foo bar").await?;

let alice_call = Message::load_from_db(&alice, alice_call.id).await?;
Expand Down Expand Up @@ -529,3 +555,114 @@ async fn test_forward_call() -> Result<()> {

Ok(())
}

/// Tests that "end call" message referring
/// to a text message does not make receive_imf fail.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_end_text_call() -> Result<()> {
let mut tcm = TestContextManager::new();
let alice = &tcm.alice().await;

let received1 = receive_imf(
alice,
b"From: bob@example.net\n\
To: alice@example.org\n\
Message-ID: <first@example.net>\n\
Date: Sun, 22 Mar 2020 22:37:57 +0000\n\
Chat-Version: 1.0\n\
\n\
Hello\n",
false,
)
.await?
.unwrap();
assert_eq!(received1.msg_ids.len(), 1);
let msg = Message::load_from_db(alice, received1.msg_ids[0])
.await
.unwrap();
assert_eq!(msg.viewtype, Viewtype::Text);

// Receiving "Call ended" message that refers
// to the text message does not result in an error.
let received2 = receive_imf(
alice,
b"From: bob@example.net\n\
To: alice@example.org\n\
Message-ID: <second@example.net>\n\
Date: Sun, 22 Mar 2020 23:37:57 +0000\n\
In-Reply-To: <first@example.net>\n\
Chat-Version: 1.0\n\
Chat-Content: call-ended\n\
\n\
Call ended\n",
false,
)
.await?
.unwrap();
assert_eq!(received2.msg_ids.len(), 1);
assert_eq!(received2.chat_id, DC_CHAT_ID_TRASH);

Ok(())
}

/// Tests that partially downloaded "call ended"
/// messages are not processed.
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_no_partial_calls() -> Result<()> {
let mut tcm = TestContextManager::new();
let alice = &tcm.alice().await;

let seen = false;

// The messages in the test
// have no `Date` on purpose,
// so they are treated as new.
let received_call = receive_imf(
alice,
b"From: bob@example.net\n\
To: alice@example.org\n\
Message-ID: <first@example.net>\n\
Chat-Version: 1.0\n\
Chat-Content: call\n\
Chat-Webrtc-Room: YWFhYWFhYWFhCg==\n\
\n\
Hello, this is a call\n",
seen,
)
.await?
.unwrap();
assert_eq!(received_call.msg_ids.len(), 1);
let call_msg = Message::load_from_db(alice, received_call.msg_ids[0])
.await
.unwrap();
assert_eq!(call_msg.viewtype, Viewtype::Call);
assert_eq!(call_state(alice, call_msg.id).await?, CallState::Alerting);

let imf_raw = b"From: bob@example.net\n\
To: alice@example.org\n\
Message-ID: <second@example.net>\n\
In-Reply-To: <first@example.net>\n\
Chat-Version: 1.0\n\
Chat-Content: call-ended\n\
\n\
Call ended\n";
receive_imf_from_inbox(
alice,
"second@example.net",
imf_raw,
seen,
Some(imf_raw.len().try_into().unwrap()),
)
.await?;

// The call is still not ended.
assert_eq!(call_state(alice, call_msg.id).await?, CallState::Alerting);

// Fully downloading the message ends the call.
receive_imf_from_inbox(alice, "second@example.net", imf_raw, seen, None)
.await
.context("Failed to fully download end call message")?;
assert_eq!(call_state(alice, call_msg.id).await?, CallState::Missed);

Ok(())
}
12 changes: 7 additions & 5 deletions src/receive_imf.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -999,7 +999,7 @@ pub(crate) async fn receive_imf_inner(
}
}

if mime_parser.is_call() {
if is_partial_download.is_none() && mime_parser.is_call() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, instead of this i'd suggest to only parse minimum set of headers in mimeparser for partial downloads. Otherwise it's hard (not possible in practice) to always check this is_partial_download for all kinds of functionality we have like calls, webxdcs, etc.

context
.handle_call_msg(insert_msg_id, &mime_parser, from_id)
.await?;
Expand Down Expand Up @@ -1157,8 +1157,9 @@ async fn decide_chat_assignment(
{
info!(context, "Chat edit/delete/iroh/sync message (TRASH).");
true
} else if mime_parser.is_system_message == SystemMessage::CallAccepted
|| mime_parser.is_system_message == SystemMessage::CallEnded
} else if is_partial_download.is_none()
&& (mime_parser.is_system_message == SystemMessage::CallAccepted
|| mime_parser.is_system_message == SystemMessage::CallEnded)
{
info!(context, "Call state changed (TRASH).");
true
Expand Down Expand Up @@ -1986,8 +1987,9 @@ async fn add_parts(

handle_edit_delete(context, mime_parser, from_id).await?;

if mime_parser.is_system_message == SystemMessage::CallAccepted
|| mime_parser.is_system_message == SystemMessage::CallEnded
if is_partial_download.is_none()
&& (mime_parser.is_system_message == SystemMessage::CallAccepted
|| mime_parser.is_system_message == SystemMessage::CallEnded)
{
if let Some(field) = mime_parser.get_header(HeaderDef::InReplyTo) {
if let Some(call) =
Expand Down
点击 这是indexloc提供的php浏览器服务，不要输入任何密码和下载