[reSIProcate] [ReSIProcate_1.1_RC2] submit a patch forDUM/ClientPublication.cxx
Byron Campen
bcampen at estacado.net
Thu Mar 22 10:30:27 CDT 2007
Probably not. The only stuff I've been backporting to the 1.0 branch
is show-stopper type issues (crash bugs, serious memory leaks, etc).
Best regards,
Byron Campen
> 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 at estacado.net]
> 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 at estacado.net] 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 at list.resiprocate.org
>>>> [mailto:resiprocate-devel-bounces at list.resiprocate.org] 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 at estacado.net] 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 at list.resiprocate.org
>>>>>>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> resiprocate-devel mailing list
>>>>> resiprocate-devel at list.resiprocate.org
>>>>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>>>
>>>>
>>>> _______________________________________________
>>>> resiprocate-devel mailing list
>>>> resiprocate-devel at list.resiprocate.org
>>>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>>>
>>>> _______________________________________________
>>>> resiprocate-devel mailing list
>>>> resiprocate-devel at list.resiprocate.org
>>>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>>>
>>
>> _______________________________________________
>> resiprocate-devel mailing list
>> resiprocate-devel at list.resiprocate.org
>> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 2423 bytes
Desc: not available
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20070322/6e9872f7/attachment.bin>
More information about the resiprocate-devel
mailing list