[asterisk-commits] elguero: branch 1.8 r399794 - /branches/1.8/channels/chan_sip.c

[SVN commits to the Asterisk project]

Author: elguero
Date: Wed Sep 25 14:25:57 2013
New Revision: 399794

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=399794
Log:
Fix Realtime Peer Update Problem When Un-registering And Expires Header In 200ok

1st Issue
When a realtime peer sends an un-REGISTER request, Asterisk
un-registers the peer but the database table record still has regseconds and
fullcontact for the peer.  This results in calls attempting to be routed to the
peer which is no longer registered.  The expected behavior is to get
busy/congested when attempting to call an un-registered peer through the
dialplan.

What was discovered is that we are clearing out the peer's registration in the
database in parse_register_contact() when calling expire_register() but then
upon returning from parse_register_contact(), update_peer() is run which stores
back in the database table regseconds and fullcontact.

2nd Issue
The reporter pointed out that the 200 ok being returned by Asterisk
after un-registering a peer contains a Contact header with ;expires= and the
Expires header is not set to 0.  This is actually a regression.

Tests were created for this second issue (ASTERISK-22548).  The tests have been
reviewed and a Ship It! was received on those tests.

This patch does the following:

* Do not ignore the Expires header value even when it is set to 0.  The patch
  sets the pvt->expiry earlier on in the function so that it is set properly and
  used.

* If pvt->expiry is 0, do not call update_peer since that means the peer has
  already been un-registered and there is no need to update the database record
  again since nothing has changed.

(closes issue ASTERISK-22428)
Reported by: Ben Smithurst
Tested by: Ben Smithurst, Michael L. Young
Patches:
  asterisk-22428-rt-peer-update-and-expires-header.diff
                                              by Michael L. Young (license 5026)

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

Modified:
    branches/1.8/channels/chan_sip.c

Modified: branches/1.8/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/channels/chan_sip.c?view=diff&rev=399794&r1=399793&r2=399794
==============================================================================
--- branches/1.8/channels/chan_sip.c (original)
+++ branches/1.8/channels/chan_sip.c Wed Sep 25 14:25:57 2013
@@ -14549,6 +14549,14 @@
 		}
 	}
 
+	if (expire > max_expiry) {
+		expire = max_expiry;
+	}
+	if (expire < min_expiry && expire != 0) {
+		expire = min_expiry;
+	}
+	pvt->expiry = expire;
+
 	copy_socket_data(&pvt->socket, &req->socket);
 
 	do {
@@ -14687,12 +14695,6 @@
 	AST_SCHED_DEL_UNREF(sched, peer->expire,
 			unref_peer(peer, "remove register expire ref"));
 
-	if (expire > max_expiry) {
-		expire = max_expiry;
-	}
-	if (expire < min_expiry) {
-		expire = min_expiry;
-	}
 	if (peer->is_realtime && !ast_test_flag(&peer->flags[1], SIP_PAGE2_RTCACHEFRIENDS)) {
 		peer->expire = -1;
 	} else {
@@ -14702,7 +14704,6 @@
 			unref_peer(peer, "remote registration ref");
 		}
 	}
-	pvt->expiry = expire;
 	snprintf(data, sizeof(data), "%s:%d:%s:%s", ast_sockaddr_stringify(&peer->addr),
 		 expire, peer->username, peer->fullcontact);
 	/* We might not immediately be able to reconnect via TCP, but try caching it anyhow */
@@ -15465,7 +15466,10 @@
 						break;
 					case PARSE_REGISTER_UPDATE:
 						ast_string_field_set(p, fullcontact, peer->fullcontact);
-						update_peer(peer, p->expiry);
+						/* If expiry is 0, peer has been unregistered already */
+						if (p->expiry != 0) {
+							update_peer(peer, p->expiry);
+						}
 						/* Say OK and ask subsystem to retransmit msg counter */
 						transmit_response_with_date(p, "200 OK", req);
 						send_mwi = 1;