[reSIProcate] ClientPublication bug with 401/auth and queued changes

[Scott Godin]

Looks good to me.  Thanks Aron!

On Wed, Sep 22, 2010 at 7:54 PM, <aron_rosenberg at logitech.com> wrote:

> I found a bug in ClientPublication which is causing it to fail a
> transaction if an update request is made while an existing request is on the
> wire that will result in a 401. A very simple scenario that can cause this
> is if the server has nonce re-use disabled (or lifetime of 0) and an updated
> PUBLISH is requested by the user while a current PUBLISH is on the wire.
>
> Scenario
>
> Client Server
> PUBLISH 1 ---------------------->
> <----------------401----PUB1-----
>
> PUBLISH 2 with auth -------->
> <----------------200 ok------------
>
> PUBLISH 3 with auth ------->
> PUBLISH 4 requested (gets queued in ClientPublication)
> <--------------401--PUB3-------
>
>
> When the 401 for PUB3 arrives, the stack doesn't do anything with it and
> dum calls ClientPublicationHandler::onFailure with a 401
>
> I believe the bug is that ClientPublication.cxx is incrementing the CSeq
> number on the shared_ptr request even in the case where it is queuing the
> new PUBLISH rather than sending it right away. This is causing the CSeq test
> in DialogSet::handledByAuthOrRedirect(const SipMessage& msg) to fail when
> the 401 arrives becuase the CSeq is now greater in the stored / shared_ptr
> request than what just came on the wire.
>
> You can force this using the new resip/dum/test/basicPublication.cxx if you
> define PUB_REALLY_FAST and have a server which doesn't allow nonce re-use.
>
> The following patch fixes the issue and I will commit it if nobody says
> otherwise. All it basically does is only increment the CSeq number when the
> request is actually sent to the wire, rather than when a request could be
> queued up.
>
>
> Index: resip/dum/ClientPublication.cxx
> ===================================================================
> --- resip/dum/ClientPublication.cxx (revision 8815)
> +++ resip/dum/ClientPublication.cxx (working copy)
> @@ -45,7 +45,6 @@
> ClientPublication::end()
> {
> InfoLog (<< "End client publication to " <<
> mPublish->header(h_RequestLine).uri());
> - mPublish->header(h_CSeq).sequence()++;
> mPublish->header(h_Expires).value() = 0;
> send(mPublish);
> }
> @@ -230,7 +229,6 @@
> {
> expiration = mPublish->header(h_Expires).value();
> }
> - mPublish->header(h_CSeq).sequence()++;
> send(mPublish);
> }
>
> @@ -283,7 +281,6 @@
> }
> }
>
> - mPublish->header(h_CSeq).sequence()++;
> mPublish->setContents(mDocument);
> send(mPublish);
> }
> @@ -328,6 +325,7 @@
> }
> else
> {
> + request->header(h_CSeq).sequence()++;
> mDum.send(request);
> mWaitingForResponse = true;
> mPendingPublish = false;
>
>
> Aron Rosenberg
> Logitech Inc, (SightSpeed Group)
>
> _______________________________________________
> resiprocate-devel mailing list
> resiprocate-devel at resiprocate.org
> https://list.resiprocate.org/mailman/listinfo/resiprocate-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20100923/189f50dc/attachment.htm>