Merge "res_pjsip_outbound_registration: Fix SRV failover on timeout" into 16

res/res_pjsip_outbound_registration.c

@@ -334,6 +334,14 @@ struct sip_outbound_registration_client_state {
 	 * module unload.
 	 */
 	pjsip_regc *client;
+	/*!
+	 * \brief Last tdata sent
+	 * We need the original tdata to resend a request on auth failure
+	 * or timeout.  On an auth failure, we use the original tdata
+	 * to initialize the new tdata for the authorized response.  On a timeout
+	 * we need it to skip failed SRV entries if any.
+	 */
+	pjsip_tx_data *last_tdata;
 	/*! \brief Timer entry for retrying on temporal responses */
 	pj_timer_entry timer;
 	/*! \brief Optional line parameter placed into Contact */
@@ -543,6 +551,13 @@ static pj_status_t registration_client_send(struct sip_outbound_registration_cli
 
 	/* Due to the message going out the callback may now be invoked, so bump the count */
 	ao2_ref(client_state, +1);
+	/*
+	 * We also bump tdata in expectation of saving it to client_state->last_tdata.
+	 * We have to do it BEFORE pjsip_regc_send because if it succeeds, it decrements
+	 * the ref count on its own.
+	 */
+	pjsip_tx_data_add_ref(tdata);
+
 	/*
 	 * Set the transport in case transports were reloaded.
 	 * When pjproject removes the extraneous error messages produced,
@@ -552,13 +567,26 @@ static pj_status_t registration_client_send(struct sip_outbound_registration_cli
 	pjsip_regc_set_transport(client_state->client, &selector);
 	status = pjsip_regc_send(client_state->client, tdata);
 
-	/* If the attempt to send the message failed and the callback was not invoked we need to
-	 * drop the reference we just added
+	/*
+	 * If the attempt to send the message failed and the callback was not invoked we need to
+	 * drop the references we just added
 	 */
 	if ((status != PJ_SUCCESS) && !(*callback_invoked)) {
+		pjsip_tx_data_dec_ref(tdata);
 		ao2_ref(client_state, -1);
+		return status;
 	}
 
+	/*
+	 * Decref the old last_data before replacing it.
+	 * BTW, it's quite possible that last_data == tdata
+	 * if we're trying successive servers in an SRV set.
+	 */
+	if (client_state->last_tdata) {
+		pjsip_tx_data_dec_ref(client_state->last_tdata);
+	}
+	client_state->last_tdata = tdata;
+
 	return status;
 }
 
@@ -1077,11 +1105,25 @@ static void sip_outbound_registration_response_cb(struct pjsip_regc_cbparam *par
 		retry_after = pjsip_msg_find_hdr(param->rdata->msg_info.msg, PJSIP_H_RETRY_AFTER,
 			NULL);
 		response->retry_after = retry_after ? retry_after->ivalue : 0;
+
+		/*
+		 * If we got a response from the server, we have to use the tdata
+		 * from the transaction, not the tdata saved when we sent the
+		 * request.  If we use the saved tdata, we won't process responses
+		 * like 423 Interval Too Brief correctly and we'll wind up sending
+		 * the bad Expires value again.
+		 */
+		pjsip_tx_data_dec_ref(client_state->last_tdata);
+
 		tsx = pjsip_rdata_get_tsx(param->rdata);
 		response->old_request = tsx->last_tx;
 		pjsip_tx_data_add_ref(response->old_request);
 		pjsip_rx_data_clone(param->rdata, 0, &response->rdata);
+	} else {
+		/* old_request steals the reference */
+		response->old_request = client_state->last_tdata;
 	}
+	client_state->last_tdata = NULL;
 
 	/*
 	 * Transfer response reference to serializer task so the
@@ -1127,6 +1169,9 @@ static void sip_outbound_registration_client_state_destroy(void *obj)
 	ast_taskprocessor_unreference(client_state->serializer);
 	ast_free(client_state->transport_name);
 	ast_free(client_state->registration_name);
+	if (client_state->last_tdata) {
+		pjsip_tx_data_dec_ref(client_state->last_tdata);
+	}
 }
 
 /*! \brief Allocator function for registration state */