[reSIProcate] Potential bug in TransportSelector

[Scott Godin]

I suspect the if statement is only there so that we never add a
transport=udp parameter (since it is the default).  Instead of removing
the if, we could just add an else clause:

 

else

{

   contact.uri().remove(p_transport)

}

 

Scott

 

From: resiprocate-devel-bounces at list.resiprocate.org
[mailto:resiprocate-devel-bounces at list.resiprocate.org] On Behalf Of
Jason Fischl
Sent: Friday, February 09, 2007 2:20 PM
To: resiprocate
Subject: [reSIProcate] Potential bug in TransportSelector

 

In TransportSelector::transmit we are seeing a bug in the following
case: 

UAC sends a SUBSCRIBE over transport at TCP:5060 and puts the correct
contact in the message
UAC receives 200/SUBSCRIBE with a Record-Route of UDP:5061 

With the following code, we correctly send over UDP to target 5061 but
we leave transport=tcp in the Contact which causes a problem for the UAS
since it looks like a Contact refresh and subsequent requests from the
peer are sent to TCP:5061 where there is no listener. 



      if (target.transport)
      {
         // There is a contact header and it contains exactly one entry
         if (msg->exists(h_Contacts) &&
msg->header(h_Contacts).size()==1) 
         {
            for (NameAddrs::iterator i=msg->header(h_Contacts).begin();
i != msg->header(h_Contacts).end(); i++)
            {
               NameAddr& contact = *i;
               // No host specified, so use the ip address and port of
the 
               // transport used. Otherwise, leave it as is.
               if (contact.uri().host().empty())
               {
                  contact.uri().host() =
(target.transport->hasSpecificContact() ? 
 
target.transport->interfaceName() : 
                                          Tuple::inet_ntop(source) );
                  contact.uri().port() = target.transport->port(); 

                  if (target.transport->transport() != UDP)
                  {
                     contact.uri().param(p_transport) =
Tuple::toData(target.transport->transport());
                  }


My proposal is to remove the if (target.transport->transport() != UDP).
Does anybody remember why this is here? 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20070209/f61cc21a/attachment.htm>