From 2ceaf1dd6ff4809246adf0f97d92244dcc1907e6 Mon Sep 17 00:00:00 2001
From: Alexandre Savard <alexandre.savard@savoirfairelinux.com>
Date: Thu, 26 Apr 2012 16:27:28 -0400
Subject: [PATCH] #9980: Fix registration timer and transport shutdown on 401,
 default registration timer to 3600

---
 daemon/src/sip/sipaccount.cpp  | 50 ++++++++++++--------
 daemon/src/sip/sipaccount.h    |  2 +
 daemon/src/sip/sipvoiplink.cpp | 85 ++++++++++++++++++++++------------
 3 files changed, 88 insertions(+), 49 deletions(-)

diff --git a/daemon/src/sip/sipaccount.cpp b/daemon/src/sip/sipaccount.cpp
index e553300671..8d259ff77a 100644
--- a/daemon/src/sip/sipaccount.cpp
+++ b/daemon/src/sip/sipaccount.cpp
@@ -42,13 +42,15 @@
 #include "manager.h"
 #include <pwd.h>
 #include <sstream>
+#include <stdlib.h>
 
 const char * const SIPAccount::IP2IP_PROFILE = "IP2IP";
 const char * const SIPAccount::OVERRTP_STR = "overrtp";
 const char * const SIPAccount::SIPINFO_STR = "sipinfo";
 
 namespace {
-    const int MIN_REGISTRATION_TIME = 600;
+    const int MIN_REGISTRATION_TIME = 60;
+    const int DEFAULT_REGISTRATION_TIME = 3600;
 }
 
 SIPAccount::SIPAccount(const std::string& accountID)
@@ -97,6 +99,7 @@ SIPAccount::SIPAccount(const std::string& accountID)
     , zrtpNotSuppWarning_(true)
     , registrationStateDetailed_()
     , keepAliveTimer_()
+    , keepAliveTimerActive_(false)
     , link_(SIPVoIPLink::instance())
     , receivedParameter_()
     , rPort_(-1)
@@ -402,7 +405,6 @@ void SIPAccount::setAccountDetails(std::map<std::string, std::string> details)
     localPort_ = atoi(details[CONFIG_LOCAL_PORT].c_str());
     publishedPort_ = atoi(details[CONFIG_PUBLISHED_PORT].c_str());
     if (stunServer_ != details[CONFIG_STUN_SERVER]) {
-        DEBUG("Stun server changed!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!");
         link_->sipTransport.destroyStunResolver(stunServer_);
         // pj_stun_sock_destroy(pj_stun_sock *stun_sock);
     }
@@ -410,6 +412,8 @@ void SIPAccount::setAccountDetails(std::map<std::string, std::string> details)
     stunEnabled_ = details[CONFIG_STUN_ENABLE] == "true";
     dtmfType_ = details[CONFIG_ACCOUNT_DTMF_TYPE];
     registrationExpire_ = atoi(details[CONFIG_ACCOUNT_REGISTRATION_EXPIRE].c_str());
+    if(registrationExpire_ < MIN_REGISTRATION_TIME)
+        registrationExpire_ = MIN_REGISTRATION_TIME;
 
     userAgent_ = details[CONFIG_ACCOUNT_USERAGENT];
 
@@ -585,7 +589,13 @@ void SIPAccount::startKeepAliveTimer() {
     if (isTlsEnabled())
         return;
 
-    DEBUG("SIP ACCOUNT: start keep alive timer");
+    if (isIP2IP())
+        return;
+
+    if(keepAliveTimerActive_)
+        return;
+
+    DEBUG("SipAccount: start keep alive timer for account %s", getAccountID().c_str());
 
     // make sure here we have an entirely new timer
     memset(&keepAliveTimer_, 0, sizeof(pj_timer_entry));
@@ -593,25 +603,31 @@ void SIPAccount::startKeepAliveTimer() {
     pj_time_val keepAliveDelay_;
     keepAliveTimer_.cb = &SIPAccount::keepAliveRegistrationCb;
     keepAliveTimer_.user_data = this;
+    keepAliveTimer_.id = rand();
 
     // expiration may be undetermined during the first registration request
     if (registrationExpire_ == 0) {
-        DEBUG("Registration Expire == 0, take 60");
-        keepAliveDelay_.sec = 60;
+        DEBUG("SipAccount: Registration Expire: 0, taking 60 instead");
+        keepAliveDelay_.sec = 3600;
     }
     else {
-        DEBUG("Registration Expire == %d", registrationExpire_);
-        keepAliveDelay_.sec = registrationExpire_;
+        DEBUG("SipAccount: Registration Expire: %d", registrationExpire_);
+        keepAliveDelay_.sec = registrationExpire_ + MIN_REGISTRATION_TIME;
     }
 
-
     keepAliveDelay_.msec = 0;
 
+    keepAliveTimerActive_ = true;
+
     link_->registerKeepAliveTimer(keepAliveTimer_, keepAliveDelay_);
 }
 
 void SIPAccount::stopKeepAliveTimer() {
-     link_->cancelKeepAliveTimer(keepAliveTimer_);
+    DEBUG("SipAccount: stop keep alive timer %d for account %s", keepAliveTimer_.id, getAccountID().c_str());
+
+    keepAliveTimerActive_ = false;
+
+    link_->cancelKeepAliveTimer(keepAliveTimer_);
 }
 
 pjsip_ssl_method SIPAccount::sslMethodStringToPjEnum(const std::string& method)
@@ -677,7 +693,7 @@ void SIPAccount::initStunConfiguration()
 void SIPAccount::loadConfig()
 {
     if (registrationExpire_ == 0)
-        registrationExpire_ = MIN_REGISTRATION_TIME; /** Default expire value for registration */
+        registrationExpire_ = DEFAULT_REGISTRATION_TIME; /** Default expire value for registration */
 
     if (tlsEnable_ == "true") {
         initTlsConfiguration();
@@ -837,6 +853,8 @@ void SIPAccount::keepAliveRegistrationCb(UNUSED pj_timer_heap_t *th, pj_timer_en
 {
     SIPAccount *sipAccount = static_cast<SIPAccount *>(te->user_data);
 
+    ERROR("SipAccount: Keep alive registration callback for account %s", sipAccount->getAccountID().c_str());
+
     if (sipAccount == NULL) {
         ERROR("Sip account is NULL while registering a new keep alive timer");
         return;
@@ -850,16 +868,10 @@ void SIPAccount::keepAliveRegistrationCb(UNUSED pj_timer_heap_t *th, pj_timer_en
     if (sipAccount->isTlsEnabled())
         return;
 
-    if (sipAccount->isRegistered()) {
-        // send a new register request
-        sipAccount->registerVoIPLink();
+    sipAccount->stopKeepAliveTimer();
 
-        // make sure the current timer is deactivated
-        sipAccount->stopKeepAliveTimer();
-
-        // register a new timer
-        sipAccount->startKeepAliveTimer();
-    }
+    if (sipAccount->isRegistered())
+        sipAccount->registerVoIPLink();
 }
 
 namespace {
diff --git a/daemon/src/sip/sipaccount.h b/daemon/src/sip/sipaccount.h
index 6b97d5b1cc..733eb24185 100644
--- a/daemon/src/sip/sipaccount.h
+++ b/daemon/src/sip/sipaccount.h
@@ -736,6 +736,8 @@ class SIPAccount : public Account {
          */
         pj_timer_entry keepAliveTimer_;
 
+        bool keepAliveTimerActive_;
+
 
         /**
          * Voice over IP Link contains a listener thread and calls
diff --git a/daemon/src/sip/sipvoiplink.cpp b/daemon/src/sip/sipvoiplink.cpp
index e85bc66236..1dc9c62250 100644
--- a/daemon/src/sip/sipvoiplink.cpp
+++ b/daemon/src/sip/sipvoiplink.cpp
@@ -618,6 +618,8 @@ void SIPVoIPLink::sendUnregister(Account *a)
 
 void SIPVoIPLink::registerKeepAliveTimer(pj_timer_entry &timer, pj_time_val &delay)
 {
+    DEBUG("UserAgent: Register new keep alive timer %d with delay %d", timer.id, delay.sec);
+
     if (timer.id == -1)
         WARN("UserAgent: Timer already scheduled");
 
@@ -1586,19 +1588,36 @@ void update_contact_header(pjsip_regc_cbparam *param, SIPAccount *account)
     pj_pool_release(pool);
 }
 
-void lookForReceivedParameter(pjsip_regc_cbparam *param, SIPAccount *account)
+static void lookForReceivedParameter(pjsip_regc_cbparam *param, SIPAccount *account)
 {
     pj_str_t receivedValue = param->rdata->msg_info.via->recvd_param;
 
     if (receivedValue.slen) {
         std::string publicIpFromReceived(receivedValue.ptr, receivedValue.slen);
-        DEBUG("Cool received received parameter... uhhh?, the value is %s", publicIpFromReceived.c_str());
         account->setReceivedParameter(publicIpFromReceived);
     }
 
     account->setRPort(param->rdata->msg_info.via->rport_param);
 }
 
+static void processRegistrationError(pjsip_regc_cbparam *param, SIPAccount *account, const RegistrationState &state)
+{
+    if(param == NULL) {
+        ERROR("UserAgent: param is NULL while processing registration error");
+        return;
+    }
+
+    if(account == NULL) {
+        ERROR("UserAgent: Account is NULL while processing registration error");
+        return;
+    }
+
+    account->stopKeepAliveTimer();
+    account->setRegistrationState(ErrorAuth);
+    account->setRegister(false);
+    SIPVoIPLink::instance()->sipTransport.shutdownSipTransport(*account);
+}
+
 void registration_cb(pjsip_regc_cbparam *param)
 {
     if (param == NULL) {
@@ -1607,12 +1626,13 @@ void registration_cb(pjsip_regc_cbparam *param)
     }
 
     SIPAccount *account = static_cast<SIPAccount *>(param->token);
-
     if (account == NULL) {
         ERROR("SipVoipLink: account doesn't exist in registration callback");
         return;
     }
 
+    std::string accountid = account->getAccountID();
+
     if (account->isContactUpdateEnabled())
         update_contact_header(param, account);
 
@@ -1628,47 +1648,52 @@ void registration_cb(pjsip_regc_cbparam *param)
     }
 
     if (param->status != PJ_SUCCESS) {
-        account->setRegistrationState(ErrorAuth);
-        account->setRegister(false);
-        SIPVoIPLink::instance()->sipTransport.shutdownSipTransport(*account);
+        ERROR("UserAgent: Could not register account %s with error %d", accountid.c_str(), param->code);
+        processRegistrationError(param, account, ErrorAuth);
         return;
     }
 
     if (param->code < 0 || param->code >= 300) {
         switch (param->code) {
-            case PJSIP_SC_NOT_ACCEPTABLE_ANYWHERE:
-                lookForReceivedParameter(param, account);
-                account->setRegistrationState(ErrorNotAcceptable);
-                SIPVoIPLink::instance()->sendRegister(account);
+            case PJSIP_SC_MULTIPLE_CHOICES: // 300
+            case PJSIP_SC_MOVED_PERMANENTLY: // 301
+            case PJSIP_SC_MOVED_TEMPORARILY: // 302
+            case PJSIP_SC_USE_PROXY: // 305
+            case PJSIP_SC_ALTERNATIVE_SERVICE: // 380
+                ERROR("UserAgent: Could not register account %s with error %d", accountid.c_str(), param->code);
+                processRegistrationError(param, account, Error);
                 break;
-
-            case PJSIP_SC_SERVICE_UNAVAILABLE:
-            case PJSIP_SC_REQUEST_TIMEOUT:
-                account->setRegistrationState(ErrorHost);
-                account->setRegister(false);
-                SIPVoIPLink::instance()->sipTransport.shutdownSipTransport(*account);
+            case PJSIP_SC_SERVICE_UNAVAILABLE: // 503
+                ERROR("UserAgent: Could not register account %s with error %d", accountid.c_str(), param->code);
+                processRegistrationError(param, account, ErrorHost);
                 break;
-
-            case PJSIP_SC_UNAUTHORIZED:
-            case PJSIP_SC_FORBIDDEN:
-            case PJSIP_SC_NOT_FOUND:
-                account->setRegistrationState(ErrorAuth);
-                account->setRegister(false);
-                SIPVoIPLink::instance()->sipTransport.shutdownSipTransport(*account);
+            case PJSIP_SC_UNAUTHORIZED: // 401
+                // Automatically answered by PJSIP
+                account->registerVoIPLink();
                 break;
-
-            case PJSIP_SC_INTERVAL_TOO_BRIEF:
+            case PJSIP_SC_FORBIDDEN: // 403
+            case PJSIP_SC_NOT_FOUND: // 404
+                ERROR("UserAgent: Could not register account %s with error %d", accountid.c_str(), param->code);
+                processRegistrationError(param, account, ErrorAuth);
+                break;
+            case PJSIP_SC_REQUEST_TIMEOUT: // 408
+                ERROR("UserAgent: Could not register account %s with error %d", accountid.c_str(), param->code);
+                processRegistrationError(param, account, ErrorHost);
+                break;
+            case PJSIP_SC_INTERVAL_TOO_BRIEF: // 423
                 // Expiration Interval Too Brief
                 account->doubleRegistrationExpire();
                 account->registerVoIPLink();
                 account->setRegister(false);
-                SIPVoIPLink::instance()->sipTransport.shutdownSipTransport(*account);
                 break;
-
+            case PJSIP_SC_NOT_ACCEPTABLE_ANYWHERE: // 606
+                lookForReceivedParameter(param, account);
+                account->setRegistrationState(ErrorNotAcceptable);
+                account->registerVoIPLink();
+                break;
             default:
-                account->setRegistrationState(Error);
-                account->setRegister(false);
-                SIPVoIPLink::instance()->sipTransport.shutdownSipTransport(*account);
+                ERROR("UserAgent: Could not register account %s with error %d", param->code);
+                processRegistrationError(param, account, Error);
                 break;
         }
 
-- 
GitLab