[reSIProcate] [ReSIProcate_1.1_RC2] submit a patch forDUM/ClientPublication.cxx
Byron Campen
bcampen at estacado.net
Thu Mar 22 10:14:57 CDT 2007
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/7d285147/attachment.bin>
More information about the resiprocate-devel
mailing list