< Previous by Date Date Index Next by Date >
< Previous in Thread Thread Index Next in Thread >

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