Re: [reSIProcate] [ReSIProcate_1.1_RC2] submit a patch forDUM/ClientPublication.cxx

[Byron Campen <bcampen@xxxxxxxxxxxx>]

	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@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

Attachment: smime.p7s
Description: S/MIME cryptographic signature