[Asterisk-code-review] res pjsip session.c: Don't send extra BYE if SDP invalid. (asterisk[master])

[Richard Mudgett]

Richard Mudgett has uploaded a new change for review.

  https://gerrit.asterisk.org/3135

Change subject: res_pjsip_session.c: Don't send extra BYE if SDP invalid.
......................................................................

res_pjsip_session.c: Don't send extra BYE if SDP invalid.

When an answer SDP is invalid we were disconnecting the outgoing call and
sending two BYE requests.  The first BYE was sent by PJPROJECT because of
the invalid SDP answer.  The second BYE was sent by Asterisk because it
thought the canceled call was the result of the RFC5407 section 3.1.2 race
condition.

* Made not send the BYE on a canceled session if the SDP negotiation is
incomplete because PJPROJECT has already sent a BYE for the failed
negotiation.

ASTERISK-25772 #close
Reported by:  Dmitriy Serov

Change-Id: I44ad0bd0605e8eeb7035c890d6f97a1331f1a836
---
M res/res_pjsip_session.c
1 file changed, 42 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/35/3135/1

diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 405fdf8..9f98e34 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -46,6 +46,7 @@
 #include "asterisk/acl.h"
 #include "asterisk/features_config.h"
 #include "asterisk/pickup.h"
+#include "asterisk/test.h"
 
 #define SDP_HANDLER_BUCKETS 11
 
@@ -2687,14 +2688,49 @@
 					}
 				} else if (tsx->state == PJSIP_TSX_STATE_TERMINATED) {
 					if (inv->cancelling && tsx->status_code == PJSIP_SC_OK) {
-						/* This is a race condition detailed in RFC 5407 section 3.1.2.
-						 * We sent a CANCEL at the same time that the UAS sent us a 200 OK for
-						 * the original INVITE. As a result, we have now received a 200 OK for
-						 * a cancelled call. Our role is to immediately send a BYE to end the
-						 * dialog.
+						int sdp_negotiation_done =
+							pjmedia_sdp_neg_get_state(inv->neg) == PJMEDIA_SDP_NEG_STATE_DONE;
+
+						/*
+						 * We can get here for the following reasons.
+						 *
+						 * 1) The race condition detailed in RFC5407 section 3.1.2.
+						 * We sent a CANCEL at the same time that the UAS sent us a
+						 * 200 OK with a valid SDP for the original INVITE.  As a
+						 * result, we have now received a 200 OK for a cancelled
+						 * call and the SDP negotiation is complete.  We need to
+						 * immediately send a BYE to end the dialog.
+						 *
+						 * 2) We sent a CANCEL and hit the race condition but the
+						 * UAS sent us an invalid SDP with the 200 OK.  In this case
+						 * the SDP negotiation is incomplete and PJPROJECT has
+						 * already sent the BYE for us because of the invalid SDP.
+						 *
+						 * 3) We didn't send a CANCEL but the UAS sent us an invalid
+						 * SDP with the 200 OK.  In this case the SDP negotiation is
+						 * incomplete and PJPROJECT has already sent the BYE for us
+						 * because of the invalid SDP.
 						 */
-						if (pjsip_inv_end_session(inv, 500, NULL, &tdata) == PJ_SUCCESS
+						ast_test_suite_event_notify("PJSIP_SESSION_CANCELED",
+							"Endpoint: %s\r\n"
+							"Channel: %s\r\n"
+							"Message: %s\r\n"
+							"SDP: %s",
+							ast_sorcery_object_get_id(session->endpoint),
+							session->channel ? ast_channel_name(session->channel) : "",
+							pjsip_rx_data_get_info(e->body.tsx_state.src.rdata),
+							sdp_negotiation_done ? "complete" : "incomplete");
+						if (!sdp_negotiation_done) {
+							ast_debug(1, "Endpoint '%s(%s)': Incomplete SDP negotiation cancelled session.  %s\n",
+								ast_sorcery_object_get_id(session->endpoint),
+								session->channel ? ast_channel_name(session->channel) : "",
+								pjsip_rx_data_get_info(e->body.tsx_state.src.rdata));
+						} else if (pjsip_inv_end_session(inv, 500, NULL, &tdata) == PJ_SUCCESS
 							&& tdata) {
+							ast_debug(1, "Endpoint '%s(%s)': Ending session due to RFC5407 race condition.  %s\n",
+								ast_sorcery_object_get_id(session->endpoint),
+								session->channel ? ast_channel_name(session->channel) : "",
+								pjsip_rx_data_get_info(e->body.tsx_state.src.rdata));
 							ast_sip_session_send_request(session, tdata);
 						}
 					}

-- 
To view, visit https://gerrit.asterisk.org/3135
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I44ad0bd0605e8eeb7035c890d6f97a1331f1a836
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>