[Serusers] Re: [Serdev] Bug? Inconsistent return of loose_route function

[Cesc Santasusana]

Hi,

Ok ... so it is a bug ... now how to fix it? 

> static inline int after_loose(struct sip_msg* _m, int preloaded)
>            ..... 
>            if (enable_double_rr && is_2rr(&puri.params)) {
>                            ....
>                            if (res > 0) { /* No next route found */
>                                         DBG("after_loose: No next URI found\n");
>                                         return (preloaded ? NOT_RR_DRIVEN : RR_DRIVEN);
>                                       or simply 
>                                         return (preloaded RR_DRIVEN);    

Cesc



Unclassified

>>> Jan Janak <jan at iptel.org> 05/23/05 07:25PM >>>
Hello, comments inline.

On 23-05-2005 18:57, Cesc Santasusana wrote:
> Hi,
> 
> I found what i think is a bug in the return value of loose_route() .... 
> bear with me because it is a rather long email.
> 
> First, the scenario:
> 
> phone1 ----- SER1 ----- SER2 ----- phone2
> 
> p1 to s1 and p2 to s2 use UDP
> s1 to s2 uses tcp or tls
> 
> The bug comes into place when there is a change of protocol in ser, 
> from UDP into TCP or TLS.
> We do not use a preloaded route set from the phones.
> What ser does is it adds 2 route headers, using the r2 parameter.
> Route: <route1>, <route2>
> 
> CASE A: --------------------------------------------
> In this scenario, say p1 places a call to p2, going through s1 and s2, 
> creating the route set with the record-route.
> Now, let's say p2 generates the BYE message. 
> When the BYE gets to s1, the last hop, in the config we do:
> if( loose_route () ) {
>        t_relay();
> }
> But, and here is the inconsistency, loose_route (remember we are using
> TCP/TLS, so there are 2 routes in one header field) returns FALSE.
> 
> CASE B: --------------------------------------------
> If this experiment is repeated using ALL UDP, the route headers when they 
> get to s1 contain only one route, and the call to loose_route() returns ..... TRUE!
> 
> 
> Unless the config file does not allow blind sip message relaying, both cases end 
> up working ... the BYE gets to the final destination. But if the config file does not
> accept relaying ... the request-uri in the BYE is not taken as a local domain ... and 
> thus the message is rejected ... (To Alex Mack: could this be the solution to your problem? 
> the BYEs not reaching the phone? add some log output and check it ... )
> 
> ================================================================
> So, this is not consistent. Now, what is the solution? 
> I think that both case A and B should return the same, but what, true or false?

  Yes, they should return the same value in this case and it should be
  true (see below). It looks like you discovered a bug.

> As I see it, the solution causing less trouble is that both return true. So, the last proxy
> considers this BYE as being loose_routed.
> For this ... change the code in:
> rr::loose_route.c:: static inline int after_loose(struct sip_msg* _m, int preloaded)
>            ..... 
>            if (enable_double_rr && is_2rr(&puri.params)) {
>                            ....
>                            if (res > 0) { /* No next route found */
>                                         DBG("after_loose: No next URI found\n");
>                                         return (preloaded ? NOT_RR_DRIVEN : RR_DRIVEN);
>                                       or simply 
>                                         return (preloaded RR_DRIVEN);    
> 
> 
> And now ... the other option is that both return false ... but this would make big changes.
> Correct me if i am wrong, but wouldn't this be the compliant solution? 
> Considering that the r-uri is not part of the route set, when the last proxy processes 
> all route headers and there are no more left, there is no loose routing done. The 
> destination is taken from the r-uri, not the route set ... thus loose_route should return
> false.
> Or I am getting it all wrong? rfc and goal of loose_route() ?

  The meaning of the return value of loose_route is as follows (or
  should be):

  1 - Record routing was used to route the message
  0 - Record routing was not (usually requests establishing a dialog or
      transactional requests).

  So the function should return true in the mentioned case, because
  record routing was used although the message does not contain any
  Route header fields anymore.

  In other words the purpose of loose_route return value is to distinguish
  mid-dialog requests (such as ACK or BYE) from dialog-establishing
  requests (such as INVITE). The reason why we have to do this is that we
  typically do not apply all the logic in the script to mid-dialog
  requests (because you should not change the request-uri of the request
  anymore once the dialog has been established -- changes caused by
  loose_route do not count).

  You may also wonder why don't we use the contents of the Request-URI
  for this -- there should be the Contact value of phone1 in the
  Request-URI in mid-dialog requests and the value should not match
  uri==myself so all we need to do is to enclose all the processing
  logic within if (uri == myself) condition.

  This would work in a perfect world without the need to be backwards
  compatible with strict routers. In our world we have to be backwards
  compatible with strict routers and we have to deal with broken
  implementations too (that strip ;lr parameters). Due to backwards
  compatibility with strict routers it can happen that the Request-URI
  will contain the URI of the proxy that is currently processing the
  request and subsequent if (uri==myself) would be true.

    Jan.