Repository navigation
fix: ignore an ACK whose CSeq does not match the server INVITE - #155
Merged
Merged
Conversation
ACKs for 2xx are routed to the server INVITE transaction per dialog (waiting_ack), so a delayed ACK of an earlier re-INVITE on the same dialog reaches the transaction of the current re-INVITE. Any ACK used to move it to Confirmed: Timer G stopped retransmitting the 2xx, the waiting_ack entry was removed so the real ACK lost its route, and the stale ACK was handed to the TU as if it acknowledged this INVITE. An ACK carries the CSeq number of the INVITE it acknowledges (RFC 3261 §17.1.1.3, §13.2.2.4). Ignore an ACK with any other CSeq number before touching the transaction state.
shenjinti
added a commit
that referenced
this pull request
Oct 6, 2026
- EndpointOption::auto_ack_2xx shipped in #146 without any tests: drive a raw UDP peer against a client INVITE transaction and assert that the default auto-ACKs the 2xx once (and re-ACKs each retransmission below the TU), while auto_ack_2xx = false never puts an ACK for a 2xx on the wire and terminates the transaction at once. - Cross-cover #155 x #149: a delayed ACK with a stale CSeq must not confirm the server INVITE transaction nor stop its 2xx retransmissions, and a later ACK with the matching CSeq still confirms the dialog without a BYE.
This was referenced Oct 6, 2026
A late ACK of a re-INVITE 2xx is routed to the next re-INVITE, and the call is ended with a BYE
#165
Closed
shenjinti
added a commit
that referenced
this pull request
Oct 6, 2026
Ships this round on top of 0.7.0: proxy-mode auto_ack_2xx (#146/#172), ACK CSeq routing (#155/#170/#172), digest auth_username (#153), dead stream retirement (#161), remote-ack getter (#157), UAS ACK timeout → BYE (#149/#169), RFC 6026 Accepted state + Timer L/M with documented deviations (#164/#169), CANCEL/2xx race handling (#162/#171), in-dialog Via transport (#163), flow-reuse and ACK hardening tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #154.
Problem
A server INVITE transaction in
CompletedorConfirmedmoves toConfirmedon any ACK (transaction.rs, theCompleted | Confirmed if req.method == Method::Ackarm ofon_received_request); it never checks the ACK's CSeq.An ACK for a 2xx has its own branch, so
EndpointInner::on_received_messageroutes it per dialog throughwaiting_ack. A delayed ACK of an earlier INVITE or re-INVITE on the same dialog therefore reaches the transaction of the current re-INVITE. That transaction then stops Timer G (a lost 2xx is never retransmitted), removes itswaiting_ackentry (the real ACK loses its route and is dropped), and hands the stale ACK to the TU as this re-INVITE's ACK.Spec
Fix
In that arm, compare the ACK's CSeq number with
self.original's. On a mismatch, log at debug level and returnNonebefore touching any state: no transition, Timer G and thewaiting_ackroute stay as they are, nothing goes to the TU. 16 lines insrc/transaction/transaction.rs, plus tests. No public API change.Contract / coverage
TransactionKey::from_requestrejects it).cargo test --lib.Acceptedstate): feat(transaction): RFC 6026 Accepted state + Timer L/M for INVITE 200 OK retransmission absorption #128 keeps the same unchecked ACK arm and itsAcceptedarm also forwards any ACK to the TU. If feat(transaction): RFC 6026 Accepted state + Timer L/M for INVITE 200 OK retransmission absorption #128 lands, the same CSeq-number guard belongs in theAcceptedarm; happy to rebase either way.Left out to keep this minimal (happy to follow up):
CSeq: 2 INVITEis still accepted, as today.finished_transactionsACK path inon_received_message(transaction already dropped by the TU) removes thewaiting_ackentry without a CSeq check. The ACK is absorbed silently there either way and nothing is retransmitted any more, so there is no observable effect.waiting_ackholds one entry per dialog, so a delayed ACK of re-INVITE N is now ignored rather than routed back to a still-live transaction N (the N+1 entry already overwrote N before this change).Tests
New
src/transaction/tests/test_server_invite_ack.rs(real sockets and the endpoint serve loop, same pattern astest_server_invite_drop.rs):test_server_invite_ignores_ack_with_other_cseq(UDP): re-INVITECSeq: 2answered 200; a delayedCSeq: 1ACK is not delivered, the state staysCompleted, Timer G keeps running, thewaiting_ackroute survives and the 200 is retransmitted; theCSeq: 2ACK then confirms, stops Timer G and clearswaiting_ack.test_server_invite_ignores_ack_with_other_cseq_over_tcp: same dialog over TCP; theCSeq: 1andCSeq: 2ACKs are written back to back on the ordered stream, and the first ACK the transaction delivers is theCSeq: 2one, which confirms it and clearswaiting_ack.Checks (on
main@ 3ea35cd)src/change reverted and the tests kept,cargo test --lib test_server_invite_ack→ 2 failed (an ACK with CSeq 1 must not be delivered by the CSeq 2 INVITE transaction, got Some("ACK … CSeq: 1 ACK …")over UDP;left: 1, right: 2over TCP).cargo test→ 334 lib + 65 doc passed, 0 failed (main: 332 lib).rustfmt --checkclean on the changed files.