Re: [reSIProcate] [ReSIProcate_1.1_RC2] submit a patch forDUM/ClientPublication.cxx
OK Byron,
Thanks for your collaboration.
Will you also deliver a 1.0.4 release ?
Regards
Fabrice ROUILLIER
-----Message d'origine-----
De : Byron Campen [mailto:bcampen@xxxxxxxxxxxx]
Envoyé : jeudi 22 mars 2007 16:15
À : Byron Campen
Cc : zze-Omnipresence ROUILLIER F ext RD-MAPS-REN; resiprocate-devel
Objet : Re: [reSIProcate] [ReSIProcate_1.1_RC2] submit a patch
forDUM/ClientPublication.cxx
So, here's a revised patch. Since 412 doesn't necessarily mean that the
resource is gone, we probably should be calling onFailure.
Index: resip/dum/ClientPublication.cxx
===================================================================
--- resip/dum/ClientPublication.cxx (revision 7014)
+++ resip/dum/ClientPublication.cxx (working copy)
@@ -106,10 +106,19 @@
{
if (code == 412)
{
- InfoLog(<< "SIPIfMatch failed -- republish");
- mPublish->remove(h_SIPIfMatch);
- update(mDocument);
- return;
+ if(mPublish->header(h_Expires).value() != 0)
+ {
+ InfoLog(<< "SIPIfMatch failed -- republish");
+ mPublish->remove(h_SIPIfMatch);
+ update(mDocument);
+ return;
+ }
+ else
+ {
+ handler->onFailure(getHandle(), msg);
+ delete this;
+ return;
+ }
}
else if (code == 423) // interval too short
{
If I see no objections today, I will be committing this evening, and
RC3 will follow on Friday.
Best regards,
Byron Campen
> Okay, I'm starting to feel a little easier about this. The 412 may
> mean the resource is gone, but it may just mean that the resource has
> changed due to a PUBLISH from another UA. In either case, we probably
> don't need to continue trying to remove it. Does anyone have a problem
> with this patch (ie, can give us a reason why the patch might cause
> incorrect behavior)?
>
> Best regards,
> Byron Campen
>
>> Sorry,
>>
>> I made a mistake. I am saying that :
>>
>> the client publishes again with expires set to 0, no SIP-If-Match and
>> the last body sent.
>>
>> Regards
>>
>> Fabrice ROUILLIER
>>
>>
>> -----Message d'origine-----
>> De : Byron Campen [mailto:bcampen@xxxxxxxxxxxx] Envoyé : mardi 20
>> mars 2007 20:10 À : zze-Omnipresence ROUILLIER F ext RD-MAPS-REN Cc :
>> Michael Froman; resiprocate-devel Objet : Re: [reSIProcate]
>> [ReSIProcate_1.1_RC2] submit a patch forDUM/ClientPublication.cxx
>>
>> I'm confused. Are you or are you not saying that the PUBLISH is
>> going out with a body?
>>
>> Best regards,
>> Byron Campen
>>
>>> Michael,
>>>
>>> You are right on your analysis.
>>>
>>> But, the fact is (and I do not mentionned it before) that when the
>>> Dum::ClientPublication removes the SIP-If-Match header and updates
>>> the publication, the previously sent body is also attached (see
>>> update method). So the client publishes again with expires set to 0,
>>> no SIP-If-Match and no body. Then the ESC responds with a 412 and so
>>> one...
>>>
>>> Perhaps, is there also a bug in the end() method, the document may
>>> be deleted ?
>>>
>>> Regards
>>>
>>> Fabrice ROUILLIER
>>>
>>> -----Message d'origine-----
>>> De : resiprocate-devel-bounces@xxxxxxxxxxxxxxxxxxxx
>>> [mailto:resiprocate-devel-bounces@xxxxxxxxxxxxxxxxxxxx] De la part
>>> de Michael Froman Envoyé : lundi 19 mars 2007 20:04 À : resiprocate-
>>> devel Objet : Re: [reSIProcate] [ReSIProcate_1.1_RC2] submit a patch
>>> forDUM/ClientPublication.cxx
>>>
>>> Actually, I not sure that there is a bug here.
>>>>>> In fact publishing again with expires set to 0 and without a Sip-
>>>>>> if-match will raise a 412 again and again !!!
>>> If the client publishes again with expires set to 0 (and no body)
>>> and no SIP-If-Match, the ESC should be responding with a 400 Invalid
>>> Request as detailed in RFC3093, Section 6 (Processing PUBLISH
>>> Requests), step 5:
>>> * If the request has no message body and contained no
>>> entity- tag,
>>> the ESC SHOULD reject the request with an appropriate
>>> response,
>>> such as 400 (Invalid Request), and skip the remainder of
>>> the
>>> steps. Alternatively, in case either ESC local policy or
>>> the
>>> event package has defined semantics for an initial
>>> publication
>>> containing no message body, the ESC MAY accept it.
>>>
>>> What implementation is responding to the rePUBLISH with a 412?
>>>
>>>
>>> Regards,
>>> Michael Froman.
>>>
>>>
>>> On Mar 16, 2007, at 2:04 PM, Byron Campen wrote:
>>>
>>>> Well, we haven't exactly codified who is responsible for applying
>>>> patches. Usually it just goes to whoever knows the code fairly
>>>> well, and is around. However, IETF is happening next week, so a lot
>>>> of people are in the air right now (both figuratively and
>>>> literally).
>>>> DUM
>>>> is something that I have just started wading into, and I am uneasy
>>>> about applying patches without feedback from those who wrote most
>>>> of that code.
>>>>
>>>> Scott, have you looked at this?
>>>>
>>>> As for when the next release is, the answer is soon (I had
>>>> originally intended to designate 1.1-RC2 as the official release
>>>> this evening, but since a couple of bugs have been discovered in
>>>> the last few days, I'll have to wait for the fixes and cut RC3,
>>>> probably sometime early next week.)
>>>>
>>>> Best regards,
>>>> Byron Campen
>>>>
>>>>
>>>>> Byron
>>>>>
>>>>> I do not think that a call to handler->onFailure() is necessary,
>>>>> the aim is "in fine" to do the unPublish.
>>>>>
>>>>> Just another question, who is responsible of merging this patch
>>>>> into the reSIProcate project ?
>>>>>
>>>>> Any idea for the next release date ?
>>>>>
>>>>> Best Regards
>>>>>
>>>>> Fabrice ROUILLIER
>>>>>
>>>>> De : Byron Campen [mailto:bcampen@xxxxxxxxxxxx] Envoyé : jeudi 15
>>>>> mars 2007 22:50 À : zze-Omnipresence ROUILLIER F ext RD-MAPS- REN
>>>>> Cc
>>>>> :
>>>>> resiprocate-devel; Scott Godin Objet : Re: [reSIProcate]
>>>>> [ReSIProcate_1.1_RC2] submit a patch for DUM/ClientPublication.cxx
>>>>>
>>>>> Good find. Now, would it be necessary to call handler->onFailure
>>>>> () in
>>>>> this case? Is getting a 412 considered a "failure" for an
>>>>> unPUBLISH?
>>>>> (As far as intent goes, it seems not to me)
>>>>>
>>>>> Best regards,
>>>>> Byron Campen
>>>>>
>>>>>> Dear reSIProcate team,
>>>>>>
>>>>>> I find a bug in the implementation of the "ClientPublication"
>>>>>> class when handling response to a 412 message received from
>>>>>> server.
>>>>>>
>>>>>> You previously remove the "SIP-if-match" tag and republish the
>>>>>> document.
>>>>>>
>>>>>> This SHALL NOT be done if the 412 response is received when
>>>>>> trying to end the publication (Expires header set to 0)
>>>>>>
>>>>>> In that case nothing more have to be done !
>>>>>>
>>>>>> In fact publishing again with expires set to 0 and without a Sip-
>>>>>> if-match will raise a 412 again and again !!!
>>>>>>
>>>>>>
>>>>>> void ClientPublication::dispatch(const SipMessage& msg) {
>>>>>>
>>>>>> ...
>>>>>> if (code == 412)
>>>>>> {
>>>>>> // Receive a 412 while ending a
>>>>>> publication, nothing more to do in this case.
>>>>>> if(mPublish->header(h_Expires).value
>>>>>> () != 0
>>>>>> )
>>>>>> {
>>>>>> InfoLog(<< "SIPIfMatch failed --
>>>>>> republish");
>>>>>> mPublish->remove(h_SIPIfMatch);
>>>>>> update(mDocument);
>>>>>> return;
>>>>>> }
>>>>>> else {
>>>>>> delete this;
>>>>>> return;
>>>>>> }
>>>>>> }
>>>>>> else if (code == 423) // interval too short
>>>>>>
>>>>>> ...
>>>>>> }
>>>>>>
>>>>>> Hope this will be corrected in next candidate release
>>>>>>
>>>>>> Best Regards
>>>>>>
>>>>>> Fabrice ROUILLIER
>>>>>>
>>>>>> _______________________________________________
>>>>>> resiprocate-devel mailing list
>>>>>> resiprocate-devel@xxxxxxxxxxxxxxxxxxxx
>>>>>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>>>>
>>>>
>>>> _______________________________________________
>>>> resiprocate-devel mailing list
>>>> resiprocate-devel@xxxxxxxxxxxxxxxxxxxx
>>>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>>
>>>
>>> _______________________________________________
>>> resiprocate-devel mailing list
>>> resiprocate-devel@xxxxxxxxxxxxxxxxxxxx
>>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>>
>>> _______________________________________________
>>> resiprocate-devel mailing list
>>> resiprocate-devel@xxxxxxxxxxxxxxxxxxxx
>>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>
>
> _______________________________________________
> resiprocate-devel mailing list
> resiprocate-devel@xxxxxxxxxxxxxxxxxxxx
> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel