From 97abeb4b385fdb2b1329a6a2608010ae6d9c17fa Mon Sep 17 00:00:00 2001 From: Tyson George Date: Thu, 24 Sep 2026 08:55:35 -0400 Subject: [PATCH 01/16] fix: keep confirmed dialogs confirmed on 1xx to in-dialog requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit send_dialog_request moved the dialog to Early on every non-100 provisional, including responses to our own re-INVITE or UPDATE on an established dialog. A 183 to a session-refresh re-INVITE therefore regressed a Confirmed dialog to Early for good (the final 200 does not restore it): bye() is then refused outside Confirmed and hangup() falls through to a CANCEL of the long-completed INVITE, which times out, so the call can no longer be torn down. RFC 3261 §12 has a dialog move from early to confirmed and never back, and a provisional to a mid-dialog request does not create early state. Only take the Early transition while the dialog is still in a pre-confirmation state (Calling / Trying / Early). On a confirmed dialog the provisional is still notified as before, so callers keep seeing it, but the stored state stays Confirmed. The initial INVITE path (process_invite) is unchanged, and reliable provisionals to a re-INVITE are still PRACKed. Adds tests driving a raw UDP peer: the initial INVITE's 183 still reports Early, while a 100/183/200 to an in-dialog re-INVITE or UPDATE keeps the dialog Confirmed, the 183 is still notified, and BYE succeeds. --- src/dialog/dialog.rs | 12 +- src/dialog/tests/mod.rs | 1 + .../tests/test_in_dialog_provisional.rs | 218 ++++++++++++++++++ 3 files changed, 230 insertions(+), 1 deletion(-) create mode 100644 src/dialog/tests/test_in_dialog_provisional.rs diff --git a/src/dialog/dialog.rs b/src/dialog/dialog.rs index 9f62fc2a..81db5aa0 100644 --- a/src/dialog/dialog.rs +++ b/src/dialog/dialog.rs @@ -1250,7 +1250,17 @@ impl DialogInner { if method == Method::Invite { self.handle_provisional_response(&resp).await?; } - self.transition(DialogState::Early(self.id.lock().clone(), resp))?; + // RFC 3261 §12: a dialog moves from early to confirmed + // and never back. A 1xx to a mid-dialog request (re-INVITE, + // UPDATE, ...) must not regress an established dialog to + // Early, or BYE is refused and hangup() tries to CANCEL. + // The provisional is still notified so the caller sees it. + let state = DialogState::Early(self.id.lock().clone(), resp); + if self.can_cancel() { + self.transition(state)?; + } else { + self.state_sender.send(state).ok(); + } continue; } diff --git a/src/dialog/tests/mod.rs b/src/dialog/tests/mod.rs index 52a59d7a..433615b7 100644 --- a/src/dialog/tests/mod.rs +++ b/src/dialog/tests/mod.rs @@ -5,6 +5,7 @@ mod test_client_dialog; mod test_connection_affinity; mod test_dialog_layer; mod test_dialog_states; +mod test_in_dialog_provisional; mod test_in_dialog_via; mod test_invite_auth_challenge; mod test_prack; diff --git a/src/dialog/tests/test_in_dialog_provisional.rs b/src/dialog/tests/test_in_dialog_provisional.rs new file mode 100644 index 00000000..40b82bbc --- /dev/null +++ b/src/dialog/tests/test_in_dialog_provisional.rs @@ -0,0 +1,218 @@ +//! A provisional response to an in-dialog request must not move a confirmed +//! dialog back to the early state (RFC 3261 §12: a dialog goes early → +//! confirmed and never back; a 1xx to a mid-dialog request does not create +//! early state). Otherwise `bye()` refuses to run on the dialog and +//! `hangup()` falls through to a CANCEL of the long-completed INVITE. +use crate::dialog::{ + dialog::{DialogState, DialogStateReceiver}, + dialog_layer::DialogLayer, + invitation::InviteOption, + invite_dialog::InviteDialog, +}; +use crate::sip::{prelude::HeadersExt, Method, Request, SipMessage, Uri}; +use crate::transport::{udp::UdpConnection, TransportLayer}; +use crate::EndpointBuilder; +use std::net::SocketAddr; +use std::time::Duration; +use tokio::net::UdpSocket; +use tokio::sync::mpsc::unbounded_channel; +use tokio_util::sync::CancellationToken; + +const PEER_TAG: &str = "peer-tag"; + +/// Receive the next request with `method` on the raw peer socket, skipping +/// anything else (retransmissions, ACKs we are not waiting for). +async fn recv_request(socket: &UdpSocket, method: Method) -> (Request, SocketAddr) { + let mut buf = vec![0u8; 4096]; + loop { + let (len, from) = tokio::time::timeout(Duration::from_secs(2), socket.recv_from(&mut buf)) + .await + .unwrap_or_else(|_| panic!("timeout waiting for {method}")) + .expect("recv_from failed"); + let text = std::str::from_utf8(&buf[..len]).expect("non utf-8 SIP message"); + if let Ok(SipMessage::Request(req)) = SipMessage::try_from(text) { + if req.method == method { + return (req, from); + } + } + } +} + +/// Build a response to `req` as the peer UA, adding the peer's To tag when +/// the request does not carry one yet. +fn response(req: &Request, code: u16, reason: &str, contact: &str) -> String { + let to = req.to_header().unwrap().value().to_string(); + let to = if to.contains(";tag=") { + to + } else { + format!("{to};tag={PEER_TAG}") + }; + format!( + "SIP/2.0 {code} {reason}\r\n\ + Via: {}\r\n\ + From: {}\r\n\ + To: {to}\r\n\ + Call-ID: {}\r\n\ + CSeq: {}\r\n\ + Contact: <{contact}>\r\n\ + Content-Length: 0\r\n\r\n", + req.via_header().unwrap().value(), + req.from_header().unwrap().value(), + req.call_id_header().unwrap().value(), + req.cseq_header().unwrap().value(), + ) +} + +async fn reply(socket: &UdpSocket, to: SocketAddr, req: &Request, code: u16, reason: &str) { + let contact = format!("sip:bob@{}", socket.local_addr().unwrap()); + socket + .send_to(response(req, code, reason, &contact).as_bytes(), to) + .await + .expect("send_to failed"); +} + +fn drain_states(rx: &mut DialogStateReceiver) -> Vec { + let mut states = Vec::new(); + while let Ok(state) = rx.try_recv() { + states.push(state); + } + states +} + +/// Set up a UAC endpoint and a raw UDP peer, and establish a dialog whose +/// initial INVITE is answered 100 → 183 → 200. Returns the confirmed dialog. +async fn establish( + token: &CancellationToken, +) -> crate::Result<(InviteDialog, DialogStateReceiver, UdpSocket)> { + let peer = UdpSocket::bind("127.0.0.1:0").await?; + let peer_port = peer.local_addr()?.port(); + + let transport_layer = TransportLayer::new(token.child_token()); + let udp = UdpConnection::create_connection( + "127.0.0.1:0".parse().unwrap(), + None, + Some(token.child_token()), + ) + .await?; + let uac_addr = udp.get_addr().addr.clone(); + transport_layer.add_transport(udp.into()); + let endpoint = EndpointBuilder::new() + .with_user_agent("rsipstack-test") + .with_transport_layer(transport_layer) + .with_cancel_token(token.child_token()) + .build(); + let endpoint_inner = endpoint.inner.clone(); + tokio::spawn(async move { + let _ = endpoint_inner.serve().await; + }); + let dialog_layer = DialogLayer::new(endpoint.inner.clone()); + + let (state_sender, mut state_receiver) = unbounded_channel(); + let invite_option = InviteOption { + caller: Uri::try_from("sip:alice@example.com")?, + callee: Uri::try_from(format!("sip:bob@127.0.0.1:{peer_port};transport=udp").as_str())?, + contact: Uri::try_from(format!("sip:alice@{uac_addr}").as_str())?, + ..Default::default() + }; + let invite = tokio::spawn(async move { + dialog_layer + .do_invite(invite_option, state_sender) + .await + .map(|(dialog, _)| dialog) + }); + + let (req, uac) = recv_request(&peer, Method::Invite).await; + reply(&peer, uac, &req, 100, "Trying").await; + reply(&peer, uac, &req, 183, "Session Progress").await; + reply(&peer, uac, &req, 200, "OK").await; + recv_request(&peer, Method::Ack).await; + + let dialog = invite.await.expect("do_invite task panicked")?; + assert!(dialog.state().is_confirmed(), "initial INVITE must confirm"); + + // The initial INVITE's 183 still produces early state, as before. + let states = drain_states(&mut state_receiver); + assert!( + states.iter().any(|s| matches!(s, DialogState::Early(_, _))), + "a 183 to the initial INVITE must report Early, got {states:?}" + ); + assert!( + matches!(states.last(), Some(DialogState::Confirmed(_, _))), + "initial INVITE must end Confirmed, got {states:?}" + ); + Ok((dialog, state_receiver, peer)) +} + +/// Send `method` in-dialog, answer it 100 → 183 → 200 from the peer, and +/// check the dialog stays Confirmed throughout and can still send BYE. +async fn assert_provisional_keeps_confirmed(method: Method) -> crate::Result<()> { + let token = CancellationToken::new(); + let (dialog, mut states, peer) = establish(&token).await?; + + let requester = dialog.clone(); + let pending = tokio::spawn(async move { + match method { + Method::Invite => requester.reinvite(None, None).await, + Method::Update => requester.update(None, None).await, + _ => unreachable!(), + } + }); + + let (req, uac) = recv_request(&peer, method).await; + reply(&peer, uac, &req, 100, "Trying").await; + reply(&peer, uac, &req, 183, "Session Progress").await; + // Let the dialog process the provisionals while the request is pending. + tokio::time::sleep(Duration::from_millis(100)).await; + assert!( + dialog.state().is_confirmed(), + "a 183 to an in-dialog {method} must not leave Confirmed, state: {}", + dialog.state() + ); + + reply(&peer, uac, &req, 200, "OK").await; + let resp = tokio::time::timeout(Duration::from_secs(2), pending) + .await + .expect("in-dialog request did not complete") + .expect("request task panicked")? + .expect("no final response"); + assert_eq!(resp.status_code, crate::sip::StatusCode::OK); + assert!( + dialog.state().is_confirmed(), + "dialog must be Confirmed after the in-dialog {method} completes, state: {}", + dialog.state() + ); + // The 183 is still delivered to the caller, it just does not change the + // dialog's state. + let states = drain_states(&mut states); + assert!( + states.iter().any(|s| matches!( + s, + DialogState::Early(_, r) if r.status_code == crate::sip::StatusCode::SessionProgress + )), + "the 183 to an in-dialog {method} must still be notified, got {states:?}" + ); + + // The call can still be hung up with a BYE. + let hanger = dialog.clone(); + let bye = tokio::spawn(async move { hanger.bye().await }); + let (bye_req, uac) = recv_request(&peer, Method::Bye).await; + reply(&peer, uac, &bye_req, 200, "OK").await; + tokio::time::timeout(Duration::from_secs(2), bye) + .await + .expect("bye did not complete") + .expect("bye task panicked")?; + assert!(dialog.state().is_terminated()); + + token.cancel(); + Ok(()) +} + +#[tokio::test] +async fn test_reinvite_provisional_keeps_dialog_confirmed() -> crate::Result<()> { + assert_provisional_keeps_confirmed(Method::Invite).await +} + +#[tokio::test] +async fn test_update_provisional_keeps_dialog_confirmed() -> crate::Result<()> { + assert_provisional_keeps_confirmed(Method::Update).await +} From aa5e0e1e69cec7fd66673e35ee05b5be2c8279fe Mon Sep 17 00:00:00 2001 From: Tyson George Date: Thu, 24 Sep 2026 08:53:46 -0400 Subject: [PATCH 02/16] fix: notify dialog state only for transitions that are applied DialogInner::transition sent the new state to subscribers before deciding whether to apply it, so ignored transitions were still notified: a second Terminated when two teardown paths race (a local BYE completing while the peer's BYE is handled), and a WaitAck on an already confirmed dialog. The late-update check added in 0.7.0 reads the state under a lock it then releases, so a concurrent transition can still slip a notification past it. Decide under the state lock, update the state, then send the notification while still holding the lock, so every notification describes a transition that happened and notifications follow the order of state changes. Updates arriving after Terminated, including the event-only variants (Updated/Notify/Info/Options/Refer), stay dropped as in 0.7.0; while the dialog is live, event-only variants are sent as before. Adds an end-to-end test over UDP through DialogLayer: the peer sends an INFO, then a BYE before the application answers it; once the INFO is answered, no Confirmed may follow the Terminated notification. --- src/dialog/dialog.rs | 43 ++- src/dialog/tests/mod.rs | 1 + src/dialog/tests/test_dialog_states.rs | 270 ++++++++++++++++++ .../tests/test_state_after_terminated.rs | 184 ++++++++++++ 4 files changed, 472 insertions(+), 26 deletions(-) create mode 100644 src/dialog/tests/test_state_after_terminated.rs diff --git a/src/dialog/dialog.rs b/src/dialog/dialog.rs index 9f62fc2a..91f7553e 100644 --- a/src/dialog/dialog.rs +++ b/src/dialog/dialog.rs @@ -1546,18 +1546,17 @@ impl DialogInner { } } pub(super) fn transition(&self, state: DialogState) -> Result<()> { - // Try to send state update, but don't fail if channel is closed - // Late state updates after termination (e.g. CANCEL's 200 arriving - // after the INVITE's 487) are no longer broadcast and do not change - // the lifecycle state — observers already saw Terminated. - let terminated_now = matches!(&*self.state.lock(), DialogState::Terminated(..)); - if terminated_now && !matches!(state, DialogState::Terminated(..)) { - debug!(target = ?state, "dialog already terminated, ignoring late transition"); + // Decide under the state lock and notify only transitions that are + // applied, while still holding it, so notifications follow the order + // in which the state changed. + let mut old_state = self.state.lock(); + // Late updates after termination (e.g. CANCEL's 200 arriving after + // the INVITE's 487, or a second Terminated from a racing teardown) + // are neither applied nor broadcast: observers already saw Terminated. + if let DialogState::Terminated(id, _) = &*old_state { + debug!(id = %id, target = %state, "dialog already terminated, ignoring late transition"); return Ok(()); } - - self.state_sender.send(state.clone()).ok(); - // In-dialog request events do not change the established lifecycle state. match state { DialogState::Updated(_, _, _) @@ -1565,28 +1564,20 @@ impl DialogInner { | DialogState::Info(_, _, _) | DialogState::Options(_, _, _) | DialogState::Refer(_, _, _) => { + // Try to send state update, but don't fail if channel is closed + self.state_sender.send(state).ok(); return Ok(()); } _ => {} } - let mut old_state = self.state.lock(); - match (&*old_state, &state) { - (DialogState::Terminated(id, _), _) => { - warn!( - id = %id, - target = %state, - "dialog already terminated, ignoring transition" - ); - return Ok(()); - } - (DialogState::Confirmed(_, _), DialogState::WaitAck(_, _)) => { - warn!(target = %state, "dialog already confirmed, ignoring transition"); - return Ok(()); - } - _ => {} + if let (DialogState::Confirmed(_, _), DialogState::WaitAck(_, _)) = (&*old_state, &state) { + warn!(target = %state, "dialog already confirmed, ignoring transition"); + return Ok(()); } debug!(from = %old_state, to = %state, "transitioning state"); - *old_state = state; + *old_state = state.clone(); + // Try to send state update, but don't fail if channel is closed + self.state_sender.send(state).ok(); Ok(()) } diff --git a/src/dialog/tests/mod.rs b/src/dialog/tests/mod.rs index 52a59d7a..56e56a23 100644 --- a/src/dialog/tests/mod.rs +++ b/src/dialog/tests/mod.rs @@ -12,5 +12,6 @@ mod test_proxy_headers; mod test_refer; mod test_server_dialog; mod test_session_id; +mod test_state_after_terminated; mod test_sub_pub; mod test_uas_ack_timeout; diff --git a/src/dialog/tests/test_dialog_states.rs b/src/dialog/tests/test_dialog_states.rs index ad5a90b0..f41c639d 100644 --- a/src/dialog/tests/test_dialog_states.rs +++ b/src/dialog/tests/test_dialog_states.rs @@ -509,3 +509,273 @@ async fn test_dialog_id_creation() -> crate::Result<()> { Ok(()) } + +/// Name of a state variant, for asserting on notification sequences. +fn state_name(state: &DialogState) -> &'static str { + match state { + DialogState::Calling(_) => "Calling", + DialogState::Trying(_) => "Trying", + DialogState::Early(_, _) => "Early", + DialogState::WaitAck(_, _) => "WaitAck", + DialogState::Confirmed(_, _) => "Confirmed", + DialogState::Updated(_, _, _) => "Updated", + DialogState::Publish(_, _, _) => "Publish", + DialogState::Notify(_, _, _) => "Notify", + DialogState::Info(_, _, _) => "Info", + DialogState::Options(_, _, _) => "Options", + DialogState::Refer(_, _, _) => "Refer", + DialogState::Message(_, _, _) => "Message", + DialogState::Terminated(_, _) => "Terminated", + } +} + +/// Drain every notification currently queued on a state receiver. +fn drain_states( + receiver: &mut tokio::sync::mpsc::UnboundedReceiver, +) -> Vec<&'static str> { + let mut names = Vec::new(); + while let Ok(state) = receiver.try_recv() { + names.push(state_name(&state)); + } + names +} + +#[tokio::test] +async fn test_dialog_lifecycle_notifies_each_state_once() -> crate::Result<()> { + let endpoint = create_test_endpoint().await?; + let (state_sender, mut state_receiver) = unbounded_channel(); + let dialog_id = DialogId { + call_id: "test-call-id-lifecycle".to_string(), + local_tag: "alice-tag-456".to_string(), + remote_tag: "bob-tag-789".to_string(), + }; + let invite_req = create_invite_request("alice-tag-456", "", "test-call-id-lifecycle"); + let (tu_sender, _tu_receiver) = unbounded_channel(); + let dialog_inner = DialogInner::new( + TransactionRole::Client, + dialog_id.clone(), + invite_req, + endpoint.inner.clone(), + state_sender, + None, + Some(crate::sip::Uri::try_from( + "sip:alice@alice.example.com:5060", + )?), + tu_sender, + )?; + + dialog_inner.transition(DialogState::Calling(dialog_id.clone()))?; + dialog_inner.transition(DialogState::Trying(dialog_id.clone()))?; + let ringing = create_response( + StatusCode::Ringing, + "alice-tag-456", + "bob-tag-789", + "test-call-id-lifecycle", + ); + dialog_inner.transition(DialogState::Early(dialog_id.clone(), ringing))?; + let ok = create_response( + StatusCode::OK, + "alice-tag-456", + "bob-tag-789", + "test-call-id-lifecycle", + ); + dialog_inner.transition(DialogState::Confirmed(dialog_id.clone(), ok))?; + dialog_inner.transition(DialogState::Terminated( + dialog_id.clone(), + TerminatedReason::UacBye, + ))?; + + assert_eq!( + drain_states(&mut state_receiver), + vec!["Calling", "Trying", "Early", "Confirmed", "Terminated"] + ); + Ok(()) +} + +#[tokio::test] +async fn test_no_state_notification_after_terminated() -> crate::Result<()> { + let endpoint = create_test_endpoint().await?; + let (state_sender, mut state_receiver) = unbounded_channel(); + let dialog_id = DialogId { + call_id: "test-call-id-double-term".to_string(), + local_tag: "alice-tag-456".to_string(), + remote_tag: "bob-tag-789".to_string(), + }; + let invite_req = + create_invite_request("alice-tag-456", "bob-tag-789", "test-call-id-double-term"); + let (tu_sender, _tu_receiver) = unbounded_channel(); + let dialog_inner = DialogInner::new( + TransactionRole::Client, + dialog_id.clone(), + invite_req, + endpoint.inner.clone(), + state_sender, + None, + Some(crate::sip::Uri::try_from( + "sip:alice@alice.example.com:5060", + )?), + tu_sender, + )?; + + dialog_inner.transition(DialogState::Confirmed( + dialog_id.clone(), + Response::default(), + ))?; + // Two teardown paths racing: our BYE completes, and the peer's BYE is + // handled as well. + dialog_inner.transition(DialogState::Terminated( + dialog_id.clone(), + TerminatedReason::UacBye, + ))?; + dialog_inner.transition(DialogState::Terminated( + dialog_id.clone(), + TerminatedReason::UasBye, + ))?; + // A late state change after termination must not be applied or notified. + dialog_inner.transition(DialogState::Confirmed( + dialog_id.clone(), + Response::default(), + ))?; + + assert_eq!( + drain_states(&mut state_receiver), + vec!["Confirmed", "Terminated"], + "subscribers must see exactly one Terminated and nothing after it" + ); + assert!(matches!( + &*dialog_inner.state.lock(), + DialogState::Terminated(_, TerminatedReason::UacBye) + )); + Ok(()) +} + +#[tokio::test] +async fn test_ignored_waitack_after_confirmed_is_not_notified() -> crate::Result<()> { + let endpoint = create_test_endpoint().await?; + let (state_sender, mut state_receiver) = unbounded_channel(); + let dialog_id = DialogId { + call_id: "test-call-id-waitack".to_string(), + local_tag: "bob-tag-789".to_string(), + remote_tag: "alice-tag-456".to_string(), + }; + let invite_req = create_invite_request("alice-tag-456", "", "test-call-id-waitack"); + let (tu_sender, _tu_receiver) = unbounded_channel(); + let dialog_inner = DialogInner::new( + TransactionRole::Server, + dialog_id.clone(), + invite_req, + endpoint.inner.clone(), + state_sender, + None, + None, + tu_sender, + )?; + + dialog_inner.transition(DialogState::Confirmed( + dialog_id.clone(), + Response::default(), + ))?; + // e.g. a second accept() after the ACK already confirmed the dialog. + dialog_inner.transition(DialogState::WaitAck(dialog_id.clone(), Response::default()))?; + + assert_eq!(drain_states(&mut state_receiver), vec!["Confirmed"]); + assert!(dialog_inner.is_confirmed()); + Ok(()) +} + +#[tokio::test] +async fn test_info_answered_after_bye_does_not_notify_confirmed() -> crate::Result<()> { + use crate::dialog::server_dialog::ServerInviteDialog; + use crate::transaction::{key::TransactionKey, transaction::Transaction}; + use std::sync::Arc; + + let endpoint = create_test_endpoint().await?; + let (state_sender, mut state_receiver) = unbounded_channel(); + let dialog_id = DialogId { + call_id: "test-call-id-info-after-bye".to_string(), + local_tag: "bob-tag-789".to_string(), + remote_tag: "alice-tag-456".to_string(), + }; + let invite_req = create_invite_request("alice-tag-456", "", "test-call-id-info-after-bye"); + let (tu_sender, _tu_receiver) = unbounded_channel(); + let dialog_inner = DialogInner::new( + TransactionRole::Server, + dialog_id.clone(), + invite_req, + endpoint.inner.clone(), + state_sender, + None, + None, + tu_sender, + )?; + dialog_inner.transition(DialogState::Confirmed( + dialog_id.clone(), + Response::default(), + ))?; + let dialog = ServerInviteDialog { + inner: Arc::new(dialog_inner), + }; + + let in_dialog_request = |method: crate::sip::Method, cseq: &str, branch: &str| Request { + method, + uri: crate::sip::Uri::try_from("sip:bob@127.0.0.1:5060").unwrap(), + headers: vec![ + Via::new(format!("SIP/2.0/UDP 127.0.0.1:5060;branch={}", branch)).into(), + CSeq::new(cseq).into(), + From::new("Alice ;tag=alice-tag-456").into(), + To::new("Bob ;tag=bob-tag-789").into(), + CallId::new("test-call-id-info-after-bye").into(), + MaxForwards::new("70").into(), + ] + .into(), + version: crate::sip::Version::V2, + body: vec![], + }; + let server_tx = |req: Request| -> crate::Result { + let key = TransactionKey::from_request(&req, TransactionRole::Server)?; + Ok(Transaction::new_server( + key, + req, + endpoint.inner.clone(), + None, + )) + }; + + // An INFO arrives while confirmed; the application holds its handle. + let mut info_tx = server_tx(in_dialog_request( + crate::sip::Method::Info, + "2 INFO", + "z9hG4bK-info", + ))?; + let mut info_dialog = dialog.clone(); + let info_task = tokio::spawn(async move { info_dialog.handle(&mut info_tx).await }); + + assert_eq!(drain_states(&mut state_receiver), vec!["Confirmed"]); + let handle = match state_receiver.recv().await { + Some(DialogState::Info(_, _, handle)) => handle, + other => panic!("expected Info, got {:?}", other.as_ref().map(state_name)), + }; + + // Before the INFO is answered, the peer hangs up. + let mut bye_tx = server_tx(in_dialog_request( + crate::sip::Method::Bye, + "3 BYE", + "z9hG4bK-bye", + ))?; + dialog.clone().handle(&mut bye_tx).await.ok(); + + // The application answers the INFO after the dialog has terminated. + handle.reply(StatusCode::OK).await.ok(); + info_task.await.expect("info task panicked").ok(); + + assert_eq!( + drain_states(&mut state_receiver), + vec!["Terminated"], + "no Confirmed may be notified after Terminated" + ); + assert!(matches!( + &*dialog.inner.state.lock(), + DialogState::Terminated(_, TerminatedReason::UacBye) + )); + Ok(()) +} diff --git a/src/dialog/tests/test_state_after_terminated.rs b/src/dialog/tests/test_state_after_terminated.rs new file mode 100644 index 00000000..70373b63 --- /dev/null +++ b/src/dialog/tests/test_state_after_terminated.rs @@ -0,0 +1,184 @@ +//! Once a dialog has terminated (RFC 3261 §15: a BYE ends the dialog), the +//! state channel must not report it as anything else. A mid-dialog request +//! that the application answers after the peer's BYE must not produce a +//! `Confirmed` notification after `Terminated`. +use crate::dialog::{ + dialog::{Dialog, DialogState, DialogStateReceiver}, + dialog_layer::DialogLayer, + invite_dialog::InviteDialog, +}; +use crate::sip::{prelude::HeadersExt, Method, Response, SipMessage, StatusCode}; +use crate::transport::{udp::UdpConnection, TransportLayer}; +use crate::EndpointBuilder; +use std::net::SocketAddr; +use std::sync::Arc; +use std::time::Duration; +use tokio::net::UdpSocket; +use tokio::sync::mpsc::{unbounded_channel, UnboundedReceiver}; +use tokio_util::sync::CancellationToken; + +const CALL_ID: &str = "state-after-terminated-test"; +const FROM_TAG: &str = "uac-tag"; + +/// A raw UAC peer. +struct Peer { + socket: UdpSocket, + uas: SocketAddr, +} + +impl Peer { + async fn send_request(&self, method: Method, cseq: u32, to_tag: Option<&str>) { + let addr = self.socket.local_addr().unwrap(); + let to = match to_tag { + Some(tag) => format!(";tag={tag}", self.uas), + None => format!("", self.uas), + }; + let msg = format!( + "{method} sip:bob@{uas} SIP/2.0\r\n\ + Via: SIP/2.0/UDP {addr};branch=z9hG4bK-{method}-{cseq}\r\n\ + Max-Forwards: 70\r\n\ + From: ;tag={FROM_TAG}\r\n\ + To: {to}\r\n\ + Call-ID: {CALL_ID}\r\n\ + CSeq: {cseq} {method}\r\n\ + Contact: \r\n\ + Content-Length: 0\r\n\r\n", + uas = self.uas, + ); + self.socket.send_to(msg.as_bytes(), self.uas).await.unwrap(); + } + + /// Wait for the final response to the request with `cseq`. + async fn recv_final(&self, cseq: u32) -> Response { + let mut buf = vec![0u8; 4096]; + loop { + let (len, _) = + tokio::time::timeout(Duration::from_secs(2), self.socket.recv_from(&mut buf)) + .await + .expect("timeout waiting for a final response") + .unwrap(); + let text = std::str::from_utf8(&buf[..len]).unwrap(); + if let Ok(SipMessage::Response(resp)) = SipMessage::try_from(text) { + let seq = resp.cseq_header().unwrap().seq().unwrap(); + if seq == cseq && resp.status_code.code() >= 200 { + return resp; + } + } + } + } +} + +/// A UAS endpoint running the usual incoming-transaction loop. +async fn setup( + token: &CancellationToken, +) -> crate::Result<(DialogStateReceiver, UnboundedReceiver, Peer)> { + let transport_layer = TransportLayer::new(token.child_token()); + let udp = UdpConnection::create_connection( + "127.0.0.1:0".parse().unwrap(), + None, + Some(token.child_token()), + ) + .await?; + let uas: SocketAddr = udp.get_addr().get_socketaddr()?; + transport_layer.add_transport(udp.into()); + let endpoint = EndpointBuilder::new() + .with_user_agent("rsipstack-test") + .with_transport_layer(transport_layer) + .with_cancel_token(token.child_token()) + .build(); + let dialog_layer = Arc::new(DialogLayer::new(endpoint.inner.clone())); + let mut incoming = endpoint.incoming_transactions()?; + let endpoint_inner = endpoint.inner.clone(); + tokio::spawn(async move { + let _ = endpoint_inner.serve().await; + }); + + let (state_sender, states) = unbounded_channel(); + let (dialog_sender, dialogs) = unbounded_channel(); + tokio::spawn(async move { + while let Some(mut tx) = incoming.recv().await { + let has_to_tag = tx.original.to_header().unwrap().tag().unwrap().is_some(); + let dialog = if has_to_tag { + dialog_layer.match_dialog(&tx) + } else if tx.original.method == Method::Invite { + let dialog = dialog_layer + .get_or_create_server_invite(&tx, state_sender.clone(), None, None) + .expect("server dialog"); + dialog_sender.send(dialog.clone()).unwrap(); + Some(Dialog::Invite(dialog)) + } else { + None + }; + if let Some(mut dialog) = dialog { + tokio::spawn(async move { + let _ = dialog.handle(&mut tx).await; + }); + } + } + }); + + let peer = Peer { + socket: UdpSocket::bind("127.0.0.1:0").await?, + uas, + }; + Ok((states, dialogs, peer)) +} + +async fn next_state(rx: &mut DialogStateReceiver) -> DialogState { + tokio::time::timeout(Duration::from_secs(2), rx.recv()) + .await + .expect("timeout waiting for dialog state") + .expect("state channel closed") +} + +#[tokio::test] +async fn test_info_answered_after_peer_bye_is_not_notified_as_confirmed() -> crate::Result<()> { + let token = CancellationToken::new(); + let (mut states, mut dialogs, peer) = setup(&token).await?; + + // Establish the call: INVITE, 200, ACK. + peer.send_request(Method::Invite, 1, None).await; + let dialog = tokio::time::timeout(Duration::from_secs(2), dialogs.recv()) + .await + .expect("timeout waiting for the server dialog") + .unwrap(); + dialog.accept(None, None)?; + let ok = peer.recv_final(1).await; + let to_tag = ok.to_header()?.tag()?.expect("To tag").value().to_string(); + peer.send_request(Method::Ack, 1, Some(&to_tag)).await; + while !matches!(next_state(&mut states).await, DialogState::Confirmed(..)) {} + + // An INFO arrives; the application takes a moment to answer it. + peer.send_request(Method::Info, 2, Some(&to_tag)).await; + let info = loop { + if let DialogState::Info(_, _, handle) = next_state(&mut states).await { + break handle; + } + }; + + // Meanwhile the peer hangs up. + peer.send_request(Method::Bye, 3, Some(&to_tag)).await; + assert_eq!(peer.recv_final(3).await.status_code, StatusCode::OK); + assert!( + matches!(next_state(&mut states).await, DialogState::Terminated(..)), + "the peer's BYE must terminate the dialog" + ); + + // Now the application answers the INFO. + info.reply(StatusCode::OK).await.ok(); + assert_eq!(peer.recv_final(2).await.status_code, StatusCode::OK); + tokio::time::sleep(Duration::from_millis(200)).await; + + let mut after = Vec::new(); + while let Ok(state) = states.try_recv() { + after.push(state.to_string()); + } + assert!( + after.is_empty(), + "nothing may be notified after Terminated, got {after:?}" + ); + assert!(dialog.state().is_terminated()); + + token.cancel(); + Ok(()) +} From 5ef8ea6e58956b625d2c9dd9d0fc12781a930723 Mon Sep 17 00:00:00 2001 From: jinti Date: Wed, 7 Oct 2026 09:26:47 +0800 Subject: [PATCH 03/16] fix(dialog): re-surface Confirmed after an in-dialog REFER is answered (RFC 3515) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit handle_refer left the dialog without the closing return_to_confirmed that handle_message/handle_notify have: answering a REFER (usually 202) never re-surfaced a Confirmed event, so TU-side logic waiting on it — e.g. an active-call bot's pending 100 Trying NOTIFY for the implicit refer subscription — starved, and the referrer saw the 202 but no NOTIFY ever. The stored dialog state was always correct (in-dialog request events do not change it); only the notification was missing. Adds a regression test that fails without the fix: answer the REFER, expect a Confirmed event and a working notify_refer. --- Cargo.toml | 2 +- src/dialog/client_dialog.rs | 7 +- src/dialog/invite_dialog.rs | 7 +- src/dialog/tests/mod.rs | 1 + src/dialog/tests/test_refer_notify.rs | 294 ++++++++++++++++++++++++++ 5 files changed, 308 insertions(+), 3 deletions(-) create mode 100644 src/dialog/tests/test_refer_notify.rs diff --git a/Cargo.toml b/Cargo.toml index cf7af89b..a337a4dd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rsipstack" -version = "0.7.1" +version = "0.7.2" edition = "2021" description = "SIP Stack Rust library for building SIP applications" license = "MIT" diff --git a/src/dialog/client_dialog.rs b/src/dialog/client_dialog.rs index f18426da..c4b3fd8c 100644 --- a/src/dialog/client_dialog.rs +++ b/src/dialog/client_dialog.rs @@ -689,7 +689,12 @@ impl ClientInviteDialog { self.inner .transition(DialogState::Refer(self.id(), tx.original.clone(), handle))?; - self.inner.process_transaction_handle(tx, rx).await + // RFC 3515: the REFER was answered (usually 202) and the dialog must + // go back to Confirmed — the implicit subscription's NOTIFYs are + // in-dialog requests that need the confirmed dialog. + let result = self.inner.process_transaction_handle(tx, rx).await; + let confirmed = self.return_to_confirmed(tx); + result.and(confirmed) } async fn handle_message(&mut self, tx: &mut Transaction) -> Result<()> { diff --git a/src/dialog/invite_dialog.rs b/src/dialog/invite_dialog.rs index fb4af377..72a3587b 100644 --- a/src/dialog/invite_dialog.rs +++ b/src/dialog/invite_dialog.rs @@ -810,7 +810,12 @@ impl InviteDialog { self.inner .transition(DialogState::Refer(self.id(), tx.original.clone(), handle))?; - self.inner.process_transaction_handle(tx, rx).await + // RFC 3515: the REFER was answered (usually 202) and the dialog must + // go back to Confirmed — the implicit subscription's NOTIFYs are + // in-dialog requests that need the confirmed dialog. + let result = self.inner.process_transaction_handle(tx, rx).await; + let confirmed = self.return_to_confirmed(tx); + result.and(confirmed) } async fn handle_message(&mut self, tx: &mut Transaction) -> Result<()> { diff --git a/src/dialog/tests/mod.rs b/src/dialog/tests/mod.rs index 52a59d7a..ead0ba64 100644 --- a/src/dialog/tests/mod.rs +++ b/src/dialog/tests/mod.rs @@ -10,6 +10,7 @@ mod test_invite_auth_challenge; mod test_prack; mod test_proxy_headers; mod test_refer; +mod test_refer_notify; mod test_server_dialog; mod test_session_id; mod test_sub_pub; diff --git a/src/dialog/tests/test_refer_notify.rs b/src/dialog/tests/test_refer_notify.rs new file mode 100644 index 00000000..9ce7f455 --- /dev/null +++ b/src/dialog/tests/test_refer_notify.rs @@ -0,0 +1,294 @@ +//! RFC 3515 regression: after an in-dialog REFER is answered (usually 202), +//! the dialog must return to `Confirmed` — the implicit subscription's +//! NOTIFYs are in-dialog requests that need the confirmed dialog. Leaving +//! the dialog in the `Refer` state starved the referrer of every NOTIFY. +use crate::dialog::{ + dialog::{Dialog, DialogState, DialogStateReceiver}, + dialog_layer::DialogLayer, + invite_dialog::InviteDialog, +}; +use crate::sip::{prelude::HeadersExt, Method, SipMessage, StatusCode}; +use crate::transaction::endpoint::EndpointOption; +use crate::transport::udp::UdpConnection; +use crate::transport::TransportLayer; +use crate::EndpointBuilder; +use std::net::SocketAddr; +use std::sync::Arc; +use std::time::{Duration, Instant}; +use tokio::net::UdpSocket; +use tokio::sync::mpsc::{unbounded_channel, UnboundedReceiver}; +use tokio_util::sync::CancellationToken; + +const CALL_ID: &str = "refer-notify-test"; +const FROM_TAG: &str = "referrer-tag"; + +struct Harness { + states: DialogStateReceiver, + dialogs: UnboundedReceiver, + peer: UdpSocket, + uas: SocketAddr, +} + +async fn setup(token: &CancellationToken) -> crate::Result { + let transport_layer = TransportLayer::new(token.child_token()); + let udp = UdpConnection::create_connection( + "127.0.0.1:0".parse().unwrap(), + None, + Some(token.child_token()), + ) + .await?; + let uas: SocketAddr = udp.get_addr().get_socketaddr()?; + transport_layer.add_transport(udp.into()); + let endpoint = EndpointBuilder::new() + .with_user_agent("rsipstack-test") + .with_transport_layer(transport_layer) + .with_cancel_token(token.child_token()) + .with_option(EndpointOption { + t1: Duration::from_millis(20), + t1x64: Duration::from_millis(64 * 20), + ..Default::default() + }) + .build(); + let dialog_layer = Arc::new(DialogLayer::new(endpoint.inner.clone())); + let mut incoming = endpoint.incoming_transactions()?; + let endpoint_inner = endpoint.inner.clone(); + tokio::spawn(async move { + let _ = endpoint_inner.serve().await; + }); + + let (state_sender, states) = unbounded_channel(); + let (dialog_sender, dialogs) = unbounded_channel(); + tokio::spawn(async move { + while let Some(mut tx) = incoming.recv().await { + let has_to_tag = tx.original.to_header().unwrap().tag().unwrap().is_some(); + let dialog = if has_to_tag { + dialog_layer.match_dialog(&tx) + } else if tx.original.method == Method::Invite { + let dialog = dialog_layer + .get_or_create_server_invite(&tx, state_sender.clone(), None, None) + .expect("server dialog"); + dialog_sender.send(dialog.clone()).unwrap(); + Some(Dialog::Invite(dialog)) + } else { + None + }; + if let Some(mut dialog) = dialog { + tokio::spawn(async move { + let _ = dialog.handle(&mut tx).await; + }); + } + } + }); + + let peer = UdpSocket::bind("127.0.0.1:0").await?; + Ok(Harness { + states, + dialogs, + peer, + uas, + }) +} + +async fn recv_message( + socket: &UdpSocket, + buf: &mut [u8], + wait: Duration, +) -> Option<(String, SocketAddr)> { + let deadline = Instant::now() + wait; + loop { + let now = Instant::now(); + if now >= deadline { + return None; + } + let Ok(Ok((len, src))) = tokio::time::timeout(deadline - now, socket.recv_from(buf)).await + else { + return None; + }; + return Some((String::from_utf8_lossy(&buf[..len]).to_string(), src)); + } +} + +/// Confirm a call, then REFER it: the dialog must answer 202, return to +/// Confirmed, and `notify_refer` must deliver the 100 Trying NOTIFY. +#[tokio::test] +async fn test_refer_returns_dialog_to_confirmed_and_notify_works() -> crate::Result<()> { + let token = CancellationToken::new(); + let Harness { + mut states, + mut dialogs, + peer, + uas, + } = setup(&token).await?; + + let peer_addr = peer.local_addr()?; + let invite = format!( + "INVITE sip:bob@{uas} SIP/2.0\r\n\ + Via: SIP/2.0/UDP {peer_addr};branch=z9hG4bK-rn-invite\r\n\ + Max-Forwards: 70\r\n\ + From: ;tag={FROM_TAG}\r\n\ + To: \r\n\ + Call-ID: {CALL_ID}\r\n\ + CSeq: 1 INVITE\r\n\ + Contact: \r\n\ + Content-Length: 0\r\n\r\n", + ); + peer.send_to(invite.as_bytes(), uas).await?; + + let dialog = tokio::time::timeout(Duration::from_secs(2), dialogs.recv()) + .await + .expect("timeout waiting for the server dialog") + .unwrap(); + dialog.accept(None, None)?; + + // First 200 OK, remember its To tag. + let mut buf = vec![0u8; 4096]; + let to_tag = loop { + let (text, _) = recv_message(&peer, &mut buf, Duration::from_secs(2)) + .await + .expect("timeout waiting for the first 200"); + if let Ok(SipMessage::Response(resp)) = SipMessage::try_from(text.as_str()) { + if resp.status_code.code() == 200 { + break resp + .to_header()? + .tag()? + .expect("the 200 must carry the local tag") + .value() + .to_string(); + } + } + }; + // Confirm the dialog. + let ack = format!( + "ACK sip:alice@{peer_addr} SIP/2.0\r\n\ + Via: SIP/2.0/UDP {peer_addr};branch=z9hG4bK-rn-ack\r\n\ + Max-Forwards: 70\r\n\ + From: ;tag={FROM_TAG}\r\n\ + To: ;tag={to_tag}\r\n\ + Call-ID: {CALL_ID}\r\n\ + CSeq: 1 ACK\r\n\ + Content-Length: 0\r\n\r\n", + ); + peer.send_to(ack.as_bytes(), uas).await?; + + // In-dialog REFER transferring the call elsewhere. + let refer = format!( + "REFER sip:bob@{uas} SIP/2.0\r\n\ + Via: SIP/2.0/UDP {peer_addr};branch=z9hG4bK-rn-refer\r\n\ + Max-Forwards: 70\r\n\ + From: ;tag={FROM_TAG}\r\n\ + To: ;tag={to_tag}\r\n\ + Call-ID: {CALL_ID}\r\n\ + CSeq: 2 REFER\r\n\ + Refer-To: \r\n\ + Contact: \r\n\ + Content-Length: 0\r\n\r\n", + ); + peer.send_to(refer.as_bytes(), uas).await?; + + // The application answers the REFER with 202 (as an RFC 3515 referee). + let mut refer_handle = None; + let deadline = Instant::now() + Duration::from_secs(2); + while Instant::now() < deadline { + while let Ok(state) = states.try_recv() { + if let DialogState::Refer(_, _, handle) = state { + refer_handle = Some(handle); + } + } + if refer_handle.is_some() { + break; + } + tokio::time::sleep(Duration::from_millis(5)).await; + } + let refer_handle = refer_handle.expect("the dialog never surfaced the REFER"); + refer_handle + .reply(StatusCode::Other(202, "Accepted".into())) + .await + .ok(); + + // The referrer sees the 202 ... + let saw_202 = loop { + let (text, _) = recv_message(&peer, &mut buf, Duration::from_secs(2)) + .await + .expect("timeout waiting for the 202"); + if text.starts_with("SIP/2.0 202") { + break true; + } + }; + assert!(saw_202); + + // ... and the dialog re-surfaces a Confirmed event: in-dialog request + // events do not change the stored state, but TU-side logic (e.g. a + // pending refer NOTIFY) waits on the Confirmed event after answering. + // Without `return_to_confirmed` in handle_refer it never arrives. + let reconfirmed = { + let deadline = Instant::now() + Duration::from_secs(2); + let mut seen = false; + while Instant::now() < deadline { + while let Ok(state) = states.try_recv() { + if matches!(state, DialogState::Confirmed(..)) { + seen = true; + } + } + if seen { + break; + } + tokio::time::sleep(Duration::from_millis(5)).await; + } + seen + }; + assert!( + reconfirmed, + "answering the REFER must re-surface a Confirmed event for the dialog" + ); + assert!(dialog.state().is_confirmed()); + + // The 100 Trying NOTIFY for the implicit subscription goes out. + let notify = dialog.notify_refer(StatusCode::Trying, "active").await; + assert!( + notify.is_ok(), + "notify_refer must work once the dialog is Confirmed again: {:?}", + notify.err() + ); + let saw_notify = loop { + let (text, src) = recv_message(&peer, &mut buf, Duration::from_secs(2)) + .await + .expect("timeout waiting for the NOTIFY"); + if text.starts_with("NOTIFY") { + assert!( + text.contains("Event: refer"), + "the NOTIFY must carry Event: refer, got {text}" + ); + assert!( + text.contains("Subscription-State: active"), + "the NOTIFY must carry Subscription-State: active, got {text}" + ); + assert!( + text.contains("SIP/2.0 100"), + "the NOTIFY body must carry the 100 Trying sipfrag, got {text}" + ); + // Answer the NOTIFY so the transaction completes. + let header = |name: &str| { + text.lines() + .find(|l| l.starts_with(name)) + .unwrap() + .strip_prefix(&format!("{name} ")) + .unwrap() + .to_string() + }; + let resp = format!( + "SIP/2.0 200 OK\r\nVia: {}\r\nFrom: {}\r\nTo: {}\r\nCall-ID: {}\r\nCSeq: {}\r\nContent-Length: 0\r\n\r\n", + header("Via:"), + header("From:"), + header("To:"), + header("Call-ID:"), + header("CSeq:"), + ); + peer.send_to(resp.as_bytes(), src).await?; + break true; + } + }; + assert!(saw_notify); + + token.cancel(); + Ok(()) +} From 7d9060d98d95ad1784025bccd599db3f0296c01a Mon Sep 17 00:00:00 2001 From: Tyson George Date: Wed, 7 Oct 2026 01:12:30 -0400 Subject: [PATCH 04/16] fix(transaction): keep a server INVITE's final response after the ACK ends it Since #169 the matching ACK terminates an Accepted server INVITE transaction, and cleanup() took last_response before the dialog built DialogState::Confirmed from it. Confirmed carried Response::default() (no CSeq) for the initial INVITE and every re-INVITE, so a TU could not tell which INVITE it confirms. The server INVITE transaction now keeps last_response and hands a copy to finished_transactions. --- src/dialog/tests/test_uas_ack_timeout.rs | 17 ++++++++++++----- src/transaction/transaction.rs | 8 +++++++- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/dialog/tests/test_uas_ack_timeout.rs b/src/dialog/tests/test_uas_ack_timeout.rs index a3bf069a..23b43daf 100644 --- a/src/dialog/tests/test_uas_ack_timeout.rs +++ b/src/dialog/tests/test_uas_ack_timeout.rs @@ -514,14 +514,18 @@ async fn test_unacked_reinvite_2xx_ends_the_session() -> crate::Result<()> { Ok(()) } -async fn wait_confirmed(states: &mut DialogStateReceiver) { +/// Wait for `Confirmed` and return the CSeq number of the 2xx it carries. +async fn wait_confirmed(states: &mut DialogStateReceiver) -> u32 { loop { let state = tokio::time::timeout(Duration::from_secs(2), states.recv()) .await .expect("timeout waiting for the call to be confirmed") .expect("state channel closed"); - if matches!(state, DialogState::Confirmed(..)) { - return; + if let DialogState::Confirmed(_, resp) = state { + let cseq = resp.cseq_header().expect("Confirmed carries the 2xx"); + assert_eq!(cseq.method().unwrap(), Method::Invite); + assert_eq!(resp.status_code.code(), 200); + return cseq.seq().unwrap(); } } } @@ -652,7 +656,7 @@ async fn test_2xx_over_tcp_is_retransmitted_until_the_ack() -> crate::Result<()> .write_all(request(Method::Ack, 1, Some(&to_tag)).as_bytes()) .await?; let acked = Instant::now(); - wait_confirmed(&mut states).await; + assert_eq!(wait_confirmed(&mut states).await, 1); let after_ack = read_tcp_messages(&mut stream, &mut buf, acked + T1 * 30).await; assert!( !after_ack @@ -790,7 +794,7 @@ async fn test_late_ack_of_an_earlier_reinvite_reaches_its_own_transaction() -> c }; let local_tag = ok.to_header()?.tag()?.unwrap().value().to_string(); peer.send_request(Method::Ack, 1, Some(&local_tag)).await; - wait_confirmed(&mut states).await; + assert_eq!(wait_confirmed(&mut states).await, 1); // re-INVITE 2 and 3 are answered; the ACK of 2 arrives after re-INVITE 3. let mut answered = None; @@ -854,6 +858,9 @@ async fn test_late_ack_of_an_earlier_reinvite_reaches_its_own_transaction() -> c )), "an ACKed call must not be ended" ); + // Each ACK confirms its own re-INVITE, in the order the ACKs arrived. + assert_eq!(wait_confirmed(&mut states).await, 2); + assert_eq!(wait_confirmed(&mut states).await, 3); assert!(terminated_reason(&mut states).is_none()); assert!(dialog.state().is_confirmed()); token.cancel(); diff --git a/src/transaction/transaction.rs b/src/transaction/transaction.rs index b9656ea5..96092137 100644 --- a/src/transaction/transaction.rs +++ b/src/transaction/transaction.rs @@ -1612,9 +1612,15 @@ impl Transaction { } self.last_ack.take().map(SipMessage::Request) } - TransactionType::ServerNonInvite | TransactionType::ServerInvite => { + TransactionType::ServerNonInvite => { self.last_response.take().map(SipMessage::Response) } + // Kept: the matching ACK terminates an Accepted server + // INVITE before the dialog reads the 2xx for + // `DialogState::Confirmed`. + TransactionType::ServerInvite => { + self.last_response.clone().map(SipMessage::Response) + } _ => None, } }; From a8d9aa15df380ea6a44594812d863fd61fbbedcd Mon Sep 17 00:00:00 2001 From: Tyson George Date: Wed, 7 Oct 2026 02:34:41 -0400 Subject: [PATCH 05/16] fix(dialog): get_client_dialog_by_call_id returns only UAC dialogs Since the unified `Dialog::Invite(InviteDialog)` (0.6.0) the lookup matches every INVITE dialog with the Call-ID, so a B2BUA that keeps the Call-ID across its legs gets the inbound (UAS) leg back as a client dialog. Check the role, as the documentation says ("client-side INVITE dialogs (UAC)") and as `Dialog::ClientInvite` did in 0.5.x. --- src/dialog/dialog_layer.rs | 5 +++- src/dialog/tests/test_dialog_layer.rs | 41 +++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/dialog/dialog_layer.rs b/src/dialog/dialog_layer.rs index 010d80e9..d77b7c0a 100644 --- a/src/dialog/dialog_layer.rs +++ b/src/dialog/dialog_layer.rs @@ -453,7 +453,10 @@ impl DialogLayer { self.inner.dialogs.with(|m| { m.values() .filter_map(|d| match d { - Dialog::Invite(client_dlg) if client_dlg.id().call_id == call_id => { + Dialog::Invite(client_dlg) + if client_dlg.role() == TransactionRole::Client + && client_dlg.id().call_id == call_id => + { Some(client_dlg.clone()) } _ => None, diff --git a/src/dialog/tests/test_dialog_layer.rs b/src/dialog/tests/test_dialog_layer.rs index b1b15adf..cec219bf 100644 --- a/src/dialog/tests/test_dialog_layer.rs +++ b/src/dialog/tests/test_dialog_layer.rs @@ -374,6 +374,47 @@ async fn test_multiple_dialogs_management() -> crate::Result<()> { Ok(()) } +/// A B2BUA that keeps the Call-ID has both legs of a call in one dialog layer: +/// the inbound one as UAS, the outbound one as UAC. Only the UAC one is a +/// client dialog. +#[tokio::test] +async fn test_get_client_dialog_by_call_id_returns_only_uac_dialogs() -> crate::Result<()> { + let endpoint = create_test_endpoint().await?; + let conn = create_mock_connection().await?; + endpoint.inner.transport_layer.add_transport(conn.clone()); + let dialog_layer = std::sync::Arc::new(DialogLayer::new(endpoint.inner.clone())); + let call_id = "b2bua-call-id"; + + let invite_req = create_invite_request("caller-tag", "", call_id, "z9hG4bKinbound"); + let key = TransactionKey::from_request(&invite_req, TransactionRole::Server)?; + let tx = Transaction::new_server(key, invite_req, endpoint.inner.clone(), Some(conn)); + let (state_sender, _) = unbounded_channel(); + let uas = dialog_layer.get_or_create_server_invite(&tx, state_sender, None, None)?; + + let peer = tokio::net::UdpSocket::bind("127.0.0.1:0").await?; + let callee = format!("sip:carol@{}", peer.local_addr()?); + let (state_sender, _) = unbounded_channel(); + let (uac, _invite) = dialog_layer.do_invite_async( + crate::dialog::invitation::InviteOption { + caller: crate::sip::Uri::try_from("sip:alice@example.com")?, + callee: crate::sip::Uri::try_from(callee.as_str())?, + contact: crate::sip::Uri::try_from("sip:alice@127.0.0.1:5060")?, + call_id: Some(call_id.to_string()), + ..Default::default() + }, + state_sender, + )?; + + let found: Vec<_> = dialog_layer + .get_client_dialog_by_call_id(call_id) + .iter() + .map(|d| (d.role(), d.id())) + .collect(); + assert_eq!(found, vec![(TransactionRole::Client, uac.id())]); + assert!(dialog_layer.get_dialog(&uas.id()).is_some()); + Ok(()) +} + #[tokio::test] async fn test_dialog_error_cases() -> crate::Result<()> { let endpoint = create_test_endpoint().await?; From 964ca662a93f30aebf2ad101cfd297f64599d769 Mon Sep 17 00:00:00 2001 From: Tyson George Date: Wed, 7 Oct 2026 04:58:54 -0400 Subject: [PATCH 06/16] fix: keep whole SIP messages out of WARN logs Some WARN records carried a whole SIP message, with the From/To/Contact URIs, display names and any body: - the dialog layer's "failed to send request" (send_prack_request and send_dialog_request) printed the full request; - the WebSocket transport's "Error parsing SIP message" printed the raw text frame; - the "bye skipped" WARN of ClientInviteDialog, InviteDialog and ServerInviteDialog printed the dialog state with Debug, which for Early, WaitAck and Confirmed includes the whole response; the returned error did the same. The send failure now logs the method at WARN and the request at DEBUG. The WebSocket parse failure logs the frame length; the frame is already logged at INFO when it is received. The BYE paths use the Display of DialogState (dialog id and state name). --- src/dialog/client_dialog.rs | 4 +- src/dialog/dialog.rs | 10 +- src/dialog/invite_dialog.rs | 4 +- src/dialog/server_dialog.rs | 4 +- src/dialog/tests/mod.rs | 1 + src/dialog/tests/test_warn_logs.rs | 195 +++++++++++++++++++++++++++++ src/transport/websocket.rs | 2 +- 7 files changed, 209 insertions(+), 11 deletions(-) create mode 100644 src/dialog/tests/test_warn_logs.rs diff --git a/src/dialog/client_dialog.rs b/src/dialog/client_dialog.rs index c4b3fd8c..1582466c 100644 --- a/src/dialog/client_dialog.rs +++ b/src/dialog/client_dialog.rs @@ -162,11 +162,11 @@ impl ClientInviteDialog { if !self.inner.is_terminated() { warn!( dialog_id = %self.id(), - state = ?self.state(), + state = %self.state(), "bye skipped: dialog not confirmed" ); return Err(crate::Error::Error(format!( - "dialog {} cannot send BYE in state {:?}", + "dialog {} cannot send BYE in state {}", self.id(), self.state() ))); diff --git a/src/dialog/dialog.rs b/src/dialog/dialog.rs index 9f62fc2a..611e95fd 100644 --- a/src/dialog/dialog.rs +++ b/src/dialog/dialog.rs @@ -707,10 +707,11 @@ impl DialogInner { warn!( id = self.id.lock().to_string(), destination = tx.destination.as_ref().map(|d| d.to_string()).as_deref(), - "failed to send request error: {}\n{}", - e, - tx.original + method = %method, + "failed to send request error: {}", + e ); + debug!(id = self.id.lock().to_string(), req = %tx.original, "request that failed to send"); return Err(e); } } @@ -1153,10 +1154,11 @@ impl DialogInner { warn!( id = self.id.lock().to_string(), destination = tx.destination.as_ref().map(|d| d.to_string()).as_deref(), - req = %tx.original, + method = %method, "failed to send request error: {}", e ); + debug!(id = self.id.lock().to_string(), req = %tx.original, "request that failed to send"); return Err(e); } } diff --git a/src/dialog/invite_dialog.rs b/src/dialog/invite_dialog.rs index 72a3587b..4cd6ea2e 100644 --- a/src/dialog/invite_dialog.rs +++ b/src/dialog/invite_dialog.rs @@ -429,11 +429,11 @@ impl InviteDialog { if !self.inner.is_terminated() { warn!( dialog_id = %self.id(), - state = ?self.state(), + state = %self.state(), "bye skipped: dialog not confirmed or waiting ack" ); return Err(crate::Error::Error(format!( - "dialog {} cannot send BYE in state {:?}", + "dialog {} cannot send BYE in state {}", self.id(), self.state() ))); diff --git a/src/dialog/server_dialog.rs b/src/dialog/server_dialog.rs index 3456600f..e3ace560 100644 --- a/src/dialog/server_dialog.rs +++ b/src/dialog/server_dialog.rs @@ -356,11 +356,11 @@ impl ServerInviteDialog { if !self.inner.is_terminated() { warn!( dialog_id = %self.id(), - state = ?self.state(), + state = %self.state(), "bye skipped: dialog not confirmed or waiting ack" ); return Err(crate::Error::Error(format!( - "dialog {} cannot send BYE in state {:?}", + "dialog {} cannot send BYE in state {}", self.id(), self.state() ))); diff --git a/src/dialog/tests/mod.rs b/src/dialog/tests/mod.rs index ead0ba64..bb4036dc 100644 --- a/src/dialog/tests/mod.rs +++ b/src/dialog/tests/mod.rs @@ -15,3 +15,4 @@ mod test_server_dialog; mod test_session_id; mod test_sub_pub; mod test_uas_ack_timeout; +mod test_warn_logs; diff --git a/src/dialog/tests/test_warn_logs.rs b/src/dialog/tests/test_warn_logs.rs new file mode 100644 index 00000000..ecd4c6f4 --- /dev/null +++ b/src/dialog/tests/test_warn_logs.rs @@ -0,0 +1,195 @@ +//! WARN records carry metadata only; the whole SIP message is logged below WARN. +use crate::dialog::dialog::{DialogInner, DialogState}; +use crate::dialog::{client_dialog::ClientInviteDialog, invite_dialog::InviteDialog}; +use crate::dialog::{server_dialog::ServerInviteDialog, DialogId}; +use crate::sip::{Request, SipMessage, Uri}; +use crate::transaction::{endpoint::TargetLocator, key::TransactionRole}; +use crate::transport::{SipAddr, TransportLayer}; +use std::sync::{Arc, Mutex}; +use tokio::sync::mpsc::unbounded_channel; +use tracing::{field::Field, span, Event, Level, Metadata, Subscriber}; + +const MARKER: &str = "private-marker"; + +/// Records every event as (level, " field=value ..."). +#[derive(Clone, Default)] +struct Capture(Arc>>); + +impl Subscriber for Capture { + fn enabled(&self, _: &Metadata<'_>) -> bool { + true + } + fn new_span(&self, _: &span::Attributes<'_>) -> span::Id { + span::Id::from_u64(1) + } + fn record(&self, _: &span::Id, _: &span::Record<'_>) {} + fn record_follows_from(&self, _: &span::Id, _: &span::Id) {} + fn event(&self, event: &Event<'_>) { + let mut line = String::new(); + event.record(&mut |f: &Field, v: &dyn std::fmt::Debug| { + line.push_str(&format!(" {}={:?}", f.name(), v)) + }); + self.0 + .lock() + .unwrap() + .push((*event.metadata().level(), line)); + } + fn enter(&self, _: &span::Id) {} + fn exit(&self, _: &span::Id) {} +} + +impl Capture { + fn lines(&self, level: Level, needle: &str) -> Vec { + let records = self.0.lock().unwrap(); + let hits = records + .iter() + .filter(|(l, s)| *l == level && s.contains(needle)); + hits.map(|(_, s)| s.clone()).collect() + } +} + +/// Fails every lookup, so `Transaction::send` returns an error. +struct NoRoute; + +#[async_trait::async_trait] +impl TargetLocator for NoRoute { + async fn locate(&self, _: &Uri) -> crate::Result { + Err(crate::Error::Error("no route".to_string())) + } +} + +/// A request or response whose From carries `MARKER`. +fn message(start_line: &str, cseq: &str) -> SipMessage { + let text = format!( + "{start_line}\r\nVia: SIP/2.0/UDP 127.0.0.1;branch=z9hG4bKwarn\r\nCSeq: {cseq}\r\n\ + From: \"{MARKER}\" ;tag=a\r\n\ + To: ;tag=b\r\nCall-ID: warn-call\r\n\r\n" + ); + SipMessage::try_from(text.as_str()).unwrap() +} + +fn request(method: &str, cseq: u32) -> Request { + let start_line = format!("{method} sip:bob@127.0.0.1:5999 SIP/2.0"); + match message(&start_line, &format!("{cseq} {method}")) { + SipMessage::Request(req) => req, + other => panic!("{other:?}"), + } +} + +/// A UAC dialog on an endpoint whose every request send fails. +fn dialog() -> crate::Result> { + let endpoint = crate::EndpointBuilder::new() + .with_transport_layer(TransportLayer::new(Default::default())) + .with_target_locator(Box::new(NoRoute)) + .build(); + let id = DialogId { + call_id: "warn-call".to_string(), + local_tag: "a".to_string(), + remote_tag: "b".to_string(), + }; + let (state_tx, _) = unbounded_channel(); + let (tu_tx, _) = unbounded_channel(); + let invite = request("INVITE", 1); + let inner = DialogInner::new( + TransactionRole::Client, + id, + invite, + endpoint.inner.clone(), + state_tx, + None, + None, + tu_tx, + )?; + Ok(Arc::new(inner)) +} + +#[tokio::test] +async fn test_failed_request_send_warns_without_the_request() -> crate::Result<()> { + let capture = Capture::default(); + let _guard = tracing::subscriber::set_default(capture.clone()); + let inner = dialog()?; + + // send_dialog_request, then send_prack_request. + assert!(inner.do_request(request("INFO", 2)).await.is_err()); + assert!(inner.send_prack_request(request("PRACK", 3)).await.is_err()); + + let warns = capture.lines(Level::WARN, "failed to send request"); + assert_eq!(warns.len(), 2, "{warns:?}"); + assert!(warns.iter().all(|w| !w.contains(MARKER)), "{warns:?}"); + assert!(warns[0].contains("method=INFO") && warns[1].contains("method=PRACK")); + let debugs = capture.lines(Level::DEBUG, "request that failed to send"); + assert_eq!(debugs.len(), 2, "{debugs:?}"); + assert!(debugs.iter().all(|d| d.contains(MARKER)), "{debugs:?}"); + Ok(()) +} + +#[tokio::test] +async fn test_bye_in_early_state_warns_without_the_response() -> crate::Result<()> { + let capture = Capture::default(); + let _guard = tracing::subscriber::set_default(capture.clone()); + let inner = dialog()?; + let SipMessage::Response(ringing) = message("SIP/2.0 180 Ringing", "1 INVITE") else { + unreachable!() + }; + inner.transition(DialogState::Early(inner.id.lock().clone(), ringing))?; + + let errors = [ + ClientInviteDialog { + inner: inner.clone(), + } + .bye() + .await, + InviteDialog { + inner: inner.clone(), + } + .bye() + .await, + ServerInviteDialog { inner }.bye().await, + ]; + for e in errors { + let e = e.expect_err("BYE in Early must fail").to_string(); + assert!(e.contains("(Early)") && !e.contains(MARKER), "{e}"); + } + let warns = capture.lines(Level::WARN, "bye skipped"); + assert_eq!(warns.len(), 3, "{warns:?}"); + assert!(warns.iter().all(|w| !w.contains(MARKER)), "{warns:?}"); + Ok(()) +} + +#[cfg(feature = "websocket")] +#[tokio::test] +async fn test_websocket_parse_failure_warns_without_the_message() -> crate::Result<()> { + use crate::transport::{stream::StreamConnection, websocket::WebSocketConnection}; + use futures::SinkExt; + use tokio_tungstenite::tungstenite::{handshake::server::Response, Message}; + + let capture = Capture::default(); + let _guard = tracing::subscriber::set_default(capture.clone()); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await?; + let addr = listener.local_addr()?; + tokio::spawn(async move { + let (stream, _) = listener.accept().await.unwrap(); + let sip = |_: &_, mut resp: Response| { + let headers = resp.headers_mut(); + headers.insert("sec-websocket-protocol", "sip".parse().unwrap()); + Ok(resp) + }; + let mut ws = tokio_tungstenite::accept_hdr_async(stream, sip) + .await + .unwrap(); + let text = format!("NOT SIP \r\n\r\n"); + ws.send(Message::Text(text.into())).await.unwrap(); + ws.close(None).await.ok(); + }); + let remote = SipAddr::new(crate::sip::transport::Transport::Ws, addr.into()); + let conn = WebSocketConnection::connect(&remote, None).await?; + conn.serve_loop(unbounded_channel().0).await?; + + let warns = capture.lines(Level::WARN, "Error parsing SIP message"); + assert_eq!(warns.len(), 1, "{warns:?}"); + assert!(!warns[0].contains(MARKER), "{warns:?}"); + assert!(warns[0].contains("len=")); + // Still logged, below WARN, when it is received. + assert!(!capture.lines(Level::INFO, MARKER).is_empty()); + Ok(()) +} diff --git a/src/transport/websocket.rs b/src/transport/websocket.rs index ee6ed530..250ae5c5 100644 --- a/src/transport/websocket.rs +++ b/src/transport/websocket.rs @@ -340,7 +340,7 @@ impl StreamConnection for WebSocketConnection { } } Err(e) => { - warn!(error = %e, src = %remote_addr, raw_message = ?text.as_str(), "Error parsing SIP message"); + warn!(error = %e, src = %remote_addr, len = text.len(), "Error parsing SIP message"); } } } From e640905cb8cd82afc03ab88cde77196c5bdaf3d9 Mon Sep 17 00:00:00 2001 From: Tyson George Date: Wed, 7 Oct 2026 06:38:53 -0400 Subject: [PATCH 07/16] fix(dialog): a BYE ends the dialog even when its transaction fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC 3261 §15.1.1: the session ends once the BYE is passed to the client transaction, and the dialog ends on a 481, a 408 or no response. `bye_with_headers` notified `Terminated` only after the BYE transaction returned Ok. When it returned an error (the target locator fails, or a 401/407 to the BYE cannot be answered) the dialog stayed `Confirmed`, and callers that remove the dialog on `Terminated` kept it forever. `DialogInner::send_bye`, used by `InviteDialog` and both deprecated wrappers, notifies `Terminated` (`UacBye` / `UasBye`, as before) whatever the transaction returns and still returns its error. --- src/dialog/client_dialog.rs | 8 +- src/dialog/dialog.rs | 11 ++ src/dialog/invite_dialog.rs | 8 +- src/dialog/server_dialog.rs | 8 +- src/dialog/tests/test_client_dialog.rs | 162 ++++++++++++++++++++++++- 5 files changed, 183 insertions(+), 14 deletions(-) diff --git a/src/dialog/client_dialog.rs b/src/dialog/client_dialog.rs index c4b3fd8c..6d859054 100644 --- a/src/dialog/client_dialog.rs +++ b/src/dialog/client_dialog.rs @@ -157,6 +157,9 @@ impl ClientInviteDialog { /// # Returns /// * `Ok(())` - BYE was sent successfully or dialog is already terminated. /// * `Err(Error)` - Failed to build/send BYE request, or dialog is in a state where BYE does not apply. + /// + /// Once the BYE is handed to its transaction the dialog is `Terminated`, + /// even when an error is returned (RFC 3261 §15.1.1). pub async fn bye_with_headers(&self, headers: Option>) -> Result<()> { if !self.inner.is_confirmed() { if !self.inner.is_terminated() { @@ -178,10 +181,7 @@ impl ClientInviteDialog { self.inner .make_request(crate::sip::Method::Bye, None, None, None, headers, None)?; - self.inner.do_request(request).await?; - self.inner - .transition(DialogState::Terminated(self.id(), TerminatedReason::UacBye))?; - Ok(()) + self.inner.send_bye(request, TerminatedReason::UacBye).await } /// Send a BYE request with a SIP `Reason` header. diff --git a/src/dialog/dialog.rs b/src/dialog/dialog.rs index 9f62fc2a..46432a4e 100644 --- a/src/dialog/dialog.rs +++ b/src/dialog/dialog.rs @@ -1312,6 +1312,17 @@ impl DialogInner { self.send_dialog_request(request).boxed().await } + /// Send the BYE that ends this dialog. RFC 3261 §15.1.1: the session ends + /// once the BYE is handed to its transaction, and a 481, a 408 or no + /// response ends the dialog. So whatever the transaction returns, the + /// dialog is `Terminated(reason)`; a failure is still returned. + pub(super) async fn send_bye(&self, request: Request, reason: TerminatedReason) -> Result<()> { + let result = self.do_request(request).await; + let id = self.id.lock().clone(); + self.transition(DialogState::Terminated(id, reason))?; + result.map(|_| ()) + } + /// RFC 3261 §13.3.1.4: the server transaction of an INVITE or re-INVITE /// retransmitted our 2xx (`answered_2xx`) for 64*T1 and ended without an /// ACK. The dialog is terminated with [`TerminatedReason::Timeout`] and the diff --git a/src/dialog/invite_dialog.rs b/src/dialog/invite_dialog.rs index 72a3587b..8c185cd1 100644 --- a/src/dialog/invite_dialog.rs +++ b/src/dialog/invite_dialog.rs @@ -422,6 +422,9 @@ impl InviteDialog { /// # Returns /// * `Ok(())` - BYE was sent successfully or dialog is already terminated. /// * `Err(Error)` - Failed to build/send BYE request, or dialog is in a state where BYE does not apply. + /// + /// Once the BYE is handed to its transaction the dialog is `Terminated`, + /// even when an error is returned (RFC 3261 §15.1.1). pub async fn bye_with_headers(&self, headers: Option>) -> Result<()> { let confirmed_or_waiting_ack = self.inner.is_confirmed() || (self.role() == TransactionRole::Server && self.inner.waiting_ack()); @@ -445,14 +448,11 @@ impl InviteDialog { .inner .make_request(Method::Bye, None, None, None, headers, None)?; - self.inner.do_request(request).await?; let reason = match self.role() { TransactionRole::Server => TerminatedReason::UasBye, TransactionRole::Client => TerminatedReason::UacBye, }; - self.inner - .transition(DialogState::Terminated(self.id(), reason))?; - Ok(()) + self.inner.send_bye(request, reason).await } /// Send a BYE request with a SIP `Reason` header. diff --git a/src/dialog/server_dialog.rs b/src/dialog/server_dialog.rs index 3456600f..c4e09f75 100644 --- a/src/dialog/server_dialog.rs +++ b/src/dialog/server_dialog.rs @@ -351,6 +351,9 @@ impl ServerInviteDialog { /// # Returns /// * `Ok(())` - BYE was sent successfully or dialog is already terminated. /// * `Err(Error)` - Failed to build/send BYE request, or dialog is in a state where BYE does not apply. + /// + /// Once the BYE is handed to its transaction the dialog is `Terminated`, + /// even when an error is returned (RFC 3261 §15.1.1). pub async fn bye_with_headers(&self, headers: Option>) -> Result<()> { if !self.inner.is_confirmed() && !self.inner.waiting_ack() { if !self.inner.is_terminated() { @@ -372,10 +375,7 @@ impl ServerInviteDialog { self.inner .make_request(crate::sip::Method::Bye, None, None, None, headers, None)?; - self.inner.do_request(request).await?; - self.inner - .transition(DialogState::Terminated(self.id(), TerminatedReason::UasBye))?; - Ok(()) + self.inner.send_bye(request, TerminatedReason::UasBye).await } /// Send a BYE request with a SIP `Reason` header. diff --git a/src/dialog/tests/test_client_dialog.rs b/src/dialog/tests/test_client_dialog.rs index 9b7838c4..b487a139 100644 --- a/src/dialog/tests/test_client_dialog.rs +++ b/src/dialog/tests/test_client_dialog.rs @@ -3,15 +3,19 @@ use crate::dialog::{ dialog::{DialogInner, DialogState, TerminatedReason}, DialogId, }; -use crate::sip::{headers::*, prelude::HeadersExt, Request, Response, StatusCode, Uri}; -use crate::transaction::endpoint::TargetLocator; +use crate::sip::{headers::*, prelude::HeadersExt, Method, Request, Response, SipMessage}; +use crate::sip::{StatusCode, Uri}; +use crate::transaction::endpoint::{EndpointOption, TargetLocator}; use crate::transaction::key::TransactionRole; use crate::transport::transport_layer::DomainResolver; use crate::transport::SipConnection; use crate::transport::{udp::UdpConnection, SipAddr, TransportLayer}; use crate::EndpointBuilder; use async_trait::async_trait; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; +use std::time::Duration; +use tokio::net::UdpSocket; use tokio::sync::mpsc::unbounded_channel; use tokio::sync::oneshot; use tokio_util::sync::CancellationToken; @@ -1695,3 +1699,157 @@ async fn test_cancel_returns_error_on_transaction_timeout() -> crate::Result<()> Ok(()) } + +/// Resolves like the default rule until the flag is set, then fails. +struct FlakyLocator(Arc); + +#[async_trait] +impl TargetLocator for FlakyLocator { + async fn locate(&self, uri: &Uri) -> crate::Result { + if self.0.load(Ordering::SeqCst) { + return Err(crate::Error::Error("no route".to_string())); + } + SipAddr::try_from(uri) + } +} + +/// Waits for a `method` request at `peer` and answers it with `status` (if any). +async fn answer_next(peer: &UdpSocket, method: Method, status: Option<&str>) -> crate::Result<()> { + let mut buf = vec![0u8; 4096]; + loop { + let recv = tokio::time::timeout(Duration::from_secs(2), peer.recv_from(&mut buf)); + let (len, from) = recv + .await + .unwrap_or_else(|_| panic!("no {method} request"))?; + let Ok(SipMessage::Request(req)) = SipMessage::try_from(&buf[..len]) else { + continue; + }; + if req.method != method { + continue; + } + if let Some(status) = status { + let to = req.to_header()?.value().to_string(); + let to = if to.contains(";tag=") { + to + } else { + format!("{to};tag=bob") + }; + let resp = format!( + "SIP/2.0 {status}\r\nVia: {}\r\nFrom: {}\r\nTo: {to}\r\nCall-ID: {}\r\n\ + CSeq: {}\r\nContact: \r\nContent-Length: 0\r\n\r\n", + req.via_header()?.value(), + req.from_header()?.value(), + req.call_id_header()?.value(), + req.cseq_header()?.value(), + peer.local_addr()?, + ); + peer.send_to(resp.as_bytes(), from).await?; + } + return Ok(()); + } +} + +/// RFC 3261 §15.1.1: once the BYE is handed to its client transaction the +/// session is over, and a 481, a 408 or no response also ends the dialog. +/// Every outcome of `bye()` leaves the dialog `Terminated`, notified once. +#[tokio::test] +async fn test_bye_terminates_the_dialog_whatever_the_outcome() -> crate::Result<()> { + use crate::dialog::{authenticate::Credential, dialog_layer::DialogLayer}; + use crate::dialog::{invitation::InviteOption, server_dialog::ServerInviteDialog}; + // (case, answer to the BYE, the BYE has no route, bye() via: the + // deprecated wrappers are driven over the same dialog) + for (case, answer, no_route, via) in [ + ( + "481", + Some("481 Call/Transaction Does Not Exist"), + false, + "invite", + ), + ("408", Some("408 Request Timeout"), false, "invite"), + ("no response", None, false, "invite"), + ( + "401 without a challenge", + Some("401 Unauthorized"), + false, + "invite", + ), + ("no route", None, true, "invite"), + ("no route", None, true, "client"), + ("no route", None, true, "server"), + ] { + let token = CancellationToken::new(); + let peer = UdpSocket::bind("127.0.0.1:0").await?; + let no_route_flag = Arc::new(AtomicBool::new(false)); + let tl = TransportLayer::new(token.child_token()); + let udp = UdpConnection::create_connection("127.0.0.1:0".parse()?, None, None).await?; + let local = udp.get_addr().get_socketaddr()?; + tl.add_transport(udp.into()); + let endpoint = EndpointBuilder::new() + .with_transport_layer(tl) + .with_cancel_token(token.child_token()) + .with_target_locator(Box::new(FlakyLocator(no_route_flag.clone()))) + .with_option(EndpointOption { + t1: Duration::from_millis(10), + t1x64: Duration::from_millis(640), + ..Default::default() + }) + .build(); + let inner = endpoint.inner.clone(); + tokio::spawn(async move { inner.serve().await }); + let (state_sender, mut states) = unbounded_channel(); + let invite = InviteOption { + caller: Uri::try_from("sip:alice@example.com")?, + callee: Uri::try_from(format!("sip:bob@{}", peer.local_addr()?).as_str())?, + contact: Uri::try_from(format!("sip:alice@{local}").as_str())?, + // Answers a 401 to the BYE, which carries no challenge. + credential: Some(Credential { + username: "alice".into(), + password: "secret".into(), + realm: None, + auth_username: None, + }), + ..Default::default() + }; + let layer = DialogLayer::new(endpoint.inner.clone()); + let invite = tokio::spawn(async move { layer.do_invite(invite, state_sender).await }); + answer_next(&peer, Method::Invite, Some("200 OK")).await?; + let (dialog, _) = invite.await.unwrap()?; + while states.try_recv().is_ok() {} + + no_route_flag.store(no_route, Ordering::SeqCst); + let inner = dialog.inner.clone(); + let state = dialog.inner.clone(); + let bye = tokio::spawn(async move { + match via { + "client" => ClientInviteDialog { inner }.bye().await, + "server" => ServerInviteDialog { inner }.bye().await, + _ => dialog.bye().await, + } + }); + if !no_route { + answer_next(&peer, Method::Bye, answer).await?; + } + let result = tokio::time::timeout(Duration::from_secs(3), bye) + .await + .unwrap_or_else(|_| panic!("{case} via {via}: bye() hangs")) + .unwrap(); + let mut reasons = Vec::new(); + while let Ok(state) = states.try_recv() { + if let DialogState::Terminated(_, reason) = state { + reasons.push(reason); + } + } + let expected = if via == "server" { + "[UasBye]" + } else { + "[UacBye]" + }; + assert_eq!(format!("{reasons:?}"), expected, "{case} via {via}"); + assert!(state.is_terminated(), "{case} via {via}"); + // The BYE's failure is still reported. + let failed = no_route || answer == Some("401 Unauthorized"); + assert_eq!(result.is_err(), failed, "{case} via {via}: {result:?}"); + token.cancel(); + } + Ok(()) +} From faab351475ea27ffd870fbf228a49a7458861048 Mon Sep 17 00:00:00 2001 From: jinti Date: Wed, 7 Oct 2026 19:51:55 +0800 Subject: [PATCH 08/16] =?UTF-8?q?feat(tls):=20injectable=20TLS=20client=20?= =?UTF-8?q?seam=20=E2=80=94=20rustls=20stays=20the=20host=20default?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - platform::tls: TlsConnector/TlsStream/TcpStream traits (poll-based, no_std-safe) + TlsClientConfig + process-wide connector registry - transport::tls::TlsConnection::connect consults the registered connector first (embedded backends: embassy-net + embedded-tls etc., the connector owns the TCP dial); without one, the built-in rustls path is unchanged - client halves are boxed so rustls and seam streams share one TlsConnectionInner::Client representation - e2e test: a registered connector bypasses rustls entirely - readme: no_std/embedded support notes --- README.md | 33 ++++- src/platform/mod.rs | 1 + src/platform/tls.rs | 86 +++++++++++++ src/transport/tls.rs | 290 +++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 401 insertions(+), 9 deletions(-) create mode 100644 src/platform/tls.rs diff --git a/README.md b/README.md index 3807981b..a1eac531 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ A RFC 3261/3262 compliant SIP stack written in Rust. The goal of this project is ## Features - **RFC 3261/3262 Compliant**: Full compliance with SIP specification -- **Multiple Transport Support**: UDP, TCP, TLS, WebSocket (TLS/WebSocket require the `rustls` and `websocket` features, enabled by default) +- **Multiple Transport Support**: UDP, TCP, TLS, WebSocket (TLS/WebSocket require the `rustls` and `websocket` features, enabled by default). The TLS client goes through a backend seam (`platform::tls`) — host default is the built-in rustls; embedded stacks can register their own connector - **Transaction Layer**: Complete SIP transaction state machine - **Dialog Layer**: SIP dialog management - **Reliable Provisionals**: PRACK (RFC 3262 / 100rel) support @@ -15,6 +15,36 @@ A RFC 3261/3262 compliant SIP stack written in Rust. The goal of this project is - **High Performance**: Built with Rust for maximum performance - **Easy to Use**: Simple and intuitive API design +## no_std / Embedded Support + +rsipstack compiles without `std` (`alloc`-only) for embassy-based targets — +verified on `xtensa-esp32s3-none-elf` (ESP32-S3): + +```bash +cargo check --no-default-features --features platform-embassy +``` + +- Core SIP codec, transaction, dialog, and **UDP transport** layers are + `no_std` + `alloc`. +- The `platform-embassy` backend maps the runtime seams onto + `embassy-time` / `embassy-sync` (critical-section based); task spawning is + injected once at startup: + + ```rust,ignore + rsipstack::platform::set_spawn_fn(|fut| spawner.spawn(fut).ok()); + ``` + +- Concurrent waits use the backend-agnostic `select2` / `select3` helpers + instead of `tokio::select!`. +- **SIPS (SIP over TLS, client role)** goes through the + `platform::tls::TlsConnector` seam: register a backend with + `platform::tls::set_client_connector` and every + `TlsConnection::connect` rides it. Host builds without a registered + connector keep using the built-in rustls path. +- TCP / WebSocket transports still require `platform-tokio`; on embedded, + use UDP or SIPS today. +- On no_std, register a `DomainResolver` instead of the tokio DNS resolver. + ## TODO - [x] Transport support - [x] UDP @@ -25,6 +55,7 @@ A RFC 3261/3262 compliant SIP stack written in Rust. The goal of this project is - [x] Transaction Layer - [x] Dialog Layer - [ ] WASM target +- [x] no_std embassy-based targets ## Use Cases diff --git a/src/platform/mod.rs b/src/platform/mod.rs index 2fffe4e6..9d929439 100644 --- a/src/platform/mod.rs +++ b/src/platform/mod.rs @@ -32,6 +32,7 @@ pub use tokio::sync::mpsc; pub mod atomic64; pub mod net; pub mod select; +pub mod tls; pub use select::{select2, select3, Either, Select2, Select3, Which3}; diff --git a/src/platform/tls.rs b/src/platform/tls.rs new file mode 100644 index 00000000..e86a2d0d --- /dev/null +++ b/src/platform/tls.rs @@ -0,0 +1,86 @@ +//! Backend-agnostic TLS client seam (SIPS). +//! +//! `transport::tls::TlsConnection::connect` consults the connector registered +//! here before falling back to the built-in rustls path, so custom TLS stacks +//! — including embedded ones over embassy-net (embedded-tls, esp-mbedtls) — +//! can provide SIPS without tokio or rustls. Host builds are unaffected when +//! nothing is registered. +//! +//! The connector **owns the TCP dial**: each backend brings its own TCP stack +//! (tokio on the host, embassy-net on embedded), so the seam only passes the +//! target address. Streams expose async-fn IO with owned buffers, which is +//！ directly implementable over `embedded-io-async` / rustls alike; the host +//! adapter buffers chunks when bridging into tokio's poll-based halves. + +use crate::Result; +use alloc::boxed::Box; +use alloc::string::String; +use alloc::sync::Arc; +use alloc::vec::Vec; +use core::net::SocketAddr; +use core::sync::atomic::{AtomicPtr, Ordering}; + +/// Backend-agnostic TLS client configuration (PEM encodings). +#[derive(Debug, Clone, Default)] +pub struct TlsClientConfig { + /// SNI / certificate verification name (e.g. `pbx.example.com`). + pub server_name: String, + /// Root certificates to trust (PEM). `None`/empty: backend default trust. + pub root_certs: Option>, + /// Client certificate for mTLS (PEM). + pub client_cert: Option>, + /// Client key for mTLS (PEM). + pub client_key: Option>, +} + +/// An established TLS client session (handshake already completed). +#[async_trait::async_trait] +pub trait TlsStream: Send + Sync + 'static { + /// Reads the next chunk of application data (one backend buffer's worth; + /// chunking carries no message framing). + async fn recv(&self) -> Result>; + + /// Sends all of `data`. + async fn send_all(&self, data: Vec) -> Result<()>; + + /// Closes the session (TLS close_notify + TCP shutdown when supported). + async fn shutdown(&self) -> Result<()>; + + /// Local socket address of the underlying TCP connection. + fn local_addr(&self) -> Result; +} + +/// Factory for outbound TLS sessions (SIPS client role). Owns the TCP dial. +#[async_trait::async_trait] +pub trait TlsConnector: Send + Sync + 'static { + async fn connect( + &self, + config: &TlsClientConfig, + addr: SocketAddr, + ) -> Result>; +} + +static CLIENT_CONNECTOR: AtomicPtr> = AtomicPtr::new(core::ptr::null_mut()); + +/// Registers the process-wide TLS client connector (thread-safe, leak-once). +/// Call once during startup, before any SIPS connection. +pub fn set_client_connector(connector: Arc) { + let leaked = alloc::boxed::Box::leak(alloc::boxed::Box::new(connector)); + CLIENT_CONNECTOR.store(leaked as *mut Arc, Ordering::Release); +} + +/// Clears a previously registered connector (mainly for tests). +pub fn clear_client_connector() { + CLIENT_CONNECTOR.store(core::ptr::null_mut(), Ordering::Release); +} + +/// Returns the registered connector, if any. +pub fn client_connector() -> Option> { + let ptr = CLIENT_CONNECTOR.load(Ordering::Acquire); + if ptr.is_null() { + None + } else { + // The Arc was leaked at registration and outlives the process. + unsafe { Some((*ptr).clone()) } + } +} diff --git a/src/transport/tls.rs b/src/transport/tls.rs index f26462b4..ff294b91 100644 --- a/src/transport/tls.rs +++ b/src/transport/tls.rs @@ -7,6 +7,9 @@ use super::{ use crate::platform::CancellationToken; use crate::sip::SipMessage; use crate::{error::Error, transport::transport_layer::TransportLayerInnerRef, Result}; +use core::future::Future; +use core::pin::Pin; +use core::task::{Context, Poll}; use rustls::client::danger::ServerCertVerifier; use rustls::crypto::CryptoProvider; use rustls::server::{ClientHello, ResolvesServerCert}; @@ -436,6 +439,117 @@ impl fmt::Debug for TlsListenerConnection { type TlsClientStream = tokio_rustls::client::TlsStream; type TlsServerStream = tokio_rustls::server::TlsStream; +/// Client halves are boxed so rustls and platform-seam streams share one +/// `TlsConnectionInner::Client` representation. +type ClientReadHalf = Box; +type ClientWriteHalf = Box; + +type BoxFuture = Pin + Send>>; + +/// Bridges a seam TLS stream (async-fn, owned-data) back into the tokio +/// world. Chunks from `recv()` are buffered; `send_all`/`shutdown` futures +/// hold an owned `Arc` + `Vec`, so nothing borrows across polls. +struct TokioSeamStream { + inner: Arc, + recv_fut: Option>>>, + send_fut: Option>>, + shutdown_fut: Option>>, + chunk: alloc::collections::VecDeque, +} + +impl TokioSeamStream { + fn new(inner: Arc) -> Self { + Self { + inner, + recv_fut: None, + send_fut: None, + shutdown_fut: None, + chunk: alloc::collections::VecDeque::new(), + } + } +} + +impl tokio::io::AsyncRead for TokioSeamStream { + fn poll_read( + self: Pin<&mut Self>, + cx: &mut Context<'_>, + rbuf: &mut tokio::io::ReadBuf<'_>, + ) -> Poll> { + let this = self.get_mut(); + loop { + if !this.chunk.is_empty() { + let n = this.chunk.len().min(rbuf.remaining()); + let data: Vec = this.chunk.drain(..n).collect(); + rbuf.put_slice(&data); + return Poll::Ready(Ok(())); + } + if this.recv_fut.is_none() { + let inner = this.inner.clone(); + this.recv_fut = Some(Box::pin(async move { inner.recv().await })); + } + match this.recv_fut.as_mut().unwrap().as_mut().poll(cx) { + Poll::Ready(Ok(data)) => { + this.recv_fut = None; + if data.is_empty() { + // Peer closed the stream. + return Poll::Ready(Ok(())); + } + this.chunk.extend(data); + } + Poll::Ready(Err(e)) => return Poll::Ready(Err(seam_io_err(e))), + Poll::Pending => return Poll::Pending, + } + } + } +} + +fn seam_io_err(e: crate::Error) -> std::io::Error { + std::io::Error::other(e.to_string()) +} + +impl tokio::io::AsyncWrite for TokioSeamStream { + fn poll_write( + self: Pin<&mut Self>, + cx: &mut Context<'_>, + buf: &[u8], + ) -> Poll> { + let this = self.get_mut(); + if this.send_fut.is_none() { + let inner = this.inner.clone(); + let data = buf.to_vec(); + this.send_fut = Some(Box::pin(async move { inner.send_all(data).await })); + } + match this.send_fut.as_mut().unwrap().as_mut().poll(cx) { + Poll::Ready(Ok(())) => { + this.send_fut = None; + Poll::Ready(Ok(buf.len())) + } + Poll::Ready(Err(e)) => Poll::Ready(Err(seam_io_err(e))), + Poll::Pending => Poll::Pending, + } + } + + fn poll_flush(self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll> { + Poll::Ready(Ok(())) // send_all is already awaited to completion + } + + fn poll_shutdown(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + let this = self.get_mut(); + if this.shutdown_fut.is_none() { + let inner = this.inner.clone(); + this.shutdown_fut = Some(Box::pin(async move { inner.shutdown().await })); + } + match this.shutdown_fut.as_mut().unwrap().as_mut().poll(cx) { + Poll::Ready(Ok(())) => { + this.shutdown_fut = None; + Poll::Ready(Ok(())) + } + Poll::Ready(Err(e)) => Poll::Ready(Err(seam_io_err(e))), + Poll::Pending => Poll::Pending, + } + } +} + // TLS connection - uses enum to handle both client and server streams #[derive(Clone)] pub struct TlsConnection { @@ -445,14 +559,7 @@ pub struct TlsConnection { #[derive(Clone)] enum TlsConnectionInner { - Client( - Arc< - StreamConnectionInner< - tokio::io::ReadHalf, - tokio::io::WriteHalf, - >, - >, - ), + Client(Arc>), Server( Arc< StreamConnectionInner< @@ -479,6 +586,13 @@ impl TlsConnection { custom_verifier: Option>, cancel_token: Option, ) -> Result { + // A registered platform connector (embedded backends) takes + // precedence over the built-in rustls path. + if let Some(connector) = crate::platform::tls::client_connector() { + return Self::connect_via_platform(remote_addr, tls_config, cancel_token, connector) + .await; + } + let mut root_store = RootCertStore::empty(); // Load CA certificates if provided @@ -552,6 +666,10 @@ impl TlsConnection { let tls_stream = connector.connect(server_name, stream).await?; let (read_half, write_half) = tokio::io::split(tls_stream); + let (read_half, write_half) = ( + Box::new(read_half) as ClientReadHalf, + Box::new(write_half) as ClientWriteHalf, + ); let connection = Self { inner: TlsConnectionInner::Client(Arc::new(StreamConnectionInner::new( @@ -570,6 +688,67 @@ impl TlsConnection { Ok(connection) } + // Client connect through the platform TLS seam (embedded backends). + async fn connect_via_platform( + remote_addr: &SipAddr, + tls_config: Option<&TlsConfig>, + cancel_token: Option, + connector: Arc, + ) -> Result { + let config = crate::platform::tls::TlsClientConfig { + server_name: tls_config + .and_then(|c| c.sni_hostname.clone()) + .unwrap_or_else(|| match &remote_addr.addr.host { + crate::sip::Host::Domain(domain) => domain.to_string(), + crate::sip::Host::IpAddr(ip) => ip.to_string(), + }), + root_certs: tls_config.and_then(|c| c.ca_certs.clone()), + client_cert: tls_config.and_then(|c| c.client_cert.clone()), + client_key: tls_config.and_then(|c| c.client_key.clone()), + }; + + let socket_addr = match &remote_addr.addr.host { + crate::sip::Host::Domain(domain) => { + let port = remote_addr.addr.port.as_ref().map_or(5061, |p| p.value()); + format!("{}:{}", domain, port).parse()? + } + crate::sip::Host::IpAddr(ip) => { + let port = remote_addr.addr.port.as_ref().map_or(5061, |p| p.value()); + SocketAddr::new(*ip, port) + } + }; + + // The connector owns the TCP dial (backend-specific TCP stack). + let tls_stream = connector.connect(&config, socket_addr).await?; + let local = tls_stream.local_addr()?; + + let local_addr = SipAddr { + r#type: Some(crate::sip::transport::Transport::Tls), + addr: local.into(), + }; + let (read_half, write_half) = tokio::io::split(TokioSeamStream::new(Arc::from(tls_stream))); + let (read_half, write_half) = ( + Box::new(read_half) as ClientReadHalf, + Box::new(write_half) as ClientWriteHalf, + ); + + let connection = Self { + inner: TlsConnectionInner::Client(Arc::new(StreamConnectionInner::new( + local_addr.clone(), + remote_addr.clone(), + read_half, + write_half, + ))), + cancel_token, + }; + debug!( + "Created TLS client connection (platform seam): {} -> {}", + local_addr, remote_addr + ); + + Ok(connection) + } + // Create TLS connection from existing client TLS stream pub async fn from_client_stream( stream: TlsClientStream, @@ -583,6 +762,10 @@ impl TlsConnection { // Split stream into read and write halves let (read_half, write_half) = tokio::io::split(stream); + let (read_half, write_half) = ( + Box::new(read_half) as ClientReadHalf, + Box::new(write_half) as ClientWriteHalf, + ); // Create TLS connection let connection = Self { @@ -704,3 +887,94 @@ impl fmt::Debug for TlsConnection { fmt::Display::fmt(self, f) } } + +#[cfg(test)] +mod platform_seam_tests { + use super::*; + use crate::platform::tls as seam; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + /// Plaintext pass-through "TLS": proves the registered connector was used + /// instead of rustls (which would fail to handshake with a plain echoer). + /// The connector dials `addr` itself, exactly like an embedded backend + /// (embassy-net + embedded-tls) would. + struct MockConnector; + + #[async_trait::async_trait] + impl seam::TlsConnector for MockConnector { + async fn connect( + &self, + _config: &seam::TlsClientConfig, + addr: SocketAddr, + ) -> Result> { + let tcp = tokio::net::TcpStream::connect(addr).await?; + let local = tcp.local_addr()?; + Ok(Box::new(MockStream { + inner: tcp.into(), + local, + })) + } + } + + struct MockStream { + inner: tokio::sync::Mutex, + local: SocketAddr, + } + + #[async_trait::async_trait] + impl seam::TlsStream for MockStream { + async fn recv(&self) -> Result> { + let mut buf = vec![0u8; 512]; + let mut tcp = self.inner.lock().await; + let n = tcp.read(&mut buf).await?; + buf.truncate(n); + Ok(buf) + } + + async fn send_all(&self, data: Vec) -> Result<()> { + let mut tcp = self.inner.lock().await; + tcp.write_all(&data).await?; + Ok(()) + } + + async fn shutdown(&self) -> Result<()> { + Ok(()) + } + + fn local_addr(&self) -> Result { + Ok(self.local) + } + } + + #[tokio::test] + async fn platform_connector_takes_precedence_over_rustls() { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap(); + let echo = tokio::spawn(async move { + let (mut sock, _) = listener.accept().await.unwrap(); + let mut buf = [0u8; 4]; + tokio::io::AsyncReadExt::read_exact(&mut sock, &mut buf) + .await + .unwrap(); + assert_eq!(&buf, b"ping"); + tokio::io::AsyncWriteExt::write_all(&mut sock, b"pong") + .await + .unwrap(); + }); + + seam::set_client_connector(std::sync::Arc::new(MockConnector)); + + let remote = SipAddr { + r#type: Some(crate::sip::transport::Transport::Tls), + addr: addr.into(), + }; + let conn = TlsConnection::connect(&remote, None, None, None) + .await + .expect("seam connector should bypass rustls entirely"); + + conn.send_raw(b"ping").await.unwrap(); + echo.await.unwrap(); + + seam::clear_client_connector(); + } +} From d89c32c9be61c886210ebf05a7958be1cdb192e6 Mon Sep 17 00:00:00 2001 From: jinti Date: Wed, 7 Oct 2026 20:39:01 +0800 Subject: [PATCH 09/16] =?UTF-8?q?fix(dialog):=20BYE=20a=20forked=202xx's?= =?UTF-8?q?=20dialog=20(RFC=203261=20=C2=A713.2.2.4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every 2xx to the INVITE with a new To tag establishes its own dialog. The transaction already re-ACKs a forked 2xx with its own tag and remote target (#172), but the Accepted-window drainer dropped it: no dialog was created for the branch and no BYE was ever sent, so the forked callee kept retransmitting its 2xx until it gave up. The drainer now inspects every message the client INVITE transaction delivers after confirmation: a 2xx whose To tag differs from the confirmed dialog's is ended with a BYE built from that response's own Contact (remote target) and Record-Route (route set), CSeq continuing the forked dialog's. The confirmed dialog's state is untouched, no dialog is registered for the branch, and no state is notified. --- src/dialog/dialog.rs | 61 ++++++++ src/dialog/invitation.rs | 12 +- src/dialog/invite_dialog.rs | 41 ++++- src/dialog/tests/mod.rs | 1 + src/dialog/tests/test_forked_2xx_bye.rs | 197 ++++++++++++++++++++++++ 5 files changed, 309 insertions(+), 3 deletions(-) create mode 100644 src/dialog/tests/test_forked_2xx_bye.rs diff --git a/src/dialog/dialog.rs b/src/dialog/dialog.rs index 992cf98d..c40e9680 100644 --- a/src/dialog/dialog.rs +++ b/src/dialog/dialog.rs @@ -1335,6 +1335,67 @@ impl DialogInner { result.map(|_| ()) } + /// End a dialog a forked 2xx established (RFC 3261 §13.2.2.4). + /// + /// Every 2xx to the INVITE with a new To tag creates its own dialog; the + /// UAC keeps a single session (the first 2xx's), so the transaction has + /// already ACKed the forked 2xx and this sends the BYE that terminates + /// the extra branch. The BYE is built from that response's own remote + /// target (Contact) and route set (Record-Route); the confirmed dialog's + /// state is not touched and no dialog is registered for the branch. + pub(super) async fn bye_forked_branch(&self, resp: &Response) -> Result<()> { + let contact_uri = resp + .typed_contact_headers()? + .first() + .map(|c| c.uri.clone()) + .ok_or_else(|| crate::Error::Error("missing Contact header".to_string()))?; + + // §12.2.1.1: the forked dialog's route set is the 2xx's Record-Route. + let mut routes: Vec = resp + .record_route_headers() + .into_iter() + .flat_map(|rr| split_rr_values(rr.value())) + .map(Route::from) + .collect(); + routes.reverse(); + + // To carries the forked branch's tag, From keeps ours. + let to = resp.to_header()?.clone(); + let id = self.id.lock().clone(); + let via = self + .endpoint_inner + .get_via(self.via_addr_for_send_transport(), None)?; + let cseq = CSeq { + seq: self.increment_local_seq(), + method: Method::Bye, + }; + + let mut headers: Vec
= vec![ + Header::Via(via.into()), + Header::CallId(id.call_id.clone().into()), + Header::From(self.from.clone().to_string().into()), + Header::To(to), + Header::CSeq(cseq.into()), + Header::UserAgent(self.endpoint_inner.user_agent.clone().into()), + ]; + if let Some(uri) = self.local_contact.as_ref() { + headers.push(Contact::from(uri.clone()).into()); + } + headers.extend(routes.into_iter().map(Header::Route)); + headers.push(Header::MaxForwards(70.into())); + + debug!(id = %id, uri = %contact_uri, "sending BYE to a forked dialog"); + self.do_request(crate::sip::Request { + method: Method::Bye, + uri: contact_uri, + headers: headers.into(), + body: Vec::new(), + version: crate::sip::Version::V2, + }) + .await?; + Ok(()) + } + /// RFC 3261 §13.3.1.4: the server transaction of an INVITE or re-INVITE /// retransmitted our 2xx (`answered_2xx`) for 64*T1 and ended without an /// ACK. The dialog is terminated with [`TerminatedReason::Timeout`] and the diff --git a/src/dialog/invitation.rs b/src/dialog/invitation.rs index f39c3269..96da705a 100644 --- a/src/dialog/invitation.rs +++ b/src/dialog/invitation.rs @@ -719,8 +719,12 @@ impl DialogLayer { // here would leave it (and its timers) in the // endpoint's table and silently stop the re-ACKs. if let Some(mut tx) = guard.invite_tx.take() { + let dlg = dialog.clone(); + let confirmed_tag = new_dialog_id.remote_tag.clone(); crate::platform::spawn(async move { - while tx.receive().await.is_some() {} + while let Some(msg) = tx.receive().await { + dlg.end_forked_branch(&msg, &confirmed_tag).await; + } debug!(id = %new_dialog_id, "accepted transaction drained (Timer M expired)"); }); } @@ -795,8 +799,12 @@ impl DialogLayer { // observes forked 2xx, and detaches it from the // endpoint's table). See do_invite for the rationale. let confirmed_id = new_id.clone(); + let confirmed_tag = new_id.remote_tag.clone(); + let forked_dlg = dialog_clone.clone(); crate::platform::spawn(async move { - while tx.receive().await.is_some() {} + while let Some(msg) = tx.receive().await { + forked_dlg.end_forked_branch(&msg, &confirmed_tag).await; + } debug!(id = %confirmed_id, "accepted transaction drained (Timer M expired)"); }); } diff --git a/src/dialog/invite_dialog.rs b/src/dialog/invite_dialog.rs index 3cb0e35a..1bc4a364 100644 --- a/src/dialog/invite_dialog.rs +++ b/src/dialog/invite_dialog.rs @@ -22,7 +22,7 @@ use crate::transaction::key::TransactionRole; use crate::transaction::transaction::{Transaction, TransactionEvent}; use crate::Result; use core::sync::atomic::Ordering; -use tracing::{debug, trace, warn}; +use tracing::{debug, info, trace, warn}; /// Unified INVITE dialog that can act as either a UAS (Server) or UAC (Client). /// @@ -405,6 +405,45 @@ impl InviteDialog { Ok(()) } + /// End a forked dialog a later 2xx established (RFC 3261 §13.2.2.4). + /// + /// Called from the Accepted-window drainer for every message the client + /// INVITE transaction delivers after the dialog confirmed. A 2xx whose To + /// tag differs from the confirmed dialog's is a forked branch: the + /// transaction has already ACKed it, and the UAC — keeping a single + /// session — terminates it with a BYE. Everything else (retransmitted + /// 2xx with the same tag, non-2xx) is ignored. + pub(super) async fn end_forked_branch(&self, msg: &SipMessage, confirmed_remote_tag: &str) { + let SipMessage::Response(resp) = msg else { + return; + }; + if resp.status_code.kind() != StatusCodeKind::Successful { + return; + } + // Same tag: a retransmission of the confirmed 2xx, already re-ACKed. + let tag = match resp + .to_header() + .ok() + .and_then(|to| to.tag().ok().flatten()) + .map(|tag| tag.value().to_string()) + { + Some(tag) => tag, + None => return, + }; + if tag == confirmed_remote_tag { + return; + } + let id = self.id(); + info!( + id = %id, + tag = %tag, + "forked 2xx acknowledged; ending the extra branch with a BYE (RFC 3261 §13.2.2.4)" + ); + if let Err(e) = self.inner.bye_forked_branch(resp).await { + warn!(id = %id, tag = %tag, error = %e, "failed to BYE the forked branch"); + } + } + // ── Shared request semantics ────────────────────────────────────────── /// Send a BYE request to terminate the dialog. diff --git a/src/dialog/tests/mod.rs b/src/dialog/tests/mod.rs index b3d43cb3..00fe78a2 100644 --- a/src/dialog/tests/mod.rs +++ b/src/dialog/tests/mod.rs @@ -5,6 +5,7 @@ mod test_client_dialog; mod test_connection_affinity; mod test_dialog_layer; mod test_dialog_states; +mod test_forked_2xx_bye; mod test_in_dialog_provisional; mod test_in_dialog_via; mod test_invite_auth_challenge; diff --git a/src/dialog/tests/test_forked_2xx_bye.rs b/src/dialog/tests/test_forked_2xx_bye.rs new file mode 100644 index 00000000..bd601edf --- /dev/null +++ b/src/dialog/tests/test_forked_2xx_bye.rs @@ -0,0 +1,197 @@ +//! RFC 3261 §13.2.2.4: a 2xx to the INVITE with a new To tag establishes its +//! own dialog. The UAC keeps a single session (the first 2xx's): the +//! transaction ACKs the forked 2xx with its own tag and remote target, and +//! the dialog layer ends the extra branch with a BYE built from that +//! response's Contact. No dialog is registered for the branch and the +//! confirmed dialog's state is untouched. + +use crate::dialog::dialog_layer::DialogLayer; +use crate::dialog::invitation::InviteOption; +use crate::dialog::DialogId; +use crate::sip::prelude::HeadersExt; +use crate::sip::{Method, Request, SipMessage, Uri}; +use crate::transport::udp::UdpConnection; +use crate::transport::TransportLayer; +use crate::EndpointBuilder; +use std::time::Duration; +use tokio::net::UdpSocket; +use tokio::sync::mpsc::unbounded_channel; +use tokio_util::sync::CancellationToken; + +/// Receive the next request of `method` at `peer`. +async fn next_request(peer: &UdpSocket, method: Method, what: &str) -> Request { + let mut buf = vec![0u8; 4096]; + loop { + let (len, _) = tokio::time::timeout(Duration::from_secs(3), peer.recv_from(&mut buf)) + .await + .unwrap_or_else(|_| panic!("timeout waiting for the {what}")) + .expect("peer socket error"); + let Ok(SipMessage::Request(req)) = SipMessage::try_from(&buf[..len]) else { + continue; + }; + if req.method == method { + return req; + } + } +} + +/// RFC 3261 §13.2.2.4: a forked 2xx (To tag `tag-b`) is ACKed with its own +/// tag and remote target (the transaction's job, pinned here end to end) and +/// then ended with a BYE built from its Contact — without touching the +/// confirmed dialog or registering the forked branch. +#[tokio::test] +async fn test_forked_2xx_is_byed_without_touching_the_confirmed_dialog() -> crate::Result<()> { + let token = CancellationToken::new(); + let peer = UdpSocket::bind("127.0.0.1:0").await?; + let tl = TransportLayer::new(token.child_token()); + let udp = UdpConnection::create_connection("127.0.0.1:0".parse()?, None, None).await?; + let uac_addr = udp.get_addr().get_socketaddr()?; + let contact = Uri::try_from(format!("sip:alice@{uac_addr}").as_str())?; + tl.add_transport(udp.into()); + let endpoint = EndpointBuilder::new() + .with_transport_layer(tl) + .with_option(crate::transaction::endpoint::EndpointOption { + t1: Duration::from_millis(10), + t1x64: Duration::from_millis(640), + ..Default::default() + }) + .build(); + let inner = endpoint.inner.clone(); + tokio::spawn(async move { inner.serve().await }); + + let layer = DialogLayer::new(endpoint.inner.clone()); + let (state_sender, mut states) = unbounded_channel(); + let invite = InviteOption { + caller: Uri::try_from("sip:alice@example.com")?, + callee: Uri::try_from(format!("sip:bob@{}", peer.local_addr()?).as_str())?, + contact, + ..Default::default() + }; + // A second handle over the same layer state, for the assertions below + // (DialogLayer is not Clone; the registry lives in the shared inner). + let assert_layer = DialogLayer { + endpoint: layer.endpoint.clone(), + inner: layer.inner.clone(), + }; + let do_invite = tokio::spawn(async move { layer.do_invite(invite, state_sender).await }); + + // The INVITE arrives; branch A answers with tag-a. + let mut buf = vec![0u8; 4096]; + let (len, from) = tokio::time::timeout(Duration::from_secs(3), peer.recv_from(&mut buf)) + .await + .expect("timeout waiting for the INVITE")?; + let invite_req = match SipMessage::try_from(&buf[..len])? { + SipMessage::Request(req) if req.method == Method::Invite => req, + other => panic!("expected the INVITE, got {other}"), + }; + let ok_a = format!( + "SIP/2.0 200 OK\r\nVia: {}\r\nFrom: {}\r\nTo: {};tag=tag-a\r\nCall-ID: {}\r\nCSeq: {}\r\nContact: \r\nContent-Length: 0\r\n\r\n", + invite_req.via_header()?.value(), + invite_req.from_header()?.value(), + invite_req.to_header()?.value(), + invite_req.call_id_header()?.value(), + invite_req.cseq_header()?.value(), + peer.local_addr()?, + ); + peer.send_to(ok_a.as_bytes(), from).await?; + + let (dialog, _) = tokio::time::timeout(Duration::from_secs(3), do_invite) + .await + .expect("timeout waiting for do_invite") + .expect("do_invite task panicked")?; + assert!(dialog.inner.state.lock().is_confirmed()); + while states.try_recv().is_ok() {} + + let confirmed = dialog.id(); + assert_eq!(confirmed.remote_tag, "tag-a"); + + // Branch B forks in: same Call-ID, new To tag, own Contact. + let ok_b = format!( + "SIP/2.0 200 OK\r\nVia: {}\r\nFrom: {}\r\nTo: {};tag=tag-b\r\nCall-ID: {}\r\nCSeq: {}\r\nContact: \r\nContent-Length: 0\r\n\r\n", + invite_req.via_header()?.value(), + invite_req.from_header()?.value(), + invite_req.to_header()?.value(), + invite_req.call_id_header()?.value(), + invite_req.cseq_header()?.value(), + peer.local_addr()?, + ); + peer.send_to(ok_b.as_bytes(), uac_addr).await?; + + // The first ACK confirms branch A; the forked 2xx is then ACKed with + // its own tag and Contact as R-URI. + let mut ack = next_request(&peer, Method::Ack, "forked ACK").await; + while !ack + .to_header() + .ok() + .and_then(|to| to.tag().ok().flatten()) + .is_some_and(|tag| tag.value() == "tag-b") + { + ack = next_request(&peer, Method::Ack, "forked ACK").await; + } + let ack_to = ack.to_header()?.value().to_string(); + assert!(ack_to.contains(";tag=tag-b"), "ACK To: {ack_to}"); + assert!( + ack.uri.to_string().contains("bob-b"), + "ACK R-URI must be the forked Contact: {}", + ack.uri + ); + + // The forked branch is then ended with a BYE from its own Contact. + let bye = next_request(&peer, Method::Bye, "forked-branch BYE").await; + let bye_to = bye.to_header()?.value().to_string(); + assert!(bye_to.contains(";tag=tag-b"), "BYE To: {bye_to}"); + assert!( + bye.uri.to_string().contains("bob-b"), + "BYE R-URI must be the forked Contact: {}", + bye.uri + ); + let cseq = bye.cseq_header()?.value().to_string(); + assert_eq!(cseq, "2 BYE", "BYE CSeq continues the forked dialog's"); + assert_eq!( + bye.call_id_header()?.value(), + invite_req.call_id_header()?.value() + ); + + // The confirmed dialog is untouched: no new state notifications, no + // dialog registered for the forked branch, and the dialog still works. + assert!( + states.try_recv().is_err(), + "the forked 2xx must not touch the confirmed dialog's state" + ); + let forked_id = DialogId { + call_id: confirmed.call_id.clone(), + local_tag: confirmed.local_tag.clone(), + remote_tag: "tag-b".to_string(), + }; + assert!( + assert_layer.get_dialog(&forked_id).is_none(), + "the forked branch must not be registered as a dialog" + ); + assert!(dialog.inner.state.lock().is_confirmed()); + + // The confirmed dialog is still usable: a BYE ends it normally. + let dialog2 = dialog.clone(); + let bye_task = tokio::spawn(async move { dialog2.bye().await }); + let bye = next_request(&peer, Method::Bye, "confirmed-dialog BYE").await; + assert!( + bye.to_header()?.value().to_string().contains(";tag=tag-a"), + "the confirmed dialog's BYE keeps tag-a: {}", + bye.to_header()?.value() + ); + let ok_bye = format!( + "SIP/2.0 200 OK\r\nVia: {}\r\nFrom: {}\r\nTo: {}\r\nCall-ID: {}\r\nCSeq: {}\r\nContent-Length: 0\r\n\r\n", + bye.via_header()?.value(), + bye.from_header()?.value(), + bye.to_header()?.value(), + bye.call_id_header()?.value(), + bye.cseq_header()?.value(), + ); + peer.send_to(ok_bye.as_bytes(), uac_addr).await?; + tokio::time::timeout(Duration::from_secs(3), bye_task) + .await + .expect("timeout waiting for bye()") + .expect("bye task panicked")?; + assert!(dialog.inner.state.lock().is_terminated()); + token.cancel(); + Ok(()) +} From da7955138179d1882bbe5d2ce3acb835abdf44ce Mon Sep 17 00:00:00 2001 From: Tyson George Date: Wed, 7 Oct 2026 08:54:28 -0400 Subject: [PATCH 10/16] fix(dialog): end a dropped INVITE whose dialog was already removed When the `do_invite` future is dropped mid-INVITE, `DialogGuardForUnconfirmed` finds the dialog by removing it from the `DialogLayer` and did nothing when it was not there. An application that called `remove_dialog` before dropping the future (for example on its own hangup) therefore got no `Terminated`, no CANCEL after a provisional, and no BYE for a 2xx that answered the abandoned INVITE, leaving the callee in a session. The guard now keeps the INVITE's dialog and falls back to it when the layer entry is gone, until `process_invite` returns. A dialog that is still registered is handled exactly as before. --- src/dialog/invitation.rs | 20 ++++-- src/dialog/tests/test_cancel_2xx_race.rs | 81 ++++++++++++++++++++---- 2 files changed, 82 insertions(+), 19 deletions(-) diff --git a/src/dialog/invitation.rs b/src/dialog/invitation.rs index f39c3269..2e53bbd0 100644 --- a/src/dialog/invitation.rs +++ b/src/dialog/invitation.rs @@ -187,16 +187,22 @@ pub(super) struct DialogGuardForUnconfirmed<'a> { pub dialog_layer_inner: &'a DialogLayerInnerRef, pub id: &'a DialogId, invite_tx: Option, + /// The INVITE's dialog, `None` once `process_invite` returned. + dialog: Option, } impl<'a> Drop for DialogGuardForUnconfirmed<'a> { fn drop(&mut self) { - let Some(dlg) = self.dialog_layer_inner.dialogs.remove(&self.id.to_string()) else { - return; - }; - - let Dialog::Invite(client_dialog) = dlg else { - return; + let client_dialog = match self.dialog_layer_inner.dialogs.remove(&self.id.to_string()) { + Some(Dialog::Invite(client_dialog)) => client_dialog, + Some(_) => return, + // The application already removed the dialog from the layer + // (`DialogLayer::remove_dialog`): its INVITE still has to end. + None => match self.dialog.take() { + Some(client_dialog) => client_dialog, + // `process_invite` returned: `do_invite` handles the outcome. + None => return, + }, }; match client_dialog.state() { @@ -688,6 +694,7 @@ impl DialogLayer { dialog_layer_inner: &self.inner, id: &id, invite_tx: Some(tx), + dialog: Some(dialog.clone()), }; let tx = guard @@ -696,6 +703,7 @@ impl DialogLayer { .expect("transcation should be avaible"); let r = dialog.process_invite(tx).boxed().await; + guard.dialog = None; self.inner.dialogs.remove(&id.to_string()); match r { diff --git a/src/dialog/tests/test_cancel_2xx_race.rs b/src/dialog/tests/test_cancel_2xx_race.rs index a95033da..96a5cbd2 100644 --- a/src/dialog/tests/test_cancel_2xx_race.rs +++ b/src/dialog/tests/test_cancel_2xx_race.rs @@ -10,6 +10,7 @@ use crate::dialog::{ dialog::{DialogState, DialogStateReceiver, TerminatedReason}, dialog_layer::DialogLayer, invitation::InviteOption, + DialogId, }; use crate::sip::{prelude::HeadersExt, Method, Request, SipMessage, Uri}; use crate::transport::{udp::UdpConnection, TransportLayer}; @@ -181,7 +182,19 @@ enum Order { InviteOkLate, } -async fn run_crossing_2xx(order: Order) -> crate::Result<()> { +/// The id the `Calling` state reports, the one the dialog is registered under. +async fn calling_id(states: &mut DialogStateReceiver) -> DialogId { + match wait_for_state(states, "Calling", Duration::from_secs(2), |s| { + matches!(s, DialogState::Calling(_)) + }) + .await + { + DialogState::Calling(id) => id, + _ => unreachable!(), + } +} + +async fn run_crossing_2xx(order: Order, provisional: u16, remove: bool) -> crate::Result<()> { let token = CancellationToken::new(); let Uac { dialog_layer, @@ -191,15 +204,27 @@ async fn run_crossing_2xx(order: Order) -> crate::Result<()> { let wait = Duration::from_secs(2); let (state_sender, mut states) = unbounded_channel(); - let invite = tokio::spawn(async move { dialog_layer.do_invite(option, state_sender).await }); + let layer = dialog_layer.clone(); + let invite = tokio::spawn(async move { layer.do_invite(option, state_sender).await }); let (inv, uac) = recv_request(&peer, Method::Invite, wait).await; - reply(&peer, uac, &inv, 180, "Ringing").await; - wait_for_state(&mut states, "Early", wait, |s| { - matches!(s, DialogState::Early(_, _)) + let id = calling_id(&mut states).await; + let reason = if provisional == 100 { + "Trying" + } else { + "Ringing" + }; + reply(&peer, uac, &inv, provisional, reason).await; + wait_for_state(&mut states, "Trying or Early", wait, |s| { + matches!(s, DialogState::Trying(_) | DialogState::Early(_, _)) }) .await; + if remove { + // The application removes the dialog from the layer first. + dialog_layer.remove_dialog(&id); + assert!(dialog_layer.is_empty()); + } // The application abandons the call: dropping the `do_invite` future // cancels the INVITE. invite.abort(); @@ -296,17 +321,25 @@ async fn run_crossing_2xx(order: Order) -> crate::Result<()> { #[tokio::test] async fn test_2xx_before_cancel_response_is_acked_and_byed() -> crate::Result<()> { - run_crossing_2xx(Order::InviteOkFirst).await + run_crossing_2xx(Order::InviteOkFirst, 180, false).await } #[tokio::test] async fn test_2xx_after_cancel_response_is_acked_and_byed() -> crate::Result<()> { - run_crossing_2xx(Order::CancelOkFirst).await + run_crossing_2xx(Order::CancelOkFirst, 180, false).await } #[tokio::test] async fn test_2xx_after_cancel_settle_window_is_acked_and_byed() -> crate::Result<()> { - run_crossing_2xx(Order::InviteOkLate).await + run_crossing_2xx(Order::InviteOkLate, 180, false).await +} + +/// The same when the application removed the dialog from the layer before +/// dropping the `do_invite` future, in Early (180) and in Trying (100). +#[tokio::test] +async fn test_removed_dialog_2xx_crossing_the_cancel_is_acked_and_byed() -> crate::Result<()> { + run_crossing_2xx(Order::InviteOkFirst, 180, true).await?; + run_crossing_2xx(Order::InviteOkFirst, 100, true).await } /// A CANCEL that wins the race (487 to the INVITE) ends the call as before: @@ -355,7 +388,7 @@ async fn test_cancel_answered_487_sends_no_bye() -> crate::Result<()> { /// Dropped before any response: Terminated(UacCancel) is reported at once and /// nothing is sent while no provisional has arrived (RFC 3261 §9.1). The first /// response then gets a CANCEL (180), or an ACK and a BYE (200). -async fn run_dropped_before_provisional(first: u16) -> crate::Result<()> { +async fn run_dropped_before_provisional(first: u16, remove: bool) -> crate::Result<()> { let token = CancellationToken::new(); let Uac { dialog_layer, @@ -365,8 +398,15 @@ async fn run_dropped_before_provisional(first: u16) -> crate::Result<()> { let wait = Duration::from_secs(2); let (state_sender, mut states) = unbounded_channel(); - let invite = tokio::spawn(async move { dialog_layer.do_invite(option, state_sender).await }); + let layer = dialog_layer.clone(); + let invite = tokio::spawn(async move { layer.do_invite(option, state_sender).await }); let (inv, uac) = recv_request(&peer, Method::Invite, wait).await; + let id = calling_id(&mut states).await; + if remove { + // The application removes the dialog from the layer first. + dialog_layer.remove_dialog(&id); + assert!(dialog_layer.is_empty()); + } invite.abort(); let _ = invite.await; let terminated = wait_for_state(&mut states, "Terminated", Duration::from_millis(200), |s| { @@ -418,15 +458,30 @@ async fn run_dropped_before_provisional(first: u16) -> crate::Result<()> { #[tokio::test] async fn test_dropped_before_provisional_is_cancelled_after_the_180() -> crate::Result<()> { - run_dropped_before_provisional(180).await + run_dropped_before_provisional(180, false).await } #[tokio::test] async fn test_dropped_before_provisional_2xx_is_acked_and_byed() -> crate::Result<()> { - run_dropped_before_provisional(200).await + run_dropped_before_provisional(200, false).await +} + +#[tokio::test] +async fn test_removed_before_provisional_is_cancelled_after_the_180() -> crate::Result<()> { + run_dropped_before_provisional(180, true).await +} + +#[tokio::test] +async fn test_removed_before_provisional_2xx_is_acked_and_byed() -> crate::Result<()> { + run_dropped_before_provisional(200, true).await } #[tokio::test] async fn test_dropped_before_provisional_final_failure_is_acked_only() -> crate::Result<()> { - run_dropped_before_provisional(486).await + run_dropped_before_provisional(486, false).await +} + +#[tokio::test] +async fn test_removed_before_provisional_final_failure_is_acked_only() -> crate::Result<()> { + run_dropped_before_provisional(486, true).await } From e524cc314adbae557c5f19fce08f4ebd5c441da2 Mon Sep 17 00:00:00 2001 From: jinti Date: Wed, 7 Oct 2026 21:00:46 +0800 Subject: [PATCH 11/16] fix(dialog): close two review nits in the recent state/notification fixes 1. A provisional response to an in-dialog request was still notified through the raw state_sender when the dialog had already terminated (the else arm of the #147 fix bypassed transition's Terminated guard). A 1xx racing a peer BYE could notify Early after Terminated, breaking the nothing-after-Terminated contract. The fallback now checks is_terminated first. 2. The forked-2xx drainer sent one BYE per received 2xx, so a forked 2xx retransmission in flight before its ACK landed produced duplicate BYEs. The drainer now tracks the forked tags it already ended and sends one BYE per branch. Both regressions are pinned by tests that fail without the fixes: test_provisional_after_terminated_is_not_notified and the retransmission window of test_forked_2xx_is_byed_without_touching_the_confirmed_dialog. --- src/dialog/dialog.rs | 6 +- src/dialog/invitation.rs | 9 +- src/dialog/invite_dialog.rs | 14 +++- src/dialog/tests/test_forked_2xx_bye.rs | 73 ++++++++++++---- .../tests/test_in_dialog_provisional.rs | 84 +++++++++++++++++++ 5 files changed, 165 insertions(+), 21 deletions(-) diff --git a/src/dialog/dialog.rs b/src/dialog/dialog.rs index c40e9680..b0e4aaae 100644 --- a/src/dialog/dialog.rs +++ b/src/dialog/dialog.rs @@ -1260,7 +1260,11 @@ impl DialogInner { let state = DialogState::Early(self.id.lock().clone(), resp); if self.can_cancel() { self.transition(state)?; - } else { + } else if !self.is_terminated() { + // Still notify the provisional so the caller sees + // it (e.g. a reliable 183 with SDP to a + // re-INVITE) — but never after the dialog + // terminated, matching `transition`'s contract. self.state_sender.send(state).ok(); } continue; diff --git a/src/dialog/invitation.rs b/src/dialog/invitation.rs index 96da705a..871b42eb 100644 --- a/src/dialog/invitation.rs +++ b/src/dialog/invitation.rs @@ -722,8 +722,10 @@ impl DialogLayer { let dlg = dialog.clone(); let confirmed_tag = new_dialog_id.remote_tag.clone(); crate::platform::spawn(async move { + let mut seen_forks: Vec = Vec::new(); while let Some(msg) = tx.receive().await { - dlg.end_forked_branch(&msg, &confirmed_tag).await; + dlg.end_forked_branch(&msg, &confirmed_tag, &mut seen_forks) + .await; } debug!(id = %new_dialog_id, "accepted transaction drained (Timer M expired)"); }); @@ -802,8 +804,11 @@ impl DialogLayer { let confirmed_tag = new_id.remote_tag.clone(); let forked_dlg = dialog_clone.clone(); crate::platform::spawn(async move { + let mut seen_forks: Vec = Vec::new(); while let Some(msg) = tx.receive().await { - forked_dlg.end_forked_branch(&msg, &confirmed_tag).await; + forked_dlg + .end_forked_branch(&msg, &confirmed_tag, &mut seen_forks) + .await; } debug!(id = %confirmed_id, "accepted transaction drained (Timer M expired)"); }); diff --git a/src/dialog/invite_dialog.rs b/src/dialog/invite_dialog.rs index 1bc4a364..cf771341 100644 --- a/src/dialog/invite_dialog.rs +++ b/src/dialog/invite_dialog.rs @@ -412,8 +412,14 @@ impl InviteDialog { /// tag differs from the confirmed dialog's is a forked branch: the /// transaction has already ACKed it, and the UAC — keeping a single /// session — terminates it with a BYE. Everything else (retransmitted - /// 2xx with the same tag, non-2xx) is ignored. - pub(super) async fn end_forked_branch(&self, msg: &SipMessage, confirmed_remote_tag: &str) { + /// 2xx with the same tag, non-2xx) is ignored. `seen` holds the forked + /// tags already BYE'd, so a retransmitted forked 2xx sends one BYE only. + pub(super) async fn end_forked_branch( + &self, + msg: &SipMessage, + confirmed_remote_tag: &str, + seen: &mut Vec, + ) { let SipMessage::Response(resp) = msg else { return; }; @@ -433,6 +439,10 @@ impl InviteDialog { if tag == confirmed_remote_tag { return; } + if seen.iter().any(|seen| seen == &tag) { + return; + } + seen.push(tag.clone()); let id = self.id(); info!( id = %id, diff --git a/src/dialog/tests/test_forked_2xx_bye.rs b/src/dialog/tests/test_forked_2xx_bye.rs index bd601edf..8d50f1fb 100644 --- a/src/dialog/tests/test_forked_2xx_bye.rs +++ b/src/dialog/tests/test_forked_2xx_bye.rs @@ -19,10 +19,14 @@ use tokio::sync::mpsc::unbounded_channel; use tokio_util::sync::CancellationToken; /// Receive the next request of `method` at `peer`. -async fn next_request(peer: &UdpSocket, method: Method, what: &str) -> Request { +async fn next_request( + peer: &UdpSocket, + method: Method, + what: &str, +) -> (Request, std::net::SocketAddr) { let mut buf = vec![0u8; 4096]; loop { - let (len, _) = tokio::time::timeout(Duration::from_secs(3), peer.recv_from(&mut buf)) + let (len, from) = tokio::time::timeout(Duration::from_secs(3), peer.recv_from(&mut buf)) .await .unwrap_or_else(|_| panic!("timeout waiting for the {what}")) .expect("peer socket error"); @@ -30,11 +34,29 @@ async fn next_request(peer: &UdpSocket, method: Method, what: &str) -> Request { continue; }; if req.method == method { - return req; + return (req, from); } } } +/// Answer a request the way the peer UA would (top Via honored). +async fn reply_ok( + peer: &UdpSocket, + req: &Request, + from: std::net::SocketAddr, +) -> crate::Result<()> { + let ok = format!( + "SIP/2.0 200 OK\r\nVia: {}\r\nFrom: {}\r\nTo: {}\r\nCall-ID: {}\r\nCSeq: {}\r\nContent-Length: 0\r\n\r\n", + req.via_header()?.value(), + req.from_header()?.value(), + req.to_header()?.value(), + req.call_id_header()?.value(), + req.cseq_header()?.value(), + ); + peer.send_to(ok.as_bytes(), from).await?; + Ok(()) +} + /// RFC 3261 §13.2.2.4: a forked 2xx (To tag `tag-b`) is ACKed with its own /// tag and remote target (the transaction's job, pinned here end to end) and /// then ended with a BYE built from its Contact — without touching the @@ -119,14 +141,14 @@ async fn test_forked_2xx_is_byed_without_touching_the_confirmed_dialog() -> crat // The first ACK confirms branch A; the forked 2xx is then ACKed with // its own tag and Contact as R-URI. - let mut ack = next_request(&peer, Method::Ack, "forked ACK").await; + let (mut ack, _) = next_request(&peer, Method::Ack, "forked ACK").await; while !ack .to_header() .ok() .and_then(|to| to.tag().ok().flatten()) .is_some_and(|tag| tag.value() == "tag-b") { - ack = next_request(&peer, Method::Ack, "forked ACK").await; + (ack, _) = next_request(&peer, Method::Ack, "forked ACK").await; } let ack_to = ack.to_header()?.value().to_string(); assert!(ack_to.contains(";tag=tag-b"), "ACK To: {ack_to}"); @@ -137,7 +159,7 @@ async fn test_forked_2xx_is_byed_without_touching_the_confirmed_dialog() -> crat ); // The forked branch is then ended with a BYE from its own Contact. - let bye = next_request(&peer, Method::Bye, "forked-branch BYE").await; + let (bye, from) = next_request(&peer, Method::Bye, "forked-branch BYE").await; let bye_to = bye.to_header()?.value().to_string(); assert!(bye_to.contains(";tag=tag-b"), "BYE To: {bye_to}"); assert!( @@ -151,6 +173,33 @@ async fn test_forked_2xx_is_byed_without_touching_the_confirmed_dialog() -> crat bye.call_id_header()?.value(), invite_req.call_id_header()?.value() ); + // The forked callee answers the BYE like a real UA would. + reply_ok(&peer, &bye, from).await?; + + // A retransmission of the forked 2xx (in flight before its ACK landed) + // must not trigger a second BYE. + peer.send_to(ok_b.as_bytes(), uac_addr).await?; + let mut buf = vec![0u8; 4096]; + let quiet_until = tokio::time::Instant::now() + Duration::from_millis(400); + loop { + let remaining = quiet_until.saturating_duration_since(tokio::time::Instant::now()); + if remaining.is_zero() { + break; + } + match tokio::time::timeout(remaining, peer.recv_from(&mut buf)).await { + Err(_) => break, // the quiet window elapsed: no duplicate BYE + Ok(Err(e)) => return Err(e.into()), + Ok(Ok((len, _))) => { + if let Ok(SipMessage::Request(req)) = SipMessage::try_from(&buf[..len]) { + assert_ne!( + req.method, + Method::Bye, + "duplicate BYE for a retransmitted forked 2xx" + ); + } + } + } + } // The confirmed dialog is untouched: no new state notifications, no // dialog registered for the forked branch, and the dialog still works. @@ -172,21 +221,13 @@ async fn test_forked_2xx_is_byed_without_touching_the_confirmed_dialog() -> crat // The confirmed dialog is still usable: a BYE ends it normally. let dialog2 = dialog.clone(); let bye_task = tokio::spawn(async move { dialog2.bye().await }); - let bye = next_request(&peer, Method::Bye, "confirmed-dialog BYE").await; + let (bye, _) = next_request(&peer, Method::Bye, "confirmed-dialog BYE").await; assert!( bye.to_header()?.value().to_string().contains(";tag=tag-a"), "the confirmed dialog's BYE keeps tag-a: {}", bye.to_header()?.value() ); - let ok_bye = format!( - "SIP/2.0 200 OK\r\nVia: {}\r\nFrom: {}\r\nTo: {}\r\nCall-ID: {}\r\nCSeq: {}\r\nContent-Length: 0\r\n\r\n", - bye.via_header()?.value(), - bye.from_header()?.value(), - bye.to_header()?.value(), - bye.call_id_header()?.value(), - bye.cseq_header()?.value(), - ); - peer.send_to(ok_bye.as_bytes(), uac_addr).await?; + reply_ok(&peer, &bye, uac_addr).await?; tokio::time::timeout(Duration::from_secs(3), bye_task) .await .expect("timeout waiting for bye()") diff --git a/src/dialog/tests/test_in_dialog_provisional.rs b/src/dialog/tests/test_in_dialog_provisional.rs index 40b82bbc..38d0e251 100644 --- a/src/dialog/tests/test_in_dialog_provisional.rs +++ b/src/dialog/tests/test_in_dialog_provisional.rs @@ -107,6 +107,22 @@ async fn establish( }); let dialog_layer = DialogLayer::new(endpoint.inner.clone()); + // Pump inbound in-dialog requests (e.g. the peer's BYE) into the layer. + let mut incoming = endpoint.incoming_transactions()?; + let pump_layer = DialogLayer { + endpoint: dialog_layer.endpoint.clone(), + inner: dialog_layer.inner.clone(), + }; + tokio::spawn(async move { + while let Some(mut tx) = incoming.recv().await { + if let Some(mut dialog) = pump_layer.match_dialog(&tx) { + tokio::spawn(async move { + let _ = dialog.handle(&mut tx).await; + }); + } + } + }); + let (state_sender, mut state_receiver) = unbounded_channel(); let invite_option = InviteOption { caller: Uri::try_from("sip:alice@example.com")?, @@ -216,3 +232,71 @@ async fn test_reinvite_provisional_keeps_dialog_confirmed() -> crate::Result<()> async fn test_update_provisional_keeps_dialog_confirmed() -> crate::Result<()> { assert_provisional_keeps_confirmed(Method::Update).await } + +/// A provisional response to an in-dialog request is never notified after +/// the dialog terminated: the peer hangs up while a re-INVITE is pending, +/// and only then answers it with a 183. +#[tokio::test] +async fn test_provisional_after_terminated_is_not_notified() -> crate::Result<()> { + let token = CancellationToken::new(); + let (dialog, mut states, peer) = establish(&token).await?; + while states.try_recv().is_ok() {} + + // A re-INVITE goes out and stays pending. + let requester = dialog.clone(); + let pending = tokio::spawn(async move { requester.reinvite(None, None).await }); + let (req, uac) = recv_request(&peer, Method::Invite).await; + + // The peer hangs up while the re-INVITE is pending. + let id = dialog.id(); + let uac_uri = format!("sip:alice@{uac}"); + let bye = format!( + "BYE {uac_uri} SIP/2.0\r\n\ + Via: SIP/2.0/UDP {peer_addr};branch=z9hG4bK-bye\r\n\ + Max-Forwards: 70\r\n\ + From: ;tag={PEER_TAG}\r\n\ + To: <{uac_uri}>;tag={local}\r\n\ + Call-ID: {call_id}\r\n\ + CSeq: 2 BYE\r\n\ + Content-Length: 0\r\n\r\n", + peer_addr = peer.local_addr()?, + local = id.local_tag, + call_id = id.call_id, + ); + peer.send_to(bye.as_bytes(), uac).await?; + // The dialog terminates and answers the BYE with a 200. + tokio::time::timeout(Duration::from_secs(2), async { + loop { + if matches!(states.recv().await, Some(DialogState::Terminated(..))) { + break; + } + } + }) + .await + .expect("timeout waiting for Terminated"); + assert!(dialog.state().is_terminated()); + + // Only now does the peer answer the re-INVITE — with a provisional. + reply(&peer, uac, &req, 183, "Session Progress").await; + tokio::time::sleep(Duration::from_millis(100)).await; + + let after: Vec = drain_states(&mut states) + .into_iter() + .map(|s| s.to_string()) + .collect(); + assert!( + after.is_empty(), + "nothing may be notified after Terminated, got {after:?}" + ); + + // Finish the pending re-INVITE so the transaction does not linger. + reply(&peer, uac, &req, 200, "OK").await; + tokio::time::timeout(Duration::from_secs(2), pending) + .await + .expect("re-INVITE did not complete") + .expect("re-INVITE task panicked")?; + + assert!(dialog.state().is_terminated()); + token.cancel(); + Ok(()) +} From 92cec74d11456479fd17fc472fc654b81527a82b Mon Sep 17 00:00:00 2001 From: jinti Date: Wed, 7 Oct 2026 22:27:35 +0800 Subject: [PATCH 12/16] test(dialog): pin the removed-dialog guard in the remaining CANCEL races MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #183. The removed-dialog variant of the drop guard is only exercised with a crossing 2xx in the InviteOkFirst ordering. Two gaps: - A CANCEL that wins the race (487 to the INVITE) with the dialog already removed: run_cancel_answered_487 now takes a provisional code and a remove flag, so test_removed_cancel_answered_487_sends_no_bye covers Early (180) and Trying (100) — CANCEL, ACK of the 487, no BYE, exactly one Terminated(UacCancel), never Confirmed. The pre-existing test gains the same Terminated/Confirmed assertions. - The CancelOkFirst wire ordering (200 to the CANCEL before the 2xx) with the dialog removed: test_removed_dialog_2xx_after_the_cancel_response_is_acked_and_byed. --- src/dialog/tests/test_cancel_2xx_race.rs | 74 +++++++++++++++++++++--- 1 file changed, 66 insertions(+), 8 deletions(-) diff --git a/src/dialog/tests/test_cancel_2xx_race.rs b/src/dialog/tests/test_cancel_2xx_race.rs index 96a5cbd2..946a01cc 100644 --- a/src/dialog/tests/test_cancel_2xx_race.rs +++ b/src/dialog/tests/test_cancel_2xx_race.rs @@ -5,7 +5,10 @@ //! //! These tests drop the `do_invite` future after a 180 — the documented way to //! abandon an outgoing call, which cancels it — and answer the INVITE with a -//! 200 from a raw UDP peer in each wire ordering. +//! 200 from a raw UDP peer in each wire ordering. The `removed` variants call +//! `DialogLayer::remove_dialog` before dropping the future: the INVITE still +//! has to end (CANCEL, and ACK + BYE for a 2xx) even though the layer entry is +//! already gone. use crate::dialog::{ dialog::{DialogState, DialogStateReceiver, TerminatedReason}, dialog_layer::DialogLayer, @@ -342,10 +345,18 @@ async fn test_removed_dialog_2xx_crossing_the_cancel_is_acked_and_byed() -> crat run_crossing_2xx(Order::InviteOkFirst, 100, true).await } -/// A CANCEL that wins the race (487 to the INVITE) ends the call as before: -/// the 487 is ACKed and no BYE is sent. +/// The same in the other wire ordering (the 200 to the CANCEL arrives before +/// the 2xx) with the dialog already removed from the layer. #[tokio::test] -async fn test_cancel_answered_487_sends_no_bye() -> crate::Result<()> { +async fn test_removed_dialog_2xx_after_the_cancel_response_is_acked_and_byed() -> crate::Result<()> +{ + run_crossing_2xx(Order::CancelOkFirst, 180, true).await +} + +/// A CANCEL that wins the race (487 to the INVITE) ends the call as before: +/// the 487 is ACKed and no BYE is sent — also when the application removed +/// the dialog from the layer before dropping the `do_invite` future. +async fn run_cancel_answered_487(provisional: u16, remove: bool) -> crate::Result<()> { let token = CancellationToken::new(); let Uac { dialog_layer, @@ -355,13 +366,26 @@ async fn test_cancel_answered_487_sends_no_bye() -> crate::Result<()> { let wait = Duration::from_secs(2); let (state_sender, mut states) = unbounded_channel(); - let invite = tokio::spawn(async move { dialog_layer.do_invite(option, state_sender).await }); + let layer = dialog_layer.clone(); + let invite = tokio::spawn(async move { layer.do_invite(option, state_sender).await }); let (inv, uac) = recv_request(&peer, Method::Invite, wait).await; - reply(&peer, uac, &inv, 180, "Ringing").await; - wait_for_state(&mut states, "Early", wait, |s| { - matches!(s, DialogState::Early(_, _)) + let id = calling_id(&mut states).await; + let reason = if provisional == 100 { + "Trying" + } else { + "Ringing" + }; + reply(&peer, uac, &inv, provisional, reason).await; + wait_for_state(&mut states, "Trying or Early", wait, |s| { + matches!(s, DialogState::Trying(_) | DialogState::Early(_, _)) }) .await; + + if remove { + // The application removes the dialog from the layer first. + dialog_layer.remove_dialog(&id); + assert!(dialog_layer.is_empty()); + } invite.abort(); let _ = invite.await; let (cancel, _) = recv_request(&peer, Method::Cancel, wait).await; @@ -381,10 +405,44 @@ async fn test_cancel_answered_487_sends_no_bye() -> crate::Result<()> { "a cancelled call must not be BYE'd", ) .await; + + // Exactly one Terminated(UacCancel), never Confirmed. + tokio::time::sleep(Duration::from_millis(200)).await; + let seen: Vec<_> = std::iter::from_fn(|| states.try_recv().ok()).collect(); + assert!( + !seen + .iter() + .any(|s| matches!(s, DialogState::Confirmed(_, _))), + "a cancelled call must not report Confirmed, got {seen:?}" + ); + let terminations: Vec<_> = seen + .iter() + .filter(|s| matches!(s, DialogState::Terminated(_, _))) + .collect(); + assert!( + matches!( + terminations.as_slice(), + [DialogState::Terminated(_, TerminatedReason::UacCancel)] + ), + "expected exactly one Terminated(UacCancel), got {seen:?}" + ); token.cancel(); Ok(()) } +#[tokio::test] +async fn test_cancel_answered_487_sends_no_bye() -> crate::Result<()> { + run_cancel_answered_487(180, false).await +} + +/// The same when the application removed the dialog from the layer before +/// dropping the `do_invite` future, in Early (180) and in Trying (100). +#[tokio::test] +async fn test_removed_cancel_answered_487_sends_no_bye() -> crate::Result<()> { + run_cancel_answered_487(180, true).await?; + run_cancel_answered_487(100, true).await +} + /// Dropped before any response: Terminated(UacCancel) is reported at once and /// nothing is sent while no provisional has arrived (RFC 3261 §9.1). The first /// response then gets a CANCEL (180), or an ACK and a BYE (200). From ea33e2031f5442a69458dd84ef73cd5183cf2466 Mon Sep 17 00:00:00 2001 From: Tyson George Date: Wed, 7 Oct 2026 10:40:55 -0400 Subject: [PATCH 13/16] fix(dialog): end the session on a never-ACKed re-INVITE 2xx in ClientInviteDialog The deprecated ClientInviteDialog has its own handle_reinvite, which missed the teardown that InviteDialog and ServerInviteDialog apply (RFC 3261 13.3.1.4): when the callee re-INVITEs a UAC dialog handled through the wrapper and never ACKs the 2xx, the 2xx was retransmitted until 64*T1 and then nothing happened. The dialog stayed Confirmed, with no event and no BYE. Track the 2xx and the ACK as the other two handlers do and call end_session_without_ack, so the dialog ends with TerminatedReason::Timeout and a BYE. --- src/dialog/client_dialog.rs | 9 ++ src/dialog/tests/test_uas_ack_timeout.rs | 115 +++++++++++++++++++++++ 2 files changed, 124 insertions(+) diff --git a/src/dialog/client_dialog.rs b/src/dialog/client_dialog.rs index 2cb34453..dafa0b7b 100644 --- a/src/dialog/client_dialog.rs +++ b/src/dialog/client_dialog.rs @@ -668,6 +668,11 @@ impl ClientInviteDialog { .transition(DialogState::Updated(self.id(), tx.original.clone(), handle))?; self.inner.process_transaction_handle(tx, rx).await?; + let answered_2xx = tx + .last_response + .as_ref() + .is_some_and(|resp| resp.status_code.kind() == crate::sip::StatusCodeKind::Successful); + let mut acked = false; // wait for ACK while let Some(msg) = tx.receive().await { @@ -675,11 +680,15 @@ impl ClientInviteDialog { SipMessage::Request(req) if req.method == crate::sip::Method::Ack => { debug!(id = %self.id(), "received ACK for re-INVITE"); self.inner.remote_ack.lock().replace(req); + acked = true; break; } _ => {} } } + self.inner + .end_session_without_ack(tx, answered_2xx && !acked) + .await; Ok(()) } diff --git a/src/dialog/tests/test_uas_ack_timeout.rs b/src/dialog/tests/test_uas_ack_timeout.rs index 23b43daf..e6f73224 100644 --- a/src/dialog/tests/test_uas_ack_timeout.rs +++ b/src/dialog/tests/test_uas_ack_timeout.rs @@ -958,3 +958,118 @@ async fn test_acked_reinvite_sends_no_bye() -> crate::Result<()> { token.cancel(); Ok(()) } + +/// The deprecated `ClientInviteDialog` has its own re-INVITE handler: a UAC +/// dialog re-INVITEd by the callee must end the same way as `InviteDialog`. +async fn legacy_client_dialog_reinvite(ack: bool) -> crate::Result<()> { + use crate::dialog::{client_dialog::ClientInviteDialog, invitation::InviteOption}; + let token = CancellationToken::new(); + let transport_layer = TransportLayer::new(token.child_token()); + let udp = UdpConnection::create_connection( + "127.0.0.1:0".parse().unwrap(), + None, + Some(token.child_token()), + ) + .await?; + let uac: SocketAddr = udp.get_addr().get_socketaddr()?; + transport_layer.add_transport(udp.into()); + let endpoint = EndpointBuilder::new() + .with_transport_layer(transport_layer) + .with_cancel_token(token.child_token()) + .with_option(short_timers()) + .build(); + let dialog_layer = Arc::new(DialogLayer::new(endpoint.inner.clone())); + let mut incoming = endpoint.incoming_transactions()?; + let endpoint_inner = endpoint.inner.clone(); + tokio::spawn(async move { endpoint_inner.serve().await }); + let layer = dialog_layer.clone(); + tokio::spawn(async move { + while let Some(mut tx) = incoming.recv().await { + if let Some(Dialog::Invite(dialog)) = layer.match_dialog(&tx) { + let mut legacy = ClientInviteDialog::try_from(dialog).expect("a UAC dialog"); + tokio::spawn(async move { legacy.handle(&mut tx).await }); + } + } + }); + let peer = Peer { + socket: UdpSocket::bind("127.0.0.1:0").await?, + uas: uac, + }; + let callee = peer.socket.local_addr()?; + let option = InviteOption { + caller: format!("sip:alice@{uac}").as_str().try_into()?, + callee: format!("sip:bob@{callee}").as_str().try_into()?, + contact: format!("sip:alice@{uac}").as_str().try_into()?, + call_id: Some(CALL_ID.to_string()), + ..Default::default() + }; + let (state_sender, mut states) = unbounded_channel(); + let invite = tokio::spawn(async move { dialog_layer.do_invite(option, state_sender).await }); + let mut buf = vec![0u8; 4096]; + let (len, _) = tokio::time::timeout(Duration::from_secs(2), peer.socket.recv_from(&mut buf)) + .await + .expect("timeout waiting for the INVITE")?; + let SipMessage::Request(req) = SipMessage::try_from(std::str::from_utf8(&buf[..len]).unwrap())? + else { + panic!("expected the INVITE"); + }; + peer.send(format!( + "SIP/2.0 200 OK\r\nVia: {}\r\nFrom: {}\r\nTo: {};tag={FROM_TAG}\r\nCall-ID: {CALL_ID}\r\n\ + CSeq: 1 INVITE\r\nContact: \r\nContent-Length: 0\r\n\r\n", + req.via_header()?.value(), + req.from_header()?.value(), + req.to_header()?.value(), + )) + .await; + let (dialog, _) = invite.await.unwrap()?; + let local_tag = dialog.id().local_tag; + + // The callee re-INVITEs and the application answers it. + peer.send_request(Method::Invite, 7, Some(&local_tag)).await; + let handle = loop { + match tokio::time::timeout(Duration::from_secs(2), states.recv()).await { + Ok(Some(DialogState::Updated(_, _, handle))) => break handle, + Ok(Some(_)) => {} + _ => panic!("timeout waiting for the re-INVITE"), + } + }; + let answered = Instant::now(); + handle.reply(crate::sip::StatusCode::OK).await.ok(); + let mut messages = Vec::new(); + while !messages.iter().any(|(_, m)| is_2xx_to(m, 7)) { + assert!(answered.elapsed() < T1X64, "timeout waiting for the 2xx"); + messages.extend(peer.collect(Instant::now() + T1 / 2).await); + } + if ack { + peer.send_request(Method::Ack, 7, Some(&local_tag)).await; + } + messages.extend(peer.collect(answered + T1X64 * 2).await); + let bye = messages + .iter() + .any(|(_, m)| matches!(m, SipMessage::Request(r) if r.method == Method::Bye)); + if ack { + assert!(!bye, "an ACKed re-INVITE must not end the session"); + assert!(terminated_reason(&mut states).is_none()); + assert!(dialog.state().is_confirmed()); + } else { + assert!(messages.iter().filter(|(_, m)| is_2xx_to(m, 7)).count() >= 2); + assert!(bye_of_dialog(&messages, &local_tag) - answered >= T1X64 - T1); + assert!(matches!( + terminated_reason(&mut states), + Some(TerminatedReason::Timeout) + )); + } + token.cancel(); + Ok(()) +} + +#[tokio::test] +async fn test_unacked_reinvite_2xx_ends_the_session_on_a_legacy_client_dialog() -> crate::Result<()> +{ + legacy_client_dialog_reinvite(false).await +} + +#[tokio::test] +async fn test_acked_reinvite_keeps_a_legacy_client_dialog() -> crate::Result<()> { + legacy_client_dialog_reinvite(true).await +} From c678342d9bb3b89e58657d4f95b1333c57b283b7 Mon Sep 17 00:00:00 2001 From: Tyson George Date: Wed, 7 Oct 2026 12:37:43 -0400 Subject: [PATCH 14/16] fix(dialog): a server in-dialog request with no route and no dial-back fails at once When the target locator fails, Transaction::send returns before the transaction enters Calling, so no timer is armed and nothing is ever sent. A server dialog then looks for a dial-back address; with none it went on to wait in tx.receive() for a response that cannot come, and BYE, re-INVITE, INFO and every other in-dialog request never returned. Return the send error in that case, as a client dialog already does. The dial-back retry is unchanged. --- src/dialog/dialog.rs | 7 +++ src/dialog/tests/test_connection_affinity.rs | 63 ++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/src/dialog/dialog.rs b/src/dialog/dialog.rs index b0e4aaae..d59152ed 100644 --- a/src/dialog/dialog.rs +++ b/src/dialog/dialog.rs @@ -1134,6 +1134,7 @@ impl DialogInner { } } let need_fallback_retry; + let mut send_error = None; match tx.send().await { Ok(_) => { debug!( @@ -1161,6 +1162,7 @@ impl DialogInner { debug!(id = self.id.lock().to_string(), req = %tx.original, "request that failed to send"); return Err(e); } + send_error = Some(e); } } @@ -1232,6 +1234,11 @@ impl DialogInner { method = %method, "no usable connection and no dial-back target; giving up after first send" ); + // The failed send never started the transaction: no response + // or timer will ever end it, so report the error now. + if let Some(e) = send_error { + return Err(e); + } } } diff --git a/src/dialog/tests/test_connection_affinity.rs b/src/dialog/tests/test_connection_affinity.rs index 489c0883..494276c7 100644 --- a/src/dialog/tests/test_connection_affinity.rs +++ b/src/dialog/tests/test_connection_affinity.rs @@ -527,6 +527,69 @@ async fn test_restored_dialog_falls_back_to_initial_via_dialback() { } } +/// Fails every lookup, as a registrar-backed locator does once the target is gone. +struct NoRoute; + +#[async_trait::async_trait] +impl crate::transaction::endpoint::TargetLocator for NoRoute { + async fn locate(&self, _: &crate::sip::Uri) -> crate::Result { + Err(crate::Error::Error("no route".to_string())) + } +} + +#[tokio::test] +async fn test_server_request_without_route_fails_unless_dialed_back() -> crate::Result<()> { + // The locator error fails the send before the transaction starts. With no + // dial-back address nothing will ever end that transaction, so the request + // must return the error at once; with one, it is dialed back as before. + let tl = TransportLayer::new(CancellationToken::new()); + let udp = UdpConnection::create_connection("127.0.0.1:0".parse()?, None, None).await?; + tl.add_transport(udp.into()); + let endpoint = EndpointBuilder::new() + .with_transport_layer(tl) + .with_target_locator(Box::new(NoRoute)) + .build(); + let confirmed = |call_id: &str, via: &str| { + let invite = plain_udp_invite("alice-tag", call_id, via); + let (ep, role) = (endpoint.inner.clone(), TransactionRole::Server); + let inner = server_dialog(ep, role, invite, "sip:bob@127.0.0.1:5060"); + let id = inner.id.lock().clone(); + inner + .transition(DialogState::Confirmed(id, Response::default())) + .unwrap(); + InviteDialog::from_inner(Arc::new(inner)) + }; + + let dialog = confirmed("no-route", "SIP/2.0/UDP a.invalid;branch=z9hG4bKnoroute"); + for method in ["INFO", "re-INVITE", "BYE"] { + let request = async { + match method { + "INFO" => dialog.info(None, None).await.map(|_| ()), + "re-INVITE" => dialog.reinvite(None, None).await.map(|_| ()), + _ => dialog.bye().await, + } + }; + let result = tokio::time::timeout(Duration::from_secs(5), request).await; + let err = result.unwrap_or_else(|_| panic!("{method} must fail at once, not hang")); + let err = err.expect_err(method).to_string(); + assert!(err.contains("no route"), "{method}: {err}"); + } + + let probe = UdpSocket::bind("127.0.0.1:0").await?; + let via = format!( + "SIP/2.0/UDP alice.invalid:5060;branch=z9hG4bKdialback;received=127.0.0.1;rport={}", + probe.local_addr()?.port() + ); + let dialog = confirmed("dialback", &via); + tokio::spawn(async move { dialog.bye().await }); + let mut buf = [0u8; 2048]; + let (len, _) = tokio::time::timeout(Duration::from_secs(3), probe.recv_from(&mut buf)) + .await + .expect("the BYE must be dialed back to the Via's received/rport")?; + assert!(buf[..len].starts_with(b"BYE ")); + Ok(()) +} + /// A cancelled (dead) WebSocket flow must not cause affinity retransmissions /// into a dead socket, and the dial-back ladder must still deliver the BYE /// through tier 1 — the address captured from the connection at creation — From 3bf17f99bf666c106e9ce218725ed7a2cffa79e1 Mon Sep 17 00:00:00 2001 From: Tyson George Date: Wed, 7 Oct 2026 14:36:16 -0400 Subject: [PATCH 15/16] fix(transaction): Timer F ends a non-INVITE client transaction in Proceeding A non-INVITE client transaction runs Timer F as Timer B, and on_timer handled Timer B only in Calling and Trying. After a provisional other than 100 (e.g. 180 to a BYE) the transaction is in Proceeding, Timer F was ignored there, and a request answered with one 1xx and then nothing never ended. RFC 3261 section 17.1.2.2: if Timer F fires in Proceeding, the TU must be informed of a timeout and the transaction terminated. Handle Timer B for a non-INVITE client in Proceeding like Timer C: a local 408 to the TU, which terminates the transaction. --- src/transaction/tests/test_client.rs | 81 ++++++++++++++++++++++++++++ src/transaction/transaction.rs | 7 ++- 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/src/transaction/tests/test_client.rs b/src/transaction/tests/test_client.rs index 70b9cd52..64ac6c9f 100644 --- a/src/transaction/tests/test_client.rs +++ b/src/transaction/tests/test_client.rs @@ -473,3 +473,84 @@ async fn test_invite_2xx_upstream_via_delivery_and_ack() -> Result<()> { } Ok(()) } + +/// RFC 3261 §17.1.2.2: Timer F still runs after a provisional response. A +/// BYE answered with one 1xx and then nothing must end with a 408 at 64*T1, +/// on unreliable and reliable transports alike. +#[tokio::test] +async fn test_non_invite_timer_f_after_provisional() -> Result<()> { + use crate::transaction::{endpoint::EndpointOption, EndpointBuilder}; + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + use tokio::net::{TcpListener, UdpSocket}; + + // The request with its start line swapped for a provisional status line. + fn provisional(code: u16, request: &[u8]) -> Vec { + let text = String::from_utf8_lossy(request); + let (_, headers) = text.split_once("\r\n").unwrap(); + format!("SIP/2.0 {code} Provisional\r\n{headers}").into_bytes() + } + + for (code, tcp) in [(100, false), (180, false), (183, true)] { + let tl = crate::transport::TransportLayer::new(Default::default()); + let udp = UdpConnection::create_connection("127.0.0.1:0".parse()?, None, None).await?; + tl.add_transport(udp.into()); + let t1 = Duration::from_millis(20); + let option = EndpointOption { + t1, + t1x64: t1 * 64, + ..Default::default() + }; + let endpoint = EndpointBuilder::new() + .with_transport_layer(tl) + .with_option(option) + .build(); + + // The peer answers the BYE with one provisional, then stays silent. + let socket = UdpSocket::bind("127.0.0.1:0").await?; + let listener = TcpListener::bind("127.0.0.1:0").await?; + let uri = match tcp { + true => format!("sip:bob@{};transport=tcp", listener.local_addr()?), + false => format!("sip:bob@{}", socket.local_addr()?), + }; + let peer = tokio::spawn(async move { + let mut buf = vec![0u8; 4096]; + if tcp { + let (mut stream, _) = listener.accept().await?; + let mut len = 0; + while !buf[..len].windows(4).any(|w| w == b"\r\n\r\n") { + len += stream.read(&mut buf[len..]).await?; + } + stream.write_all(&provisional(code, &buf[..len])).await?; + std::future::pending::<()>().await; // keep the connection open + } else { + let (len, src) = socket.recv_from(&mut buf).await?; + socket.send_to(&provisional(code, &buf[..len]), src).await?; + } + std::future::pending::>().await + }); + + let mut bye = make_invite_request(&uri)?; + bye.method = crate::sip::Method::Bye; + bye.headers.unique_push(CSeq::new("2 BYE").into()); + let key = TransactionKey::from_request(&bye, TransactionRole::Client)?; + let mut tx = Transaction::new_client(key, bye, endpoint.inner.clone(), None); + let mut codes = vec![]; + let run = async { + tx.send().await?; + while let Some(SipMessage::Response(resp)) = tx.receive().await { + codes.push(resp.status_code.code()); + } + Ok::<_, crate::Error>(()) + }; + // `receive` returning None means the transaction terminated. + let terminated = select! { + r = run => r.map(|_| true)?, + _ = endpoint.serve() => panic!("endpoint stopped"), + _ = sleep(t1 * 64 * 3) => false, + }; + peer.abort(); + assert_eq!(codes, [code, 408], "{code} over tcp={tcp}"); + assert!(terminated, "{code} over tcp={tcp}: not terminated"); + } + Ok(()) +} diff --git a/src/transaction/transaction.rs b/src/transaction/transaction.rs index 96092137..4c70e766 100644 --- a/src/transaction/transaction.rs +++ b/src/transaction/transaction.rs @@ -1161,7 +1161,12 @@ impl Transaction { } } TransactionState::Proceeding => { - if let TransactionTimer::TimerC(_) = timer { + // Timer C (client INVITE), or Timer F, run as Timer B, for a + // non-INVITE client (RFC 3261 §17.1.2.2). + if matches!(timer, TransactionTimer::TimerC(_)) + || (matches!(timer, TransactionTimer::TimerB(_)) + && self.transaction_type == TransactionType::ClientNonInvite) + { // Inform TU about timeout let timeout_response = self.endpoint_inner.make_response( &self.original, From 2dfa0d17ce68be3f48f8afe99dff8d8c9eba76a7 Mon Sep 17 00:00:00 2001 From: jinti Date: Thu, 8 Oct 2026 10:05:47 +0800 Subject: [PATCH 16/16] chore: bump version to 0.7.3 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index a337a4dd..2cf9dd1a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rsipstack" -version = "0.7.2" +version = "0.7.3" edition = "2021" description = "SIP Stack Rust library for building SIP applications" license = "MIT"