Re: [reSIProcate] Rejecting media negotiation in case of SDP-less reInvite
On Fri, Sep 16, 2011 at 7:59 AM, Francis Joanis
<francis.joanis@xxxxxxxxx> wrote:
> On Fri, Sep 16, 2011 at 7:20 AM, Robert Szokovacs
> <rszokovacs@xxxxxxxxxxxxxxx> wrote:
>> On 2011 September 16, Friday 11:56:59 Robert Szokovacs wrote:
>>> On 2011 September 15, Thursday 19:50:53 you wrote:
>>>
>>> Hi,
>>>
>>> Thanks for the answer.
>>>
>>> > I just tested what I think should produce the sequence you mention
>>> > (albeit I had reSIProcate on both sides) with the current mainline of
>>> > the code.
>>> >
>>> > When I call reject(code), the INVITE transaction is completed (an ACK
>>> > with no SDP is sent) then a BYE with the reason "Illegal Sdp
>>> > Negotiation" gets sent to B.
>>> >
>>> > This behavior is found in ClientInviteSession:reject(...):
>>> >
>>> > case UAC_Answered:{
>>> >
>>> > // We received an offer in a 2xx response, and we want to reject
>>> >
>>> > it // ACK with no body, then send bye
>>> >
>>> > sendAck();
>>> > SharedPtr<SipMessage> msg = sendBye();
>>> > transition(Terminated);
>>> > mDum.mInviteSessionHandler->onTerminated(getSessionHandle(),
>>> >
>>> > InviteSessionHandler::LocalBye, msg.get());
>>> >
>>> > break;
>>> >
>>> > }
>>> >
>>> > Could you retest with the latest release (1.7) and/or the mainline? It
>>> > should be working.
>>>
>>> It will take some time unfortunately, but yes, I will check it.
>>>
>>> However, as I look at the code in 1.7, I can't find similar mechanism in
>>> ServerInviteSession::reject and if you reverse the roles (as in my case it
>>> was), I expect you will see similar assert too.
>>
>> Also, I forgot to mention that the actual scenario I encountered was
>> reInvite,
>> so the state of the leg was SentReinviteAnswered, not UAC_Answered
>>
>> br
>>
>> Szo
>>
>
> Hi again,
>
> Thanks for the additional information, I am now able to reproduce the issue:
>
> - Both UAs connected
> - Server calls requestOffer()
> - Client calls provideOffer(sdp)
> - Server calls reject(code)
> - assert(0)
>
> On the surface it looks like code is missing from
> InviteSession::reject, but I am not sure that simply adding a "case
> SentReinviteAnswered" is enough. I'll test it more and let you know.
>
> Thanks,
> Francis
>
Hi, the following patch seems to work:
svn diff resip/dum/InviteSession.cxx
Index: resip/dum/InviteSession.cxx
===================================================================
--- resip/dum/InviteSession.cxx (revision 9266)
+++ resip/dum/InviteSession.cxx (working copy)
@@ -757,7 +757,15 @@
send(response);
break;
}
-
+ // Sent a reINVITE no offer and received a 200-offer.
+ // Simply send an ACK without an answer and stay in Connected.
+ case SentReinviteAnswered:
+ {
+ InfoLog (<< "Not sending " << statusCode << " error since
transaction already completed, sending answer-less ACK");
+ transition(Connected);
+ sendAck();
+ break;
+ }
default:
assert(0);
break;
Since the 200 OK has already been received we can't send an error code
back because the transaction is already over. What we can do instead
is send an empty ACK (as you pointed out in your scenario). If
reSIProcate (DUM) is on the receiving end of that ACK, the
onOfferRejected callback will be triggered and the original call will
be left intact (as per RFC 3261 14.1).
The only thing I don't like about the patch is that we need to supply
an error code to reject(code) and it never gets used in that case (so
the value of the error code doesn't matter).
Scott/others, could you comment on the patch? I'd like to have it
reviewed before submitting it.
Robert, could you test it and let me know if it works?
Thanks,
Francis