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
Iurii
Fix checked into CVS.
Simon
From: h323plus [mailto:h323plus-bounces@lists.packetizer.com] On Behalf Of Iurii Gordiienko Sent: Thursday, December 31, 2015 8:40 PM To: h323plus@lists.packetizer.com Subject: [h323plus] H323EndPoint::InternalMakeCall bug
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
participants (2)
-
Iurii Gordiienko
-
Simon Horne