chan_sip: Fix Realtime Peer Update Problem When Un-registering And Expires Header...
authorMichael L. Young <elgueromexicano@gmail.com>
Wed, 25 Sep 2013 19:29:38 +0000 (19:29 +0000)
committerMichael L. Young <elgueromexicano@gmail.com>
Wed, 25 Sep 2013 19:29:38 +0000 (19:29 +0000)
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/
........

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

Merged revisions 399795 from http://svn.asterisk.org/svn/asterisk/branches/11
........

Merged revisions 399796 from http://svn.asterisk.org/svn/asterisk/branches/12

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@399797 65c4cc65-6c06-0410-ace0-fbb531ad65f3

channels/chan_sip.c

index d25a47a..aaae83c 100644 (file)
@@ -16107,6 +16107,14 @@ static enum parse_register_result parse_register_contact(struct sip_pvt *pvt, st
                }
        }
 
+       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 {
@@ -16246,12 +16254,6 @@ static enum parse_register_result parse_register_contact(struct sip_pvt *pvt, st
        AST_SCHED_DEL_UNREF(sched, peer->expire,
                        sip_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 {
@@ -16261,7 +16263,6 @@ static enum parse_register_result parse_register_contact(struct sip_pvt *pvt, st
                        sip_unref_peer(peer, "remote registration ref");
                }
        }
-       pvt->expiry = expire;
        if (!build_path(pvt, peer, req, NULL)) {
                /* Tell the dialog to use the Path header in the response */
                ast_set2_flag(&pvt->flags[0], 1, SIP_USEPATH);
@@ -17283,7 +17284,10 @@ static enum check_auth_result register_verify(struct sip_pvt *p, struct ast_sock
                                                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;