[asterisk-commits] mmichelson: trunk r356477 - in /trunk: ./ channels/chan_sip.c

[SVN commits to the Asterisk project]

Author: mmichelson
Date: Thu Feb 23 09:49:13 2012
New Revision: 356477

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=356477
Log:
Fix ACK routing for non-2xx responses.

When we send an ACK for a 2xx response to an INVITE, we are supposed
to use the learned route set. However, when we receive a non-2xx final
response to an INVITE, we are supposed to send the ACK to the same place
we initially sent the INVITE.

We had been doing this up until the changes went in that would build a route
set from provisional responses. That introduced a regression where we would
use the learned route set under all circumstances.

With this change, we now will set the destination of our ACK based on the
invitestate. If it is INV_COMPLETED then that means that we have received
a non-2xx final response (INV_TERMINATED indicates a 2xx response was received).
If it is INV_CANCELLED, then that means the call is being canceled, which
means that we should be ACKing a 487 response.

The other change introduced here is setting the invitestate to INV_CONFIRMED
when we send an ACK *after* the reqprep instead of before. This way, we can
tell in reqprep more easily what the invitestate is prior to sending the ACK.

(closes issue ASTERISK-19389)
reported by Karsten Wemheuer
patches:
    ASTERISK-19389v2.patch uploaded by Mark Michelson (license #5049)
	(with some slight modifications prior to commit)
........

Merged revisions 356475 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........

Merged revisions 356476 from http://svn.asterisk.org/svn/asterisk/branches/10

Modified:
    trunk/   (props changed)
    trunk/channels/chan_sip.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-10-merged' - no diff available.

Modified: trunk/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/trunk/channels/chan_sip.c?view=diff&rev=356477&r1=356476&r2=356477
==============================================================================
--- trunk/channels/chan_sip.c (original)
+++ trunk/channels/chan_sip.c Thu Feb 23 09:49:13 2012
@@ -10666,10 +10666,14 @@
 
 	add_header(req, "Via", p->via);
 	/*
-	 * Use the learned route set unless this is a CANCEL. For a CANCEL
-	 * we have to send to the same destination as the original INVITE.
+	 * Use the learned route set unless this is a CANCEL on an ACK for a non-2xx
+	 * final response. For a CANCEL or ACK, we have to send to the same destination
+	 * as the original INVITE.
 	 */
-	if (p->route && sipmethod != SIP_CANCEL) {
+	if (sipmethod == SIP_CANCEL ||
+			(sipmethod == SIP_ACK && (p->invitestate == INV_COMPLETED || p->invitestate == INV_CANCELLED))) {
+		set_destination(p, ast_strdupa(p->uri));
+	} else if (p->route) {
 		set_destination(p, p->route->hop);
 		add_route(req, is_strict ? p->route->next : p->route);
 	}
@@ -14055,13 +14059,13 @@
 {
 	struct sip_request resp;
 	
-	if (sipmethod == SIP_ACK) {
-		p->invitestate = INV_CONFIRMED;
-	}
-
 	reqprep(&resp, p, sipmethod, seqno, newbranch);
 	if (sipmethod == SIP_CANCEL && p->answered_elsewhere) {
 		add_header(&resp, "Reason", "SIP;cause=200;text=\"Call completed elsewhere\"");
+	}
+
+	if (sipmethod == SIP_ACK) {
+		p->invitestate = INV_CONFIRMED;
 	}
 
 	return send_request(p, &resp, reliable, seqno ? seqno : p->ocseq);