[reSIProcate] Potential bug in TransportSelector

[Byron Campen]

	I feel uneasy about touching the transport param at all; perhaps the  
UAC put one into its Contact for a reason!  And if there wasn't one  
there in the first place, I think it should be safe to assume the UAC  
doesn't care what protocol stuff comes in on, so we shouldn't go  
adding one. I assume that the UAC is at least aware of what kinds of  
transports (TCP, UDP,TLS,...) it is listening on, and if its  
transport loadout is somehow incomplete, it should be prepared to  
compensate for its lack of compliance by putting explicit transport  
params into the Contact. My vote would be to remove the code block  
altogether.

Best regards,
Byron Campen


> 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?
>
> _______________________________________________
> resiprocate-devel mailing list
> resiprocate-devel at list.resiprocate.org
> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20070209/96f30f0b/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 2423 bytes
Desc: not available
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20070209/96f30f0b/attachment.bin>