Hi Benny, On 11 Sep 2008, at 10:10, Benny Prijono wrote: > On Wed, Sep 10, 2008 at 11:56 AM, Ruud Klaver <ruud at ag-projects.com> > wrote: > Hi, > > After some digging I seem to have uncovered a bug, which I think I > have managed to fix. > > First off, let me describe the scenario that led me to this. I have > three endpoints, two of which are my application that is using > PJSIP, let's call them endpoint A and B. Endpoint C is a third party > SIP UA that sets 100rel in the Required header. I am using endpoint > A to send an INVITE to a SIP address to which both endpoint B and C > have REGISTERed. This leads to the following sequence of events: > > - Endpoint B sends a 180 Trying in response to the INVITE > - Endpoint A records the Contact header of endpoint B as target for > the dialog > - Endpoint C is a little slower and also sends a 180 Trying, > including Required: 100rel > - Endpoint A sends a PRACK in response to the 180 from endpoint C, > but puts as the request URI the target recorded for the dialog, > which is the URi in the Contact header of endpoint B. > - The PRACK ends up at endpoint B instead of C, which promptly > crashes with an assertion failure, as the PJSIP_INV_REQUIRE_100REL > option was not set on pjsip_inv_verify_request(). > > I have also tried this with the pjsua application, but this does not > crash as it only sends a 100 and not a 180, which is never > propagated back upstream by the SIP proxy (the 100 that is). > > I have included a patch that at least sets the request URI in the > PRACK to that of the provisional response that triggered it, > overriding the target URI of the "dialog" (officially I don't think > there is a dialog yet because it has not been established). > > Perhaps the code that handles the receiving of the PRACK should be > made more resilient as well, as endpoint B does actually advertise > that it supports PRACK, even though it did not put it in the > Required header. > > > Hi Ruud, > > thanks for the report. You're right, it's a bug, and I've just fixed > this in http://trac.pjsip.org/repos/ticket/620 > > My solution is different though, I fixed this at the dialog level > rather than in the 100rel module. The reason is because if the > dialog record-routes, then setting the target alone in the PRACK > request URI is not sufficient, since the route set still uses the > route set of the other UAS. This might not be a big problem if > there's only one proxy involved, but nevertheless we should take > this into account. > Yeah I was afraid it may have been a bit of a hack. I see what you did there, but is there no danger in the target and route in the dialog struct being continually overwritten by incoming provisional responses? If not than this is definitely the way to go. ;) > I've also replace the assertion when unexpected PRACK is received > with responding it with 400. Btw you should always disable assertion > (with -DNDEBUG) in the release product, and in this case the PRACK > will just be ignored rather than terminating the app. So it's not a > crash. ;-) > > Thanks for the report. > > Cheers > Benny That is true, but I am still very much debugging... Also, from RFC 3262: If a PRACK request is received by the UA core that does not match any unacknowledged reliable provisional response, the UAS MUST respond to the PRACK with a 481 response. Not sure if it applies to this situation, but 481 may be a more appropriate response. Oh and thanks for the fix. Ruud Klaver AG Projects
