Bugfix/issue 66 (#252)

* bugfix #247 - sdp was being truncated on uac invite * additional test cases
drachtio · Jan 23, 2023 · c84fb08 · c84fb08
1 parent 4054dbc
commit c84fb08
Show file tree

Hide file tree

Showing 9 changed files with 361 additions and 18 deletions.
diff --git a/src/sip-dialog-controller.cpp b/src/sip-dialog-controller.cpp
@@ -177,7 +177,7 @@ namespace drachtio {
             dlg->getTransportDesc(transport) ;
             tags = makeTags( pData->getHeaders(), transport) ;
 
-            tport_t* tp = dlg->getTport() ; 
+            tport_t* tp = dlg->getTport() ; //DH: this does NOT take out a reference
             bool forceTport = NULL != tp ;  
 
             nta_leg_t *leg = const_cast<nta_leg_t *>(dlg->getNtaLeg());
@@ -201,6 +201,8 @@ namespace drachtio {
                 std::shared_ptr<UaInvalidData> pData = m_pController->findTportForSubscription( target->m_url->url_user, target->m_url->url_host ) ;
                 if( NULL != pData ) {
                     DR_LOG(log_debug) << "SipDialogController::doSendRequestInsideDialog found cached tport for this client " << std::hex << (void *) pData->getTport();
+                    //DH: I am now holding a tport that I did not take out a reference for
+                    //what if while I am holding it the registration expires and the tport is destroyed?
                     if (pData->getTport() != tp) {
                         DR_LOG(log_info) << "SipDialogController::doSendRequestInsideDialog client has done a mid-call handoff; tp is now " << std::hex << (void *) pData->getTport();
                         tp = pData->getTport();

diff --git a/src/sip-dialog-controller.hpp b/src/sip-dialog-controller.hpp
@@ -43,7 +43,7 @@ THE SOFTWARE.
 
 #define START_LEN (512)
 #define HDR_LEN (4192)
-#define BODY_LEN (8384)
+#define BODY_LEN (12288)
 
 namespace drachtio {
 

diff --git a/src/sip-dialog.cpp b/src/sip-dialog.cpp
@@ -226,6 +226,7 @@ namespace drachtio {
 
   void SipDialog::setTport(tport_t* tp) {
 		if (m_tp) tport_unref(m_tp);
+    //DH: Why are we not taking a reference to the tport?
     m_tp = tp ;
     const tp_name_t* tpn = tport_name( m_tp );
 

diff --git a/test/run_tests.js b/test/run_tests.js
@@ -2,10 +2,15 @@ const test = require('tape');
 const execCmd = require('./utils/exec');
 const delay = require('./utils/delay');
 const {start, stop } = require('./testbed');
-const logger = require('pino')({level: 'info'});
+const logger = require('pino')({level: 'debug'});
 let configFile, drachtio;
 const manageServer = !process.env.NOSERVER;
 
+/*
+process.on('uncaughtException', (err, origin) => {
+  console.log({err, origin}, 'uncaught exception');
+});
+*/
 module.exports = async function runTests(testName) {
   const fixtures = require(`./test-fixtures-${testName}`);
   logger.debug(`starting test suite ${testName}, manageServer: ${manageServer}`);
@@ -77,7 +82,7 @@ function runFixture(f) {
           if (f.script.function) {
             const args = f.script.args || (f.uas ? `127.0.0.1:${f.uas.port}` : undefined);
             scriptPromise = script[f.script.function](args, f.script.opts || {});
-            await delay(750);
+            if (f.script.delay) await delay(f.script.delay);
           }
         }
         if (f.uac) {
@@ -87,15 +92,22 @@ function runFixture(f) {
           uacPromise = execCmd(cmd, {cwd: './scenarios'});
         }
 
-        if (uacPromise) await uacPromise;
-        else if (scriptPromise) {
-          try {
-            await scriptPromise;
+        try {
+          const promises = [];
+          [uasPromise, scriptPromise, uacPromise].forEach((p) => {
+            p && promises.push(p);
+          });
+          if (promises.length > 0) {
+            logger.debug(`waiting for ${promises.length} promises to resolve..`);
+            await Promise.all(promises);
           }
-          catch (err) {
-            if (![err, '*'].includes(f.script.error)) throw err;
+        } catch (err) {
+          logger.debug({err}, 'caught error in script');
+          if (![err.message, err, '*'].includes(f.script.error)) {
+            logger.error('unexpected error in script - rethrowing');
+            throw err;
           }
-        }
+        };
 
         try {
           if (script) script.disconnect();

diff --git a/test/scenarios/register-bad-contact.xml b/test/scenarios/register-bad-contact.xml
@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="ISO-8859-1" ?>
+<!DOCTYPE scenario SYSTEM "sipp.dtd">
+
+<scenario name="Basic Sipstone UAC">
+  <!-- In client mode (sipp placing calls), the Call-ID MUST be         -->
+  <!-- generated by sipp. To do so, use [call_id] keyword.                -->
+  <send retrans="500">
+    <![CDATA[
+
+      REGISTER sip:[service]@[remote_ip]:[remote_port] SIP/2.0
+      Via: SIP/2.0/[transport] [local_ip]:[local_port];branch=[branch]
+      From: sipp <sip:sipp@[local_ip]:[local_port]>;tag=80830557
+      To: [service] <sip:[service]@[remote_ip]:[remote_port]>
+      Call-ID: [call_id]
+      CSeq: 1 REGISTER
+      Subject: uac-register-subscribe-publish
+      Contact: %
+      Expires: 3600
+      Content-Length: 0
+
+    ]]>
+  </send>
+
+  <recv response="403"/>
+
+</scenario>