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

[aron_rosenberg at logitech.com]

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)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20100922/05ffd979/attachment.htm>