Properly ACK 487 responses to canceled INVITEs.

From the review board request:
The fix from review 298 has exposed a new bug in chan_sip.

When we hang up an outgoing call, we first will dump all the outstanding 
packets on the sip_pvt using __sip_pretend_ack. Then, if we can, we send 
a CANCEL. The problem with this is that since destroyed all the outstanding 
packets on the dialog, we cannot match the incoming 487 response to our 
INVITE. Because we cannot match the response, we do not send an ACK.

To correct this, instead of using __sip_pretend_ack, I have changed the code 
to loop through the list of packets and call __sip_semi_ack on each one 
instead. This causes us to stop retransmitting the requests, but we still have 
them around in case we get responses for them.

When discussing this earlier today with Josh Colp, we both agreed that in the 
majority of cases, this would be enough of a fix. However, we also agreed that 
we should have a safety net in place in case we never receive a response to 
our initial INVITE. To handle this, I have modified __sip_autodestruct to 
behave similar to the way it does in Asterisk 1.4. If there are outstanding 
packets on the sip_pvt, the needdestroy flag is not set, and the last request 
on the dialog was either a CANCEL or BYE, then we set the needdestroy flag and 
reschedule destruction for 10 seconds in the future. If, though, the 
needdestroy flag is set, then we use __sip_pretend_ack to kill the remaining 
outstanding packets so that the monitor thread can destroy the sip_pvt.

I ran two separate tests. First, I placed a call from my Aastra phone to my 
Polycom phone. I hung up the Aastra before the Polycom answered. I verified 
through sip debug output that Asterisk properly ACKed the 487 received from the 
Polycom.

For my second test, I set up a SIPp UAS scenario so that it would send a 200 OK 
in response to a CANCEL but would not send a 487 for the original INVITE. I 
verified that after about 40 seconds, Asterisk properly cleans up the outgoing 
sip_pvt for the call.

Review: https://reviewboard.asterisk.org/r/308



git-svn-id: http://svn.digium.com/svn/asterisk/branches/1.4@205877 f38db490-d61c-443f-a65b-d21fe96a405b

1 files changed, 9 insertions, 3 deletions

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 78c39a1c3..0c403ea2c 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -2120,11 +2120,14 @@ static int __sip_autodestruct(const void *data)
 	* that were created via INVITE, then thru some sequence were CANCELED,
 	* to die, rather than infinitely be rescheduled */
 	if (p->packets && !ast_test_flag(&p->flags[0], SIP_NEEDDESTROY)) {
+		char method_str[30];
 		if (option_debug > 2)
 			ast_log(LOG_DEBUG, "Re-scheduled destruction of SIP call %s\n", p->callid ? p->callid : "<unknown>");
 		append_history(p, "ReliableXmit", "timeout");
-		if (p->method == SIP_CANCEL || p->method == SIP_BYE) {
-			ast_set_flag(&p->flags[0], SIP_NEEDDESTROY);
+		if (sscanf(p->lastmsg, "Tx: %s", method_str) == 1 || sscanf(p->lastmsg, "Rx: %s", method_str) == 1) {
+			if (method_match(SIP_CANCEL, method_str) || method_match(SIP_BYE, method_str)) {
+				ast_set_flag(&p->flags[0], SIP_NEEDDESTROY);
+			}
 		}
 		return 10000;
 	}
@@ -3665,7 +3668,10 @@ static int sip_hangup(struct ast_channel *ast)
 		if (needcancel) {	/* Outgoing call, not up */
 			if (ast_test_flag(&p->flags[0], SIP_OUTGOING)) {
 				/* stop retransmitting an INVITE that has not received a response */
-				__sip_pretend_ack(p);
+				struct sip_pkt *cur;
+				for (cur = p->packets; cur; cur = cur->next) {
+					__sip_semi_ack(p, cur->seqno, ast_test_flag(cur, FLAG_RESPONSE), cur->method ? cur->method : find_sip_method(cur->data));
+				}
 
 				/* if we can't send right now, mark it pending */
 				if (p->invitestate == INV_CALLING) {