Skip to content

fix: keep confirmed dialogs confirmed on 1xx to in-dialog requests - #147

Open
tgeorge06 wants to merge 1 commit into
restsend:mainfrom
tgeorge06:fix/no-confirmed-to-early-regression
Open

tgeorge06 wants to merge 1 commit into
restsend:mainfrom
tgeorge06:fix/no-confirmed-to-early-regression

Conversation

@tgeorge06

Copy link
Copy Markdown

Bug

DialogInner::send_dialog_request (src/dialog/dialog.rs, the Provisional arm) moves the dialog to Early on every non-100 provisional response. That includes responses to our own in-dialog requests on an established dialog. A peer that answers a re-INVITE (for example a session-timer refresh) or an UPDATE with 183 Session Progress flips a Confirmed dialog back to Early. The dialog stays there: the final 200 OK to the re-INVITE does not restore Confirmed.

Since 0.6.1 (23dfb20), bye_with_headers returns Err outside Confirmed, and hangup() picks CANCEL whenever can_cancel() is true, which covers Early. On such a dialog:

return_to_confirmed (4feaf95) fixes the same kind of state leak for requests we receive (UAS side). It does not cover a provisional response to a request we send.

Reproduction (on main @ 5958064)

This PR adds src/dialog/tests/test_in_dialog_provisional.rs. It drives a UAC dialog against a raw UDP peer:

  1. The initial INVITE gets 100, 183, 200. The dialog reaches Confirmed.
  2. We send a re-INVITE (or UPDATE). The peer replies 100, 183, 200.
  3. The test asserts the dialog stays Confirmed, reports no Early state, and that bye() succeeds.

On main, both new tests fail:

a 183 to an in-dialog INVITE must not leave Confirmed, state: <id>(Early)
a 183 to an in-dialog UPDATE must not leave Confirmed, state: <id>(Early)

With the state assertions skipped, the dialog is still Early after the re-INVITE's 200, and:

bye:    Err(Error("dialog ... cannot send BYE in state Early(...)"))
hangup: Err(DialogError("CANCEL failed with response 408 Request Timeout", ...))

RFC

  • RFC 3261 §12: a dialog created by a provisional response is early; once a 2xx arrives it is confirmed. There is no transition from confirmed back to early.
  • RFC 3261 §12.2: requests inside a dialog, and their responses, do not create or change the dialog's early/confirmed state. Only the dialog-creating INVITE does that.

Fix

The Early transition in send_dialog_request now runs only while the dialog has not been confirmed yet (can_cancel(), meaning Calling / Trying / Early). 8 lines total, most of them a comment.

can_cancel() was chosen over !is_confirmed() on purpose. !is_confirmed() would still let a 1xx reset WaitAck, and the states held after an inbound request (Refer, Message, Publish, ...), to Early.

Unchanged:

  • The initial INVITE is driven by process_invite (invite_dialog.rs, and the legacy client_dialog.rs wrapper). Its Trying→Early transitions and early route-set handling behave as before. The new test asserts that a 183 to the initial INVITE is still reported as Early.
  • handle_provisional_response still runs for provisionals to a re-INVITE, so reliable 1xx still get their PRACK.
  • The provisional still goes to the caller like before. Only the dialog-state change is skipped.

Compatibility / risk

  • One behaviour change: the dialog's stored state no longer becomes Early when a 1xx arrives for an in-dialog request on a confirmed dialog. The DialogState::Early notification for that 1xx is still sent exactly as before (it is the only way a caller sees a provisional to its re-INVITE, e.g. a reliable 183 with SDP), so no subscriber loses information.
  • No public API changes and no new dependencies.
  • cargo fmt --all -- --check passes, and cargo test passes: 332 lib tests (330 existing plus 2 new) and 65 doctests. cargo clippy --all-targets shows no warnings in the changed code; the existing warning counts are unchanged.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant