[h323plus] H323EndPoint::InternalMakeCall bug

[Iurii Gordiienko]

Hi, 
I have found an issue with H323EndPoint::InternalMakeCall function for case when it running from H323EndPoint::ForwardConnection function (may be for some others cases too). Take a look: 1.We are trying to connect to some host which uses Gatekeeper. 2.From H323EndPoint::ForwardConnection call H323EndPoint::InternalMakeCall function with some internal token="ip$xxx.xxx.xxx.xxx/port" 3.In the body of H323EndPoint::InternalMakeCall we have the replacement current callToken-connection pair to new one with "ip$xxx.xxx.xxx.xxx/port-replaced-1" string, like          connectionsActive.SetAt(adjustedToken, connectionsActive.RemoveAt(newToken));     connectionsToBeCleaned += adjustedToken; 
4.But if for some reason our virtual CreateConnection function returns NULL... 
  connection = CreateConnection(lastReference, userData, transport, NULL); 
  if (connection == NULL) {     PTRACE(1, "H323\tCreateConnection returned NULL");     connectionsMutex.Signal(); 

    return NULL;   } 
 ... we cannot clear current Connection because it has not actual callToken "ip$xxx.xxx.xxx.xxx/port" but H323EndPoint::connectionsActive has "ip$xxx.xxx.xxx.xxx/port-replaced-1" and endpoint.ClearCall(callToken, reason) or something similar will be failed because not able to found the Connection by own callToken. 
5.Also we have redundant call connectionsMutex.Signal(). 
if (connection == NULL) {     PTRACE(1, "H323\tCreateConnection returned NULL");     connectionsMutex.Signal(); 

    return NULL;   } 

My proposition - just rename back callToken for H323EndPoint::connectionsActive and remove adjustedToken from H323EndPoint::connectionsToBeCleaned if we have NULL connection. My H323EndPoint::InternalMakeCall function looks like this: 
H323Connection * H323EndPoint::InternalMakeCall(const PString & trasferFromToken,                                                 const PString & callIdentity,                                                 unsigned capabilityLevel,                                                 const PString & remoteParty,                                                 H323Transport * transport,                                                 PString & newToken,                                                 void * userData,                                                 PBoolean supplimentary                                                 ) {   PTRACE(2, "H323\tMaking call to: " << remoteParty); 
  PString alias;   H323TransportAddress address;   if (!ParsePartyName(remoteParty, alias, address)) {     PTRACE(2, "H323\tCould not parse \"" << remoteParty << '"');     return NULL;   } 
#ifdef H323_H46017   // If H.460.17 use the existing H.460.17 Transport   if (transport == NULL && RegisteredWithH46017())       transport = GetH46017Transport(); #endif 
  if (transport == NULL) {     // Restriction: the call must be made on the same transport as the one     // that the gatekeeper is using.     if (gatekeeper != NULL)       transport = gatekeeper->GetTransport().GetRemoteAddress().CreateTransport(*this); 
    // assume address is an IP address/hostname     else       transport = address.CreateTransport(*this); 
    if (transport == NULL) {       PTRACE(1, "H323\tInvalid transport in \"" << remoteParty << '"');       return NULL;     }   } 
  H323Connection * connection; 
  connectionsMutex.Wait(); 
  PString adjustedToken;   unsigned lastReference;   if (newToken.IsEmpty()) {     do {       lastReference = Q931::GenerateCallReference();       newToken = BuildConnectionToken(*transport, lastReference, FALSE);     } while (connectionsActive.Contains(newToken));   }   else {     lastReference = newToken.Mid(newToken.Find('/')+1).AsUnsigned(); 
    // Move old connection on token to new value and flag for removal     unsigned tieBreaker = 0;     do {       adjustedToken = newToken + "-replaced";       adjustedToken.sprintf("-%u", ++tieBreaker);     } while (connectionsActive.Contains(adjustedToken));     connectionsActive.SetAt(adjustedToken, connectionsActive.RemoveAt(newToken));     connectionsToBeCleaned += adjustedToken;     PTRACE(3, "H323\tOverwriting call " << newToken << ", renamed to " << adjustedToken);   }   connectionsMutex.Signal(); 
  connection = CreateConnection(lastReference, userData, transport, NULL);   if (connection == NULL) {     PTRACE(1, "H323\tCreateConnection returned NULL");          if (!adjustedToken.IsEmpty())     {         connectionsMutex.Wait(); 
        connectionsActive.SetAt(newToken, connectionsActive.RemoveAt(adjustedToken));         connectionsToBeCleaned -= adjustedToken; 
        PTRACE(3, "H323\tOverwriting call " << adjustedToken << ", renamed to " << newToken); 
        connectionsMutex.Signal();     } 
    return NULL;   }   connection->SetRemotePartyName(remoteParty); 
  if (supplimentary)        connection->SetNonCallConnection();      
  connection->Lock(); 
  connectionsMutex.Wait();   connectionsActive.SetAt(newToken, connection); 
  connectionsMutex.Signal(); 
  connection->AttachSignalChannel(newToken, transport, FALSE); 
#ifdef H323_H450   if (capabilityLevel == UINT_MAX)     connection->HandleTransferCall(trasferFromToken, callIdentity);   else {     connection->HandleIntrudeCall(trasferFromToken, callIdentity);     connection->IntrudeCall(capabilityLevel);   } #endif 
  PTRACE(3, "H323\tCreated new connection: " << newToken); 
#ifdef H323_H46017   if (RegisteredWithH46017()) {     H323Connection::CallEndReason reason = connection->SendSignalSetup(alias, address);     if (reason != H323Connection::NumCallEndReasons)       connection->ClearCall(reason);   } else #endif       new H225CallThread(*this, *connection, *transport, alias, address); 
  return connection; } 



Thanks -- Iurii Gordiienko 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.packetizer.com/pipermail/h323plus/attachments/20151231/3420a07f/attachment-0002.html>