Fixed ticket #220: Bug in retransmission of non-INVITE SIP requests in UAC transaction (thanks Martin Peterzon)

git-svn-id: http://svn.pjsip.org/repos/pjproject/branches/pjproject-0.5-stable@1189 74dad513-b988-da41-8d7b-12977e46ad98

3 files changed, 67 insertions, 115 deletions

diff --git a/pjsip/include/pjsip/sip_transaction.h b/pjsip/include/pjsip/sip_transaction.h
index d3a4c3e8..1787f787 100644
--- a/pjsip/include/pjsip/sip_transaction.h
+++ b/pjsip/include/pjsip/sip_transaction.h
@@ -74,21 +74,6 @@ typedef enum pjsip_tsx_state_e
 
 
 /**
- * Transaction timeout timer policy, to control the UAC transaction timeout
- * scheduling (see #pjsip_tsx_set_uac_timeout()).
- */
-typedef enum pjsip_tsx_timeout_policy
-{
-    PJSIP_TSX_IGNORE_100 = 1,	/**< To make the UAC transaction NOT to cancel
-				     the timeout timer when 100 response is 
-				     received.*/
-    PJSIP_TSX_IGNORE_1xx = 3	/**< To make the UAC transaction NOT to cancel
-				     the timeout timer when any 1xx responses 
-				     are received. */
-} pjsip_tsx_timeout_policy;
-
-
-/**
  * This structure describes SIP transaction object. The transaction object
  * is used to handle both UAS and UAC transaction.
  */
@@ -145,9 +130,6 @@ struct pjsip_transaction
     pj_timer_entry		retransmit_timer;/**< Retransmit timer.     */
     pj_timer_entry		timeout_timer;  /**< Timeout timer.         */
 
-    unsigned			msec_timeout;	/**< User set timeout.	    */
-    pjsip_tsx_timeout_policy	timeout_policy;	/**< Timeout policy.	    */
-
     /** Module specific data. */
     void		       *mod_data[PJSIP_MAX_MODULE];
 };
@@ -248,47 +230,6 @@ PJ_DECL(pj_status_t) pjsip_tsx_create_uas( pjsip_module *tsx_user,
 PJ_DECL(pj_status_t) pjsip_tsx_set_transport(pjsip_transaction *tsx,
 					     const pjsip_tpselector *sel);
 
-
-/**
- * Set the UAC absolute transaction timeout. Normally UAC transaction will
- * stop its timeout timer (timer B for INVITE and timer F for non-INVITE
- * transactions) after a provisional response is received.
- *
- * When this timer is set, the transaction's timer B and F will use this
- * value, and if the \a flag flag is set, the transaction will continue
- * the scheduling of the timeout timer even when provisional response has
- * been received.
- *
- * Note that this function MUST be called AFTER the transaction has been
- * created AND BEFORE any request is transmitted.
- *
- * @param tsx	    The client/UAC transaction.
- * @param msec_time The timeout value, in miliseconds. Currently this
- *		    value must be non-zero (value zero is reserved for
- *		    future use).
- * @param flag	    Option flags to control whether the transaction should
- *		    cancel the timeout timer on arrival of provisional
- *		    responses (which is yes according to RFC 3261).
- *		    The valid values are:
- *		    - PJSIP_TSX_IGNORE_100, to make the UAC transaction
- *		      NOT to cancel the timeout timer when 100 response
- *		      is received.
- *		    - PJSIP_TSX_IGNORE_1xx, to make the UAC transaction
- *		      NOT to cancel the timeout timer when any 1xx 
- *		      responses are received.
- *
- *		    Note that regardless of the values in the \a flag 
- *		    argument, the provisional response would still be
- *		    delivered to transaction user and it will still
- *		    affect the transaction state. The \a flag flag only
- *		    changes the behavior of the timeout timer of the
- *		    transaction.
- */
-PJ_DECL(pj_status_t) pjsip_tsx_set_uac_timeout(pjsip_transaction *tsx,
-					       unsigned msec_time,
-					       pjsip_tsx_timeout_policy flag);
-
-
 /**
  * Call this function to manually feed a message to the transaction.
  * For UAS transaction, application MUST call this function after
diff --git a/pjsip/src/pjsip/sip_transaction.c b/pjsip/src/pjsip/sip_transaction.c
index 911cd6c3..61b3bfa5 100644
--- a/pjsip/src/pjsip/sip_transaction.c
+++ b/pjsip/src/pjsip/sip_transaction.c
@@ -132,6 +132,8 @@ typedef struct tsx_lock_data {
 /* Timer timeout value constants */
 static const pj_time_val t1_timer_val = { PJSIP_T1_TIMEOUT/1000, 
                                           PJSIP_T1_TIMEOUT%1000 };
+static const pj_time_val t2_timer_val = { PJSIP_T2_TIMEOUT/1000, 
+                                          PJSIP_T2_TIMEOUT%1000 };
 static const pj_time_val t4_timer_val = { PJSIP_T4_TIMEOUT/1000, 
                                           PJSIP_T4_TIMEOUT%1000 };
 static const pj_time_val td_timer_val = { PJSIP_TD_TIMEOUT/1000, 
@@ -1396,23 +1398,6 @@ PJ_DEF(pj_status_t) pjsip_tsx_set_transport(pjsip_transaction *tsx,
 
 
 /*
- * Set the UAC absolute transaction timeout.
- */
-PJ_DEF(pj_status_t) pjsip_tsx_set_uac_timeout(pjsip_transaction *tsx,
-					      unsigned msec_time,
-					      pjsip_tsx_timeout_policy policy)
-{
-    PJ_ASSERT_RETURN(tsx && tsx->role==PJSIP_ROLE_UAC &&
-		     tsx->state==PJSIP_TSX_STATE_NULL, PJ_EINVALIDOP);
-
-    tsx->msec_timeout = msec_time;
-    tsx->timeout_policy = policy;
-
-    return PJ_SUCCESS;
-}
-
-
-/*
  * Set transaction status code and reason.
  */
 static void tsx_set_status_code(pjsip_transaction *tsx,
@@ -1809,7 +1794,12 @@ static void tsx_resched_retransmission( pjsip_transaction *tsx )
 
     pj_assert((tsx->transport_flag & TSX_HAS_PENDING_TRANSPORT) == 0);
 
-    msec_time = (1 << (tsx->retransmit_count)) * PJSIP_T1_TIMEOUT;
+    pj_assert(tsx->status_code < 200);
+
+    if (tsx->status_code >= 100)
+	msec_time = PJSIP_T2_TIMEOUT;
+    else
+	msec_time = (1 << (tsx->retransmit_count)) * PJSIP_T1_TIMEOUT;
 
     if (tsx->role == PJSIP_ROLE_UAC) {
 	/* Retransmission for non-INVITE transaction caps-off at T2 */
@@ -1913,22 +1903,10 @@ static pj_status_t tsx_on_state_null( pjsip_transaction *tsx,
 	}
 
 	/* Start Timer B (or called timer F for non-INVITE) for transaction 
-	 * timeout. If user has configured the timeout value with 
-	 * pjsip_tsx_set_uac_timeout(), use the timeout value there.
+	 * timeout.
 	 */
-	if (tsx->msec_timeout > 0) {
-	    pj_time_val timeout;
-
-	    timeout.sec = tsx->msec_timeout / 1000;
-	    timeout.msec = tsx->msec_timeout % 1000;
-
-	    pjsip_endpt_schedule_timer( tsx->endpt, &tsx->timeout_timer, 
-					&timeout);
-
-	} else {
-	    pjsip_endpt_schedule_timer( tsx->endpt, &tsx->timeout_timer, 
-					&timeout_timer_val);
-	}
+	pjsip_endpt_schedule_timer( tsx->endpt, &tsx->timeout_timer, 
+				    &timeout_timer_val);
 
 	/* Start Timer A (or timer E) for retransmission only if unreliable 
 	 * transport is being used.
@@ -2005,26 +1983,53 @@ static pj_status_t tsx_on_state_calling( pjsip_transaction *tsx,
 	if (msg->type != PJSIP_RESPONSE_MSG)
 	    return PJSIP_ENOTRESPONSEMSG;
 
-	/* Cancel retransmission timer A. */
-	if (tsx->retransmit_timer._timer_id != -1) {
-	    pjsip_endpt_cancel_timer(tsx->endpt, &tsx->retransmit_timer);
-	    tsx->retransmit_timer._timer_id = -1;
-	}
-	tsx->transport_flag &= ~(TSX_HAS_PENDING_RESCHED);
-
+	code = msg->line.status.code;
 
-	/* Cancel timer B (transaction timeout) but look at the timeout policy
-	 * as set by pjsip_tsx_set_uac_timeout().
+	/* If the response is final, cancel both retransmission and timeout
+	 * timer.
 	 */
-	code = msg->line.status.code;
-	if ((code==100 && tsx->timeout_policy==PJSIP_TSX_IGNORE_100) ||
-	    (code<200 && tsx->timeout_policy==PJSIP_TSX_IGNORE_1xx))
-	{
-	    /* Don't cancel the timeout timer */
-	}
-	else {
-	    pjsip_endpt_cancel_timer(tsx->endpt, &tsx->timeout_timer);
+	if (code >= 200) {
+	    if (tsx->retransmit_timer._timer_id != -1) {
+		pjsip_endpt_cancel_timer(tsx->endpt, &tsx->retransmit_timer);
+		tsx->retransmit_timer._timer_id = -1;
+	    }
+
+	    if (tsx->timeout_timer._timer_id != -1) {
+		pjsip_endpt_cancel_timer(tsx->endpt, &tsx->timeout_timer);
+		tsx->timeout_timer._timer_id = -1;
+	    }
+
+	} else {
+	    /* Cancel retransmit timer (for non-INVITE transaction, the
+	     * retransmit timer will be rescheduled at T2.
+	     */
+	    if (tsx->retransmit_timer._timer_id != -1) {
+		pjsip_endpt_cancel_timer(tsx->endpt, &tsx->retransmit_timer);
+		tsx->retransmit_timer._timer_id = -1;
+	    }
+
+	    /* For provisional response, only cancel retransmit when this
+	     * is an INVITE transaction. For non-INVITE, section 17.1.2.1
+	     * of RFC 3261 says that:
+	     *	- retransmit timer is set to T2
+	     *	- timeout timer F is not deleted.
+	     */
+	    if (tsx->method.id == PJSIP_INVITE_METHOD) {
+
+		/* Cancel timeout timer */
+		pjsip_endpt_cancel_timer(tsx->endpt, &tsx->timeout_timer);
+
+	    } else {
+		if (!tsx->is_reliable) {
+		    pjsip_endpt_schedule_timer(tsx->endpt, 
+					       &tsx->retransmit_timer,
+					       &t2_timer_val);
+		}
+	    }
 	}
+	 
+	tsx->transport_flag &= ~(TSX_HAS_PENDING_RESCHED);
+
 
 	/* Discard retransmission message if it is not INVITE.
 	 * The INVITE tdata is needed in case we have to generate ACK for
@@ -2367,7 +2372,17 @@ static pj_status_t tsx_on_state_proceeding_uac(pjsip_transaction *tsx,
 			    &msg->line.status.reason);
 
     } else {
-	tsx_set_status_code(tsx, PJSIP_SC_TSX_TIMEOUT, NULL);
+	if (event->body.timer.entry == &tsx->retransmit_timer) {
+	    /* Retransmit message. */
+            pj_status_t status;
+
+            status = tsx_retransmit( tsx, 1 );
+	    
+	    return status;
+
+	} else {
+	    tsx_set_status_code(tsx, PJSIP_SC_TSX_TIMEOUT, NULL);
+	}
     }
 
     if (PJSIP_IS_STATUS_IN_CLASS(tsx->status_code, 100)) {
diff --git a/pjsip/src/pjsip/sip_util_statefull.c b/pjsip/src/pjsip/sip_util_statefull.c
index 8ea63fda..27ce550a 100644
--- a/pjsip/src/pjsip/sip_util_statefull.c
+++ b/pjsip/src/pjsip/sip_util_statefull.c
@@ -94,6 +94,7 @@ PJ_DEF(pj_status_t) pjsip_endpt_send_request(  pjsip_endpoint *endpt,
     /* Check that transaction layer module is registered to endpoint */
     PJ_ASSERT_RETURN(mod_stateful_util.id != -1, PJ_EINVALIDOP);
 
+    PJ_UNUSED_ARG(timeout);
 
     status = pjsip_tsx_create_uac(&mod_stateful_util, tdata, &tsx);
     if (status != PJ_SUCCESS) {
@@ -105,11 +106,6 @@ PJ_DEF(pj_status_t) pjsip_endpt_send_request(  pjsip_endpoint *endpt,
     tsx_data->token = token;
     tsx_data->cb = cb;
 
-    if (timeout >= 0) {
-	status = pjsip_tsx_set_uac_timeout(tsx, timeout, PJSIP_TSX_IGNORE_1xx);
-	pj_assert(status == PJ_SUCCESS);
-    }
-
     tsx->mod_data[mod_stateful_util.id] = tsx_data;
 
     status = pjsip_tsx_send_msg(tsx, NULL);