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

[Jan Janak]

It should be fixed now, please update your sources and re-try.

 thanks for the report, Jan.

On 24-05-2005 10:42, Cesc Santasusana wrote:
> 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.