| < Previous by Date | Date Index | Next by Date > |
| < Previous in Thread | Thread Index |
|
Looks right to me. Probably also want add an else clause
to “if(mCreator)” and also return false and log error “Can’t
create a dialog, on a UAS response”. Scott From: Justin Matthews
[mailto:jmatthewsr@xxxxxxxxx] if (msg.isResponse()) {
int code = msg.header(h_StatusLine).statusCode(); if( mCreator ) { SharedPtr<SipMessage>
lastRequest(mCreator->getLastRequest()); if( 0 != lastRequest &&
(lastRequest->header(h_CSeq).sequence() !=
msg.header(h_CSeq).sequence() ||
lastRequest->header(h_CSeq).method() != msg.header(h_CSeq).method())) { InfoLog(<<
"Cannot create a dialog, mid-dialog fork not allowed."); return; } } ??? From: Scott Godin
[mailto:slgodin@xxxxxxxxxxxx] Huh? BaseCreator getLastRequest contains a copy of the initial INVITE
message – which has a CSeq number. The response we are processing
also has a CSeq number. If these don’t match, then we don’t
want to create a dialog. From: Justin Matthews
[mailto:jmatthewsr@xxxxxxxxx] I didn’t see the BaseCreator last request being updated,
so I’m not sure where to find the cseq for this re-invite. In looking at sending the re-invite the only info I could see
being updated is: mRequests[msg->header(h_CSeq).sequence()] = msg; in
Dialog::send. The problem with this is that the UAS has modified the tag so dum
can’t find any existing dialog, so the cseq of the initial re-invite
request will need to be stored somewhere at the dialogset level?
What about adding the cseq info to mLastRequest in the BaseCreator when
sending the initial re-INVITE? Thanks, -justin From: Scott Godin
[mailto:slgodin@xxxxxxxxxxxx] I don’t think this is quite right – it will not
allow multiple 200’s to an initially forked invite request to create
dialogs. I think we should check the CSeq in response and only create a
dialog if it matches the CSeq in the base creator. This will make sure we
only create multiple dialogs for an initial UAC request, but not mid dialog
requests. Scott From: Justin Matthews
[mailto:jmatthewsr@xxxxxxxxx] Further on this issue, although the patch fixes the exception and
drops the 200 for the case where dum was initially the UAS, this scenario will
still cause issues if dum was initially the UAC (since getLastRequest will
contain a valid contact). Propose the following, please note that I am not sure that
“mState == Established” is the correct check here, please advise. In DialogSet.cxx: if (dialog == 0) { if (msg.isRequest() &&
msg.header(h_RequestLine).method() == CANCEL) {
dispatchToAllDialogs(msg); return; } if (msg.isResponse())
{
int code = msg.header(h_StatusLine).statusCode();
if( code > 100 && mState == Established )
{
InfoLog(<< "Cannot
create a dialog, mid-dialog fork not allowed.");
return;
} Thanks, -justin From: Scott Godin
[mailto:slgodin@xxxxxxxxxxxx] That is pretty odd behaviour by the far end. Mid-dialog
forking is not allowed (3261 sec 14.1). If we detect a 200 with a
different to tag and the dialogset is a UAS dialogset (ie. no base creator) -
then we should either: 1.
Ack the 200 and then send a bye. or 2.
Just ignore the 200 - don't proceed to create a dialog for it - but log an
error. I'm
leaning towards 2. Scott From:
resiprocate-devel-bounces@xxxxxxxxxxxxxxxxxxxx on behalf of Justin Matthews Exception
occurs during following scenario: DUM
receives a call and establishes a session as the UAS. DUM
sends re-INVITE request DUM
receives 200 ok response with a different to: tag then the established session. The
dialogset is found, but dialogset.cxx tries to create a new dialog from this
200 and calls the following code: Dialog.cxx,
in Dialog constructor: mLocalContact
= creator->getLastRequest()->header(h_Contacts).front(); I
believe that getLastRequest does not contain a contact because this call was
established from an inbound session. For
now I just patched it this way, is there a better way? If not I will
commit this. ------------------- BaseCreator*
creator = mDialogSet.getCreator(); //assert(creator);// !jf! throw or something here //assert(creator->getLastRequest()->exists(h_Contacts)); //assert(!creator->getLastRequest()->header(h_Contacts).empty()); if(
NULL == creator ) { ErrLog(<< "BaseCreator
is null for DialogSet");
ErrLog(<< response);
throw Exception("BaseCreator
is null for DialogSet", __FILE__, __LINE__); }
SharedPtr<SipMessage>
lastRequest(creator->getLastRequest()); if(
NULL == lastRequest ||
!lastRequest->exists(h_Contacts) ||
lastRequest->header(h_Contacts).empty()) {
ErrLog(<< "No contact available for
dialogset's last request");
ErrLog(<< response);
throw Exception("No
contact available for dialogset's last request", __FILE__,
__LINE__); } mLocalContact
= creator->getLastRequest()->header(h_Contacts).front(); mRemoteTarget
= contact; ---------------------- Thanks, -justin |