diff --git a/src/lib.rs b/src/lib.rs index 5b0215a..6556527 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -208,7 +208,13 @@ //! //! * **RFC 3261** - SIP: Session Initiation Protocol (core specification) //! * **RFC 3581** - Symmetric Response Routing (rport) -//! * **RFC 6026** - Correct Transaction Handling for 2xx Responses to INVITE +//! * **RFC 6026** - Correct Transaction Handling for 2xx Responses to INVITE. +//! Server `Accepted` state + Timer L (§7.1); client `Accepted` state + Timer M +//! (§7.2); server transactions do not retransmit 2xx (§7.1). Client-side ACK +//! for 2xx is currently auto-issued by the transaction layer for rsipstack +//! 0.5.x backward compatibility — strict §7.2 + RFC 3261 §17.1.1.3 places +//! that responsibility on the TU and the auto-ACK is a candidate for +//! migration to the dialog layer in a follow-up release. //! //! ## Performance //! diff --git a/src/transaction/mod.rs b/src/transaction/mod.rs index fc6c862..1c49761 100644 --- a/src/transaction/mod.rs +++ b/src/transaction/mod.rs @@ -31,7 +31,8 @@ pub type TransactionSender = UnboundedSender; /// /// `TransactionState` represents the various states a SIP transaction can be in /// during its lifecycle. These states implement the transaction state machines -/// defined in RFC 3261 for both client and server transactions. +/// defined in RFC 3261 for both client and server transactions, with the RFC 6026 +/// `Accepted` state for INVITE 2xx response handling. /// /// # States /// @@ -39,29 +40,39 @@ pub type TransactionSender = UnboundedSender; /// * `Calling` - Initial state for client transactions when request is sent or received /// * `Trying` - Request has been sent/received, waiting for response/processing /// * `Proceeding` - Provisional response received/sent (1xx except 100 Trying) -/// * `Completed` - Final response received/sent, waiting for ACK (INVITE) or cleanup -/// * `Confirmed` - ACK received/sent for INVITE transactions +/// * `Accepted` - INVITE transaction received/sent a 2xx final response (RFC 6026 §7.1/§7.2); +/// server transaction waits for ACKs and absorbs 2xx retransmissions from the TU until +/// Timer L fires; client transaction absorbs 2xx retransmissions from the server until +/// Timer M fires. +/// * `Completed` - Final non-2xx response received/sent (RFC 3261 §17), waiting for ACK (server INVITE) +/// or response retransmissions (client). For INVITE 2xx, see `Accepted` (RFC 6026 supersedes RFC 3261 §17.2.1 paragraph 4 / §17.1.1.2). +/// * `Confirmed` - ACK received/sent for INVITE non-2xx (3xx-6xx) transactions /// * `Terminated` - Transaction has completed and is being cleaned up /// /// # State Transitions /// -/// ## Client Non-INVITE Transaction +/// ## Client Non-INVITE Transaction (RFC 3261 §17.1.2) /// ```text /// Nothing → Calling → Trying → Proceeding → Completed → Terminated /// ``` /// -/// ## Client INVITE Transaction +/// ## Client INVITE Transaction (RFC 3261 §17.1.1 + RFC 6026 §7.2) /// ```text -/// Nothing → Calling → Trying → Proceeding → Completed → Terminated -/// ↓ -/// Confirmed → Terminated +/// Nothing → Calling → Trying → Proceeding ──2xx──→ Accepted ──Timer M──→ Terminated +/// │ +/// └──3xx-6xx──→ Completed → Confirmed → Terminated +/// ``` +/// +/// ## Server INVITE Transaction (RFC 3261 §17.2.1 + RFC 6026 §7.1) +/// ```text +/// Calling → Trying → Proceeding ──2xx──→ Accepted ──Timer L──→ Terminated +/// │ +/// └──3xx-6xx──→ Completed ──ACK──→ Confirmed → Terminated /// ``` /// -/// ## Server Transactions +/// ## Server Non-INVITE Transaction (RFC 3261 §17.2.2) /// ```text /// Calling → Trying → Proceeding → Completed → Terminated -/// ↓ -/// Confirmed → Terminated (INVITE only) /// ``` /// /// # Examples @@ -75,17 +86,28 @@ pub type TransactionSender = UnboundedSender; /// TransactionState::Calling => println!("Request sent"), /// TransactionState::Trying => println!("Request sent/received"), /// TransactionState::Proceeding => println!("Provisional response"), -/// TransactionState::Completed => println!("Final response"), +/// TransactionState::Accepted => println!("2xx accepted; waiting for Timer L/M"), +/// TransactionState::Completed => println!("Final non-2xx response"), /// TransactionState::Confirmed => println!("ACK received/sent"), /// TransactionState::Terminated => println!("Transaction complete"), +/// // `#[non_exhaustive]`: downstream code MUST keep a wildcard arm to +/// // remain forward-compatible with future state additions. +/// _ => println!("future RFC-extension state"), /// } /// ``` #[derive(Debug, Clone, PartialEq)] +#[non_exhaustive] pub enum TransactionState { Nothing, Calling, Trying, Proceeding, + /// RFC 6026 §7.1 (server) / §7.2 (client): an INVITE transaction has sent or received + /// a 2xx final response. The server absorbs 2xx retransmissions from the TU and waits + /// for ACK until Timer L (= 64*T1) fires; the client absorbs server-retransmitted 2xx + /// responses until Timer M (= 64*T1) fires. Replaces the RFC 3261 §17.1.1.2 / §17.2.1 + /// behaviour that incorrectly routed 2xx through the `Completed` state. + Accepted, Completed, Confirmed, Terminated, @@ -98,6 +120,7 @@ impl std::fmt::Display for TransactionState { TransactionState::Calling => write!(f, "Calling"), TransactionState::Trying => write!(f, "Trying"), TransactionState::Proceeding => write!(f, "Proceeding"), + TransactionState::Accepted => write!(f, "Accepted"), TransactionState::Completed => write!(f, "Completed"), TransactionState::Confirmed => write!(f, "Confirmed"), TransactionState::Terminated => write!(f, "Terminated"), @@ -174,21 +197,22 @@ impl std::fmt::Display for TransactionType { /// SIP Transaction Timers /// /// `TransactionTimer` represents the various timers used in SIP transactions -/// as defined in RFC 3261. These timers ensure reliable message delivery -/// and proper transaction cleanup. +/// as defined in RFC 3261 and RFC 6026. These timers ensure reliable message +/// delivery and proper transaction cleanup. /// /// # Timer Types /// /// * `TimerA` - Retransmission timer for client transactions (unreliable transport) /// * `TimerB` - Transaction timeout timer for client transactions +/// * `TimerC` - Proceeding timeout for client INVITE /// * `TimerD` - Wait timer for response retransmissions (client) -/// * `TimerE` - Retransmission timer for non-INVITE server transactions -/// * `TimerF` - Transaction timeout timer for non-INVITE server transactions -/// * `TimerK` - Wait timer for ACK (server INVITE transactions) -/// * `TimerG` - Retransmission timer for INVITE server transactions +/// * `TimerK` - Wait timer for ACK (server INVITE non-2xx) / cleanup (client non-INVITE) +/// * `TimerG` - Retransmission timer for INVITE server transactions (non-2xx only per RFC 6026 §7.1) +/// * `TimerL` - RFC 6026 §7.1 server INVITE Accepted-state timer (64*T1) +/// * `TimerM` - RFC 6026 §7.2 client INVITE Accepted-state timer (64*T1) /// * `TimerCleanup` - Internal cleanup timer for transaction removal /// -/// # Timer Values (RFC 3261) +/// # Timer Values (RFC 3261 + RFC 6026) /// /// * T1 = 500ms (RTT estimate) /// * T2 = 4s (maximum retransmit interval) @@ -200,8 +224,10 @@ impl std::fmt::Display for TransactionType { /// * Timer D: 32 seconds for unreliable, 0 for reliable transports /// * Timer E: starts at T1, doubles up to T2 /// * Timer F: 64*T1 (32 seconds) -/// * Timer G: starts at T1, doubles up to T2 +/// * Timer G: starts at T1, doubles up to T2 (non-2xx final response retransmits only) /// * Timer K: T4 for unreliable, 0 for reliable transports +/// * **Timer L: 64*T1 (32 seconds) — RFC 6026 §7.1** +/// * **Timer M: 64*T1 (32 seconds) — RFC 6026 §7.2** /// /// # Examples /// @@ -233,6 +259,9 @@ impl std::fmt::Display for TransactionType { /// TransactionTimer::TimerB(key) => { /// println!("Transaction {} timed out", key); /// }, +/// // `#[non_exhaustive]`: downstream code MUST keep a wildcard arm to +/// // remain forward-compatible with future timer additions (e.g. RFC +/// // extensions like Timer L / Timer M / future RFC variants). /// _ => {} /// } /// # Ok(()) @@ -246,6 +275,7 @@ impl std::fmt::Display for TransactionType { /// * Cancelled when leaving states or receiving responses /// * Fire events that drive state machine transitions /// * Handle retransmissions and timeouts +#[non_exhaustive] pub enum TransactionTimer { TimerA(TransactionKey, Duration), TimerB(TransactionKey), @@ -253,6 +283,17 @@ pub enum TransactionTimer { TimerD(TransactionKey), TimerK(TransactionKey), TimerG(TransactionKey, Duration), + /// RFC 6026 §7.1: server INVITE Accepted-state timer. Fires once at 64*T1 + /// after a 2xx final response is sent, then transitions the transaction to + /// Terminated. Absorbs ACKs for the 2xx and late 2xx retransmissions from + /// the TU. Replaces the RFC 3261 §17.2.1 Timer K (T4) usage for 2xx + /// responses, which was too short to handle proxy-chain ACK fan-in. + TimerL(TransactionKey), + /// RFC 6026 §7.2: client INVITE Accepted-state timer. Fires once at 64*T1 + /// after a 2xx final response is received, then transitions the + /// transaction to Terminated. Absorbs server-retransmitted 2xx responses + /// (the TU is responsible for the ACK). + TimerM(TransactionKey), TimerCleanup(TransactionKey), } @@ -265,6 +306,8 @@ impl TransactionTimer { TransactionTimer::TimerD(key) => key, TransactionTimer::TimerG(key, _) => key, TransactionTimer::TimerK(key) => key, + TransactionTimer::TimerL(key) => key, + TransactionTimer::TimerM(key) => key, TransactionTimer::TimerCleanup(key) => key, } } @@ -283,6 +326,8 @@ impl std::fmt::Display for TransactionTimer { write!(f, "TimerG: {} {}", key, duration.as_millis()) } TransactionTimer::TimerK(key) => write!(f, "TimerK: {}", key), + TransactionTimer::TimerL(key) => write!(f, "TimerL: {}", key), + TransactionTimer::TimerM(key) => write!(f, "TimerM: {}", key), TransactionTimer::TimerCleanup(key) => write!(f, "TimerCleanup: {}", key), } } diff --git a/src/transaction/tests/test_transaction_states.rs b/src/transaction/tests/test_transaction_states.rs index ebf5fe8..4421891 100644 --- a/src/transaction/tests/test_transaction_states.rs +++ b/src/transaction/tests/test_transaction_states.rs @@ -8,8 +8,10 @@ use crate::sip::headers::*; use crate::transaction::{ key::{TransactionKey, TransactionRole}, transaction::Transaction, - TransactionState, TransactionType, + TransactionState, TransactionTimer, TransactionType, }; +use crate::transport::udp::UdpConnection; +use crate::transport::SipConnection; /// Test helper to create a mock request fn create_test_request(method: crate::sip::Method, branch: &str) -> crate::sip::Request { @@ -162,3 +164,309 @@ async fn test_transaction_types() -> crate::Result<()> { Ok(()) } + +// =========================================================================== +// RFC 6026 conformance tests +// +// Validate the Accepted state and Timer L / Timer M behaviour required by +// RFC 6026 §7.1 (server INVITE) + §7.2 (client INVITE). +// +// Test design notes: +// - Display-impl tests are pure unit tests — no async setup required. +// - State-machine tests use a mock UDP connection bound to 127.0.0.1:0 +// so respond() can issue a real UDP send into the void; UDP is +// connectionless so the send succeeds even with no peer listener, +// letting the state transition fire without a full peer orchestration. +// - Timer L / Timer M actually firing (transitioning Accepted → Terminated +// after 64*T1 ≈ 32s) is NOT exercised here because waiting 32s per test +// would balloon the suite runtime; instead these tests verify +// timer_l.is_some() / timer_m.is_some() to confirm the timers are armed. +// The transition logic itself is exercised by the unit-level state +// machine in transaction.rs on_timer dispatch. +// =========================================================================== + +/// Helper: mock UDP connection on localhost for tests that exercise respond(). +/// The connection sends UDP packets to a bogus loopback port — UDP is +/// connectionless so the send succeeds (packets are dropped at the OS). +async fn mock_udp_connection() -> crate::Result { + let conn = UdpConnection::create_connection("127.0.0.1:0".parse()?, None, None).await?; + Ok(conn.into()) +} + +#[test] +fn test_rfc6026_accepted_state_display() { + // RFC 6026 §7.1/§7.2 exposes the Accepted state via the public + // TransactionState Display impl. Lock the rendering so external + // observers (logs, metrics, debug dumps) see "Accepted". + assert_eq!( + format!("{}", TransactionState::Accepted), + "Accepted", + "TransactionState::Accepted must Display as `Accepted` (RFC 6026 §7.1/§7.2)", + ); +} + +#[test] +fn test_rfc6026_timer_l_display() { + // RFC 6026 §7.1 server INVITE Accepted-state timer. + let request = crate::sip::Request { + method: crate::sip::Method::Invite, + uri: crate::sip::Uri::try_from("sip:bob@example.com").unwrap(), + headers: vec![ + Via::new("SIP/2.0/UDP example.com:5060;branch=z9hG4bKtimerL").into(), + CSeq::new("1 INVITE").into(), + From::new("Alice ;tag=t1").into(), + CallId::new("call-l@example.com").into(), + ] + .into(), + version: crate::sip::Version::V2, + body: Default::default(), + }; + let key = TransactionKey::from_request(&request, TransactionRole::Server).unwrap(); + let timer = TransactionTimer::TimerL(key.clone()); + assert_eq!(format!("{}", timer), format!("TimerL: {}", key)); +} + +#[test] +fn test_rfc6026_timer_m_display() { + // RFC 6026 §7.2 client INVITE Accepted-state timer. + let request = crate::sip::Request { + method: crate::sip::Method::Invite, + uri: crate::sip::Uri::try_from("sip:bob@example.com").unwrap(), + headers: vec![ + Via::new("SIP/2.0/UDP example.com:5060;branch=z9hG4bKtimerM").into(), + CSeq::new("1 INVITE").into(), + From::new("Alice ;tag=t1").into(), + CallId::new("call-m@example.com").into(), + ] + .into(), + version: crate::sip::Version::V2, + body: Default::default(), + }; + let key = TransactionKey::from_request(&request, TransactionRole::Client).unwrap(); + let timer = TransactionTimer::TimerM(key.clone()); + assert_eq!(format!("{}", timer), format!("TimerM: {}", key)); +} + +#[tokio::test] +async fn test_rfc6026_server_invite_2xx_routes_to_accepted_with_timer_l() -> crate::Result<()> { + // RFC 6026 §7.1: server INVITE 2xx final response transitions + // Proceeding/Trying → Accepted (NOT Completed), and arms Timer L. + // Concurrently, Timer G must NOT be started for 2xx (§7.1 prohibits + // the server transaction from retransmitting 2xx on its own). + let endpoint = create_test_endpoint(Some("127.0.0.1:0")).await?; + let conn = mock_udp_connection().await?; + + let invite_req = create_test_request(crate::sip::Method::Invite, "z9hG4bK6026srv2xx"); + let key = TransactionKey::from_request(&invite_req, TransactionRole::Server)?; + + let mut tx = Transaction::new_server( + key.clone(), + invite_req.clone(), + endpoint.inner.clone(), + Some(conn), + ); + tx.destination = Some(crate::transport::SipAddr::from( + "127.0.0.1:1".parse::()?, + )); + + tx.reply(crate::sip::StatusCode::OK).await?; + + assert_eq!( + tx.state, + TransactionState::Accepted, + "server INVITE 2xx must route to Accepted per RFC 6026 §7.1, not Completed", + ); + assert!( + tx.timer_l.is_some(), + "Accepted state must arm Timer L (= 64*T1) per RFC 6026 §7.1", + ); + assert!( + tx.timer_g.is_none(), + "Accepted state must NOT arm Timer G — RFC 6026 §7.1 prohibits the server transaction from retransmitting 2xx", + ); + Ok(()) +} + +#[tokio::test] +async fn test_rfc6026_server_invite_4xx_still_routes_to_completed() -> crate::Result<()> { + // RFC 3261 §17.2.1 regression: non-2xx final responses (3xx-6xx) must + // continue to route to Completed (not Accepted) so the existing Timer G + // retransmit + Timer K + Timer D semantics for ACK matching are preserved. + let endpoint = create_test_endpoint(Some("127.0.0.1:0")).await?; + let conn = mock_udp_connection().await?; + + let invite_req = create_test_request(crate::sip::Method::Invite, "z9hG4bK6026srv404"); + let key = TransactionKey::from_request(&invite_req, TransactionRole::Server)?; + + let mut tx = Transaction::new_server( + key.clone(), + invite_req.clone(), + endpoint.inner.clone(), + Some(conn), + ); + tx.destination = Some(crate::transport::SipAddr::from( + "127.0.0.1:1".parse::()?, + )); + + tx.reply(crate::sip::StatusCode::NotFound).await?; + + assert_eq!( + tx.state, + TransactionState::Completed, + "server INVITE 4xx must continue to route to Completed (RFC 3261 §17.2.1 retained)", + ); + Ok(()) +} + +#[tokio::test] +async fn test_rfc6026_server_invite_accepted_registers_waiting_ack() -> crate::Result<()> { + // RFC 6026 §7.1: when a server INVITE transaction enters Accepted, it + // MUST register itself in the dialog-layer ACK routing map so the + // forthcoming ACK (sent by the UAC outside this transaction per + // §17.1.1.3) lands at this transaction key. The waiting_ack map keyed + // by DialogId is rsipstack's canonical routing primitive. + use crate::dialog::DialogId; + + let endpoint = create_test_endpoint(Some("127.0.0.1:0")).await?; + let conn = mock_udp_connection().await?; + + let invite_req = create_test_request(crate::sip::Method::Invite, "z9hG4bK6026srvack"); + let key = TransactionKey::from_request(&invite_req, TransactionRole::Server)?; + + let mut tx = Transaction::new_server( + key.clone(), + invite_req.clone(), + endpoint.inner.clone(), + Some(conn), + ); + tx.destination = Some(crate::transport::SipAddr::from( + "127.0.0.1:1".parse::()?, + )); + + tx.reply(crate::sip::StatusCode::OK).await?; + + // The 200 OK has been stored as last_response; derive the dialog_id + // (server role) and check that the waiting_ack map points back to us. + let last_response = tx + .last_response + .as_ref() + .expect("respond() must store the 2xx in last_response"); + let dialog_id = DialogId::try_from((last_response, TransactionRole::Server))?; + let registered = endpoint.inner.waiting_ack.get(&dialog_id); + assert!( + registered.is_some(), + "Accepted-state entry must register the dialog in waiting_ack for ACK routing per RFC 6026 §7.1", + ); + assert_eq!( + registered.expect("registered").value(), + &key, + "waiting_ack entry must point back to this transaction key", + ); + Ok(()) +} + +#[tokio::test] +async fn test_rfc6026_server_invite_3xx_still_arms_timer_g() -> crate::Result<()> { + // RFC 6026 §7.1 retains Timer G for non-2xx final responses + // (the §7.1 prohibition is "MUST NOT generate 2xx retransmissions", + // explicitly NOT a blanket ban). This regression test ensures the + // server-side 3xx-6xx Timer G retransmit path remains live. + let endpoint = create_test_endpoint(Some("127.0.0.1:0")).await?; + let conn = mock_udp_connection().await?; + + let invite_req = create_test_request(crate::sip::Method::Invite, "z9hG4bK6026srvtimerg"); + let key = TransactionKey::from_request(&invite_req, TransactionRole::Server)?; + + let mut tx = Transaction::new_server( + key.clone(), + invite_req.clone(), + endpoint.inner.clone(), + Some(conn), + ); + tx.destination = Some(crate::transport::SipAddr::from( + "127.0.0.1:1".parse::()?, + )); + + tx.reply(crate::sip::StatusCode::ServerInternalError) + .await?; + + assert_eq!(tx.state, TransactionState::Completed); + assert!( + tx.timer_g.is_some(), + "non-2xx final must continue to arm Timer G (RFC 3261 §17.2.1 retained; RFC 6026 §7.1 only prohibits 2xx retransmits)", + ); + Ok(()) +} + +#[tokio::test] +async fn test_rfc6026_server_invite_accepted_absorbs_duplicate_2xx() -> crate::Result<()> { + // RFC 6026 §7.1: 'absorb retransmissions of the INVITE after a 2xx + // response has been sent' implies the server transaction MUST tolerate + // a TU-retransmitted 2xx in the Accepted state — the response is + // re-sent through the transport but no state error is raised. The + // can_transition() (Accepted, Accepted) self-loop edge enables this. + let endpoint = create_test_endpoint(Some("127.0.0.1:0")).await?; + let conn = mock_udp_connection().await?; + + let invite_req = create_test_request(crate::sip::Method::Invite, "z9hG4bK6026srvdup"); + let key = TransactionKey::from_request(&invite_req, TransactionRole::Server)?; + + let mut tx = Transaction::new_server( + key.clone(), + invite_req.clone(), + endpoint.inner.clone(), + Some(conn), + ); + tx.destination = Some(crate::transport::SipAddr::from( + "127.0.0.1:1".parse::()?, + )); + + let response_1 = tx.reply(crate::sip::StatusCode::OK).await; + response_1?; + assert_eq!(tx.state, TransactionState::Accepted); + + // Second TU-driven 2xx (e.g. retransmit per §7.1 forwarding rule): + // must succeed without state-machine error; transaction stays in + // Accepted (the (Accepted, Accepted) edge in can_transition). + tx.reply(crate::sip::StatusCode::OK).await?; + assert_eq!( + tx.state, + TransactionState::Accepted, + "duplicate 2xx in Accepted must keep the transaction in Accepted, not raise a transition error", + ); + Ok(()) +} + +#[tokio::test] +async fn test_rfc6026_server_invite_accepted_rejects_3xx_transition() -> crate::Result<()> { + // can_transition matrix: once a server INVITE has entered Accepted (2xx + // already sent), a subsequent 3xx-6xx attempt must be rejected. The + // matrix has no (Accepted, Completed) edge so respond() should error. + let endpoint = create_test_endpoint(Some("127.0.0.1:0")).await?; + let conn = mock_udp_connection().await?; + + let invite_req = create_test_request(crate::sip::Method::Invite, "z9hG4bK6026srvreject"); + let key = TransactionKey::from_request(&invite_req, TransactionRole::Server)?; + + let mut tx = Transaction::new_server( + key.clone(), + invite_req.clone(), + endpoint.inner.clone(), + Some(conn), + ); + tx.destination = Some(crate::transport::SipAddr::from( + "127.0.0.1:1".parse::()?, + )); + + tx.reply(crate::sip::StatusCode::OK).await?; + assert_eq!(tx.state, TransactionState::Accepted); + + // Now try sending a 4xx — this is illegal per the can_transition matrix + // (Accepted has no edge to Completed). reply() should return an Err. + let err = tx.reply(crate::sip::StatusCode::BusyHere).await; + assert!( + err.is_err(), + "reply() must reject Accepted → Completed (no such edge in can_transition matrix)", + ); + Ok(()) +} diff --git a/src/transaction/transaction.rs b/src/transaction/transaction.rs index 133d823..bd60f21 100644 --- a/src/transaction/transaction.rs +++ b/src/transaction/transaction.rs @@ -11,7 +11,7 @@ use crate::transaction::make_tag; use crate::transport::SipAddr; use crate::{Error, Result}; use tokio::sync::mpsc::{unbounded_channel, UnboundedReceiver, UnboundedSender}; -use tracing::{debug, trace}; +use tracing::{debug, trace, warn}; pub type TransactionEventReceiver = UnboundedReceiver; pub type TransactionEventSender = UnboundedSender; @@ -178,7 +178,9 @@ pub struct Transaction { pub timer_c: Option, pub timer_d: Option, pub timer_k: Option, // server invite only - pub timer_g: Option, // server invite only + pub timer_g: Option, // server invite only (non-2xx final response retransmits per RFC 6026 §7.1) + pub timer_l: Option, // server invite only (Accepted-state 64*T1 per RFC 6026 §7.1) + pub timer_m: Option, // client invite only (Accepted-state 64*T1 per RFC 6026 §7.2) is_cleaned_up: bool, } @@ -216,6 +218,8 @@ impl Transaction { timer_d: None, timer_k: None, timer_g: None, + timer_l: None, + timer_m: None, tu_receiver, tu_sender, is_cleaned_up: false, @@ -334,7 +338,35 @@ impl Transaction { pub async fn reply(&mut self, status_code: StatusCode) -> Result<()> { self.reply_with(status_code, vec![], None).await } - // send server response + /// Send a server response. + /// + /// Validates the state transition, sends the response through the + /// transaction's connection, stores it as `last_response`, and + /// transitions the state machine. The response routing follows + /// RFC 3261 §17.2.1 + RFC 6026 §7.1 (server INVITE 2xx → Accepted; + /// server INVITE 3xx-6xx → Completed; server non-INVITE → Terminated). + /// + /// # Errors + /// Returns `Error::TransactionError` if: + /// - the transaction is a client transaction (`respond` is server-only), + /// - the requested transition is not allowed by `can_transition`, + /// - the connection is missing, + /// - the underlying transport send fails. + /// + /// # Cancel safety + /// + /// **NOT cancel-safe.** The mutation order is: store `last_response` + /// → `await` the transport send → transition the state machine. If the + /// future is dropped during the `await`, `last_response` will already + /// be updated while the response may have been only partially + /// transmitted (or not at all) and the state did not advance. If it + /// is dropped between the send completing and the transition, the + /// response is on the wire but the transaction remains in the previous + /// state and subsequent timer-driven retransmits may fire incorrectly. + /// Drive `respond` from an owning task; do not place it directly + /// inside `tokio::select!` / `tokio::time::timeout` unless this hazard + /// is acceptable. Prefer the `TransactionEvent::Respond` channel for + /// cancellation-aware integration. pub async fn respond(&mut self, response: Response) -> Result<()> { match self.transaction_type { TransactionType::ServerInvite | TransactionType::ServerNonInvite => {} @@ -351,6 +383,20 @@ impl Transaction { StatusCode::Trying => TransactionState::Trying, _ => TransactionState::Proceeding, }, + StatusCodeKind::Successful => match self.transaction_type { + // RFC 6026 §7.1: server INVITE 2xx routes to Accepted, NOT + // Completed. Pre-RFC-6026 behaviour incorrectly used the + // Completed state — that triggered Timer G 2xx retransmits + // (forbidden by §7.1) and used Timer K (T4 ≈ 5s) as the + // effective ACK window instead of Timer L (64*T1 ≈ 32s), + // causing spurious failures on proxy-chain ACK fan-in. + TransactionType::ServerInvite => TransactionState::Accepted, + // ServerNonInvite 2xx → Terminated (no ACK expected). + _ => TransactionState::Terminated, + }, + // Non-2xx final response (3xx, 4xx, 5xx, 6xx): RFC 3261 §17.2.1 + // semantics retained — server INVITE → Completed (waits for + // ACK + Timer K + Timer D); server non-INVITE → Terminated. _ => match self.transaction_type { TransactionType::ServerInvite => TransactionState::Completed, _ => TransactionState::Terminated, @@ -391,17 +437,22 @@ impl Transaction { | (&TransactionState::Calling, &TransactionState::Calling) | (&TransactionState::Calling, &TransactionState::Trying) | (&TransactionState::Calling, &TransactionState::Proceeding) + | (&TransactionState::Calling, &TransactionState::Accepted) // RFC 6026 §7.2 | (&TransactionState::Calling, &TransactionState::Completed) | (&TransactionState::Calling, &TransactionState::Terminated) | (&TransactionState::Trying, &TransactionState::Trying) // retransmission | (&TransactionState::Trying, &TransactionState::Proceeding) + | (&TransactionState::Trying, &TransactionState::Accepted) // RFC 6026 §7.1/§7.2 | (&TransactionState::Trying, &TransactionState::Completed) | (&TransactionState::Trying, &TransactionState::Confirmed) | (&TransactionState::Trying, &TransactionState::Terminated) | (&TransactionState::Proceeding, &TransactionState::Proceeding) + | (&TransactionState::Proceeding, &TransactionState::Accepted) // RFC 6026 §7.1/§7.2 | (&TransactionState::Proceeding, &TransactionState::Completed) | (&TransactionState::Proceeding, &TransactionState::Confirmed) | (&TransactionState::Proceeding, &TransactionState::Terminated) + | (&TransactionState::Accepted, &TransactionState::Accepted) // RFC 6026 §7.1: absorb 2xx retransmits + | (&TransactionState::Accepted, &TransactionState::Terminated) // RFC 6026 §7.1/§7.2: Timer L/M fires | (&TransactionState::Completed, &TransactionState::Confirmed) | (&TransactionState::Completed, &TransactionState::Terminated) | (&TransactionState::Confirmed, &TransactionState::Terminated) => Ok(()), @@ -445,6 +496,40 @@ impl Transaction { } } + /// Send an ACK for a final response on a client INVITE transaction. + /// + /// Constructs the ACK from `last_ack` (cached) or `last_response` + + /// the original INVITE, resolves the destination (Contact for 2xx + /// per RFC 3261 §13.2.2.4; Via/Record-Route for 3xx-6xx per §17.1.1.3), + /// and sends through either the supplied `connection` argument or, for + /// 2xx responses, a transport-layer connection looked up from the + /// resolved Contact. If neither path produces a connection, the ACK is + /// recorded as `last_ack` (for caller-driven retransmission) but not + /// transmitted, and `Ok(())` is still returned. The ACK is recorded as + /// `last_ack` regardless of send outcome. + /// + /// State transitions per RFC 6026 §7.2 + RFC 3261 §17.1.1.3: + /// - Completed (3xx-6xx): transitions to Terminated. + /// - Accepted (2xx): no transition (Timer M drives Termination so + /// the §7.2 server-retransmitted-2xx absorption window is preserved). + /// + /// # Errors + /// Returns `Error::TransactionError` if: + /// - the transaction is not a client INVITE, + /// - the state is neither Completed nor Accepted, + /// - no `last_response` and no `last_ack` is available, + /// - destination resolution or transport send fails. + /// + /// # Cancel safety + /// + /// **NOT cancel-safe.** This method has multiple `.await` points + /// (locator lookup, transport lookup, transport send) interleaved + /// with `last_ack` mutation and state transition. If the future is + /// dropped between the ACK transmission and the state transition, + /// the ACK will have been sent but the transaction state will remain + /// in Completed/Accepted. The next inbound message may trigger + /// inappropriate retransmits. Do not place inside `tokio::select!` + /// unless this hazard is acceptable. pub async fn send_ack(&mut self, connection: Option) -> Result<()> { if self.transaction_type != TransactionType::ClientInvite { return Err(Error::TransactionError( @@ -454,7 +539,7 @@ impl Transaction { } match self.state { - TransactionState::Completed => {} // must be in completed state, to send ACK + TransactionState::Completed | TransactionState::Accepted => {} // RFC 3261 §17.1.1 (Completed, 3xx-6xx) or RFC 6026 §7.2 (Accepted, 2xx) _ => { return Err(Error::TransactionError( format!("invalid state for sending ACK {:?}", self.state), @@ -515,10 +600,50 @@ impl Transaction { if let Some(conn) = connection { conn.send(ack, self.destination.as_ref()).await?; } - // client send ack and transition to Terminated - self.transition(TransactionState::Terminated).map(|_| ()) + // RFC 3261 §17.1.1.3 / RFC 6026 §7.2: ACK in Completed (3xx-6xx) + // immediately terminates the client transaction. ACK in Accepted + // (2xx) leaves the transaction in Accepted to absorb server- + // retransmitted 2xx duplicates per §7.2; Timer M drives the + // eventual transition to Terminated. Returning Ok(()) here lets + // the caller observe a successful ACK send without forcing a + // premature transition. + if self.state == TransactionState::Completed { + self.transition(TransactionState::Terminated).map(|_| ()) + } else { + debug_assert_eq!( + self.state, + TransactionState::Accepted, + "send_ack reached post-send dispatch in unexpected state; the entry guard restricts to Completed|Accepted", + ); + Ok(()) + } } + /// Receive the next SIP message routed to this transaction. + /// + /// Loops on the transaction's TU receiver, dispatching incoming + /// requests/responses through `on_received_request` / + /// `on_received_response`, processing timer events via `on_timer`, + /// and forwarding `Respond` events through `respond`. Returns + /// `Some(msg)` when a request/response should be propagated to the + /// dialog/TU layer; returns `None` when the transaction is + /// terminated by a `Terminate` event or the underlying channel + /// closes. + /// + /// # Cancel safety + /// + /// **NOT cancel-safe.** State mutations may occur around any inner + /// `.await` — for example, an incoming response is passed to + /// `on_received_response` which transitions state and stores + /// `last_response` BEFORE the message is returned to the caller. + /// If the future is dropped between those mutations and the + /// `return Some(msg)`, the TU will not observe the response while + /// the transaction has already advanced state. Drive + /// `Transaction::receive` from an owning task; do not place it + /// directly inside `tokio::select!` / `tokio::time::timeout` unless + /// dropped state is acceptable. If timeout is required, prefer a + /// dedicated cancellation channel that the transaction itself + /// observes via `TransactionEvent::Terminate`. pub async fn receive(&mut self) -> Option { while let Some(event) = self.tu_receiver.recv().await { match event { @@ -630,6 +755,24 @@ impl Transaction { self.respond(last_response.to_owned()).await.ok(); } } + TransactionState::Accepted => { + // RFC 6026 §7.1: server INVITE Accepted state. + // + // ACK received: pass to TU; remain in Accepted (transition to + // Terminated is driven by Timer L expiry, not by ACK arrival + // — §7.1 'Timer L reflects the amount of time the server + // transaction could receive 2xx responses for retransmission + // from the TU while it is waiting to receive an ACK'). + // + // INVITE retransmit: silently absorbed. Per §7.1 the server + // transaction MUST NOT retransmit the 2xx on its own (only + // the TU does, via respond()), so re-firing respond() here + // would re-issue the 2xx through the transport AND trigger + // a no-op Accepted-self-loop transition. Just drop. + if req.method == Method::Ack { + return Some(req.into()); + } + } TransactionState::Completed | TransactionState::Confirmed => { if req.method == Method::Ack { self.transition(TransactionState::Confirmed).ok(); @@ -658,7 +801,23 @@ impl Transaction { TransactionState::Proceeding } } + StatusCodeKind::Successful => { + // RFC 6026 §7.2: client INVITE 2xx routes to Accepted (was + // Completed pre-RFC-6026; that incorrectly used Timer D = T1*64 + // as the 2xx-retransmit-absorption window AND auto-issued an ACK + // through the transaction layer in violation of RFC 3261 + // §17.1.1.3. Accepted + Timer M = T1*64 is the spec-correct + // window; the ACK is still auto-issued below for backward + // compat with rsipstack 0.5.x — see send_ack and the + // auto-ACK gate below. + if self.transaction_type == TransactionType::ClientInvite { + TransactionState::Accepted + } else { + TransactionState::Terminated + } + } _ => { + // 3xx-6xx final response: RFC 3261 §17.1.1 unchanged. if self.transaction_type == TransactionType::ClientInvite { TransactionState::Completed } else { @@ -668,7 +827,21 @@ impl Transaction { }; self.can_transition(&new_state).ok()?; - if self.state == new_state { + + // RFC 6026 §7.2: every 2xx response received by a client INVITE in + // the Accepted state MUST be passed to the TU — both genuine + // server-retransmitted 2xx (which the TU/dialog re-acknowledges) + // and forked 2xx (which can share status code + body but differ + // in To-tag and so identify a different dialog). The pre-existing + // duplicate-suppression filter below is correct for non-INVITE + // transactions where the TU has no use for duplicates, but applying + // it to the Accepted self-loop would silently swallow forked dialogs + // and prevent the legacy auto-ACK from re-firing per retransmit. + let is_client_invite_2xx_in_accepted = self.transaction_type + == TransactionType::ClientInvite + && new_state == TransactionState::Accepted; + + if !is_client_invite_2xx_in_accepted && self.state == new_state { if let Some(last) = self.last_response.as_ref() { if last.status_code == resp.status_code && last.body == resp.body { // ignore duplicate response @@ -678,13 +851,29 @@ impl Transaction { } self.last_response.replace(resp.clone()); - let is_completed_client_invite = self.transaction_type == TransactionType::ClientInvite - && new_state == TransactionState::Completed; + // Auto-ACK gate. Pre-RFC-6026 this fired only for new_state == + // Completed (which captured both 2xx and 3xx-6xx in the old routing). + // Post-RFC-6026 split: 2xx → Accepted, 3xx-6xx → Completed. Both + // states still benefit from the convenience auto-ACK; the transaction + // layer constructs and sends the ACK via send_ack below. send_ack + // transitions Completed → Terminated (RFC 3261 §17.1.1) but leaves + // Accepted alone so Timer M can fire and the §7.2 2xx-retransmit + // absorption window is preserved. + let auto_ack_client_invite = self.transaction_type == TransactionType::ClientInvite + && (new_state == TransactionState::Completed + || new_state == TransactionState::Accepted); self.transition(new_state).ok(); - if is_completed_client_invite { - self.send_ack(connection).await.ok(); + if auto_ack_client_invite { + if let Err(e) = self.send_ack(connection).await { + warn!( + key = %self.key, + state = %self.state, + error = %e, + "auto-ACK for client INVITE final response failed; downstream TU may need to handle ACK explicitly", + ); + } } Some(SipMessage::Response(resp)) @@ -744,7 +933,20 @@ impl Transaction { } TransactionState::Completed => { if let TransactionTimer::TimerG(key, duration) = timer { - // resend the response + // RFC 6026 §7.1 defensive guard: the server transaction + // MUST NOT retransmit 2xx responses on its own. Per the + // RFC 6026 routing in respond() + on_received_response(), + // 2xx finals route to the Accepted state, not Completed — + // so `last_response` here should always be non-2xx. This + // guard catches any legacy / out-of-band code path that + // might land a 2xx in Completed; suppress the retransmit + // and let Timer D / Timer K handle Termination. + if let Some(last_response) = &self.last_response { + if last_response.status_code.kind() == StatusCodeKind::Successful { + return Ok(()); + } + } + // resend the response (non-2xx final — RFC 3261 §17.2.1) if let Some(last_response) = &self.last_response { if let Some(connection) = &self.connection { let last_response = if let Some(ref inspector) = @@ -775,6 +977,38 @@ impl Transaction { self.transition(TransactionState::Terminated)?; } } + TransactionState::Accepted => { + // RFC 6026 §7.1 (server INVITE Timer L) / §7.2 (client INVITE + // Timer M): on expiry the transaction transitions to + // Terminated. Timer L is server-only per §7.1; Timer M is + // client-only per §7.2 — mismatched pairings indicate a + // programming bug and are logged for visibility. Stray + // Timer A/B/C/D/G/K/Cleanup firings are race remnants from + // prior states (the Accepted-state entry handler cancels + // them, but fire-in-flight races are possible); listed + // explicitly so future timer additions force compile-time + // review here. + match (&self.transaction_type, &timer) { + (TransactionType::ServerInvite, TransactionTimer::TimerL(_)) + | (TransactionType::ClientInvite, TransactionTimer::TimerM(_)) => { + self.transition(TransactionState::Terminated)?; + } + (_, TransactionTimer::TimerL(_) | TransactionTimer::TimerM(_)) => { + warn!( + key = %self.key, + tx_type = %self.transaction_type, + "RFC 6026 Accepted-state timer fired with mismatched transaction type", + ); + } + (_, TransactionTimer::TimerA(_, _)) + | (_, TransactionTimer::TimerB(_)) + | (_, TransactionTimer::TimerC(_)) + | (_, TransactionTimer::TimerD(_)) + | (_, TransactionTimer::TimerG(_, _)) + | (_, TransactionTimer::TimerK(_)) + | (_, TransactionTimer::TimerCleanup(_)) => {} + } + } TransactionState::Confirmed => { if let TransactionTimer::TimerK(_) = timer { self.transition(TransactionState::Terminated)?; @@ -835,6 +1069,94 @@ impl Transaction { } } } + TransactionState::Accepted => { + self.timer_a + .take() + .map(|id| self.endpoint_inner.timers.cancel(id)); + self.timer_b + .take() + .map(|id| self.endpoint_inner.timers.cancel(id)); + self.timer_c + .take() + .map(|id| self.endpoint_inner.timers.cancel(id)); + + match self.transaction_type { + TransactionType::ServerInvite => { + // RFC 6026 §7.1: server INVITE 2xx-Accepted entry. + // + // Start Timer L (64*T1). On expiry the transaction + // transitions to Terminated (handled in on_timer for + // the Accepted state). + // + // Register the dialog in `waiting_ack` so the dialog + // layer can route the ACK for this 2xx back to this + // transaction key. + // + // Do NOT start Timer G — RFC 6026 §7.1 explicitly + // forbids the server transaction from retransmitting + // 2xx responses on its own ("It is not retransmitted + // by the server transaction; retransmissions of 2xx + // responses are handled by the TU.") + let timer_l = self.endpoint_inner.timers.timeout( + self.endpoint_inner.option.t1x64, + TransactionTimer::TimerL(self.key.clone()), + ); + self.timer_l.replace(timer_l); + + if let Some(ref resp) = self.last_response { + let dialog_id = DialogId::try_from((resp, TransactionRole::Server))?; + self.endpoint_inner + .waiting_ack + .insert(dialog_id, self.key.clone()); + } + debug!( + key = %self.key, + "entered Accepted state (server); Timer L armed, waiting for ACK and 2xx retransmits from TU" + ); + } + TransactionType::ClientInvite => { + // RFC 6026 §7.2: client INVITE 2xx-Accepted entry. + // + // Start Timer M (64*T1). On expiry the transaction + // transitions to Terminated. While in Accepted, the + // client absorbs server-retransmitted 2xx responses + // (these are forwarded to the TU as duplicates and + // ignored by the transaction state machine itself — + // see Accepted-self-loop edge in can_transition). + // + // Strict RFC 3261 §17.1.1.3 + RFC 6026 §7.2 make + // the ACK for 2xx the TU's responsibility. For + // rsipstack 0.5.x backward compatibility, the + // auto_ack_client_invite gate at the tail of + // on_received_response still emits that ACK after + // entering Accepted; see lib.rs Standards Compliance + // for the disclaimer. 3xx-6xx ACKs continue to be + // sent by the transaction layer through the + // Completed → Terminated path. + let timer_m = self.endpoint_inner.timers.timeout( + self.endpoint_inner.option.t1x64, + TransactionTimer::TimerM(self.key.clone()), + ); + self.timer_m.replace(timer_m); + debug!( + key = %self.key, + "entered Accepted state (client); Timer M armed, awaiting expiry or 2xx retransmits" + ); + } + _ => { + // Non-INVITE transactions never reach Accepted per + // RFC 6026 §4. can_transition() should have already + // rejected this; treat as a programming error. + return Err(Error::TransactionError( + format!( + "Accepted state is INVITE-only (transaction type was {})", + self.transaction_type + ), + self.key.clone(), + )); + } + } + } TransactionState::Completed => { self.timer_a .take() @@ -927,6 +1249,12 @@ impl Transaction { self.timer_g .take() .map(|id| self.endpoint_inner.timers.cancel(id)); + self.timer_l + .take() + .map(|id| self.endpoint_inner.timers.cancel(id)); + self.timer_m + .take() + .map(|id| self.endpoint_inner.timers.cancel(id)); } pub fn role(&self) -> TransactionRole {