Remove redundant user() functions

The child classes of AuthUserRequest turned out to have alternative
verions of user() set and get functions. These did the same operations
but stored the AuthUser class in different locations.
This makes user() always NULL for pointers to the parent object inside
the core authentication code. Regardess of whether an AuthUser was set.

Also digest auth had a completely alternative set of functions authUser()
to do user() operations on the parent object and locking. This has been
cleaned up.

diff --git a/src/auth/User.cc b/src/auth/User.cc
index 5c242c72e44e4a51ce3db8940822fb0133c38c2e..90f8d70de518e2bf50c8ca3da2b1a89a1d9e4206 100644 (file)
--- a/src/auth/User.cc
+++ b/src/auth/User.cc
@@ -120,7 +120,7 @@ AuthUser::~AuthUser()
         link = link->next;
         dlinkDelete(tmplink, &requests);
         dlinkNodeDelete(tmplink);
-        delete auth_user_request;
+        *auth_user_request = NULL;
     }
 
     /* free cached acl results */

diff --git a/src/auth/UserRequest.cc b/src/auth/UserRequest.cc
index 417f3812631b493cf82b92ac814aeb0486d4bd6b..4a6303f422ac067cfb05a0647628ad54fd2c9483 100644 (file)
--- a/src/auth/UserRequest.cc
+++ b/src/auth/UserRequest.cc
@@ -139,17 +139,24 @@ AuthUserRequest::~AuthUserRequest()
     debugs(29, 5, "AuthUserRequest::~AuthUserRequest: freeing request " << this);
 
     if (user()) {
+        /* AYJ: something strange: in order to be deleted this object must not be
+         * referenced anywhere. Including the AuthUser list of requests.
+         * I expect the following loop to NEVER find a pointer to this request object.
+         */
+
         /* unlink from the auth_user struct */
         link = user()->requests.head;
 
-        while (link && (link->data != this))
-            link = link->next;
-
-        assert(link != NULL);
+        while (link) {
 
-        dlinkDelete(link, &user()->requests);
+            assert( static_cast<AuthUserRequest::Pointer*>(link->data)->getRaw() != this );
 
-        dlinkNodeDelete(link);
+            if (static_cast<AuthUserRequest::Pointer*>(link->data)->getRaw() == this) {
+                dlinkDelete(link, &user()->requests);
+                dlinkNodeDelete(link);
+            }
+            link = link->next;
+        }
 
         /* unlock the request structure's lock */
         user()->unlock();

diff --git a/src/auth/basic/auth_basic.cc b/src/auth/basic/auth_basic.cc
index 3e30f6345d927f3f478c7cd9985e43b5958ce8d9..b45aa5de2771681ce18bb93f830f9164341861b9 100644 (file)
--- a/src/auth/basic/auth_basic.cc
+++ b/src/auth/basic/auth_basic.cc
@@ -122,7 +122,7 @@ AuthBasicConfig::type() const
     return basicScheme::GetInstance().type();
 }
 
-AuthBasicUserRequest::AuthBasicUserRequest() : _theUser(NULL)
+AuthBasicUserRequest::AuthBasicUserRequest()
 {}
 
 AuthBasicUserRequest::~AuthBasicUserRequest()

diff --git a/src/auth/basic/auth_basic.h b/src/auth/basic/auth_basic.h
index 4f4df1f9bd6e1c08cf2cacae647183ae4611a804..7884c3da64c6418eb32ada7dec4aa59c5e367b4a 100644 (file)
--- a/src/auth/basic/auth_basic.h
+++ b/src/auth/basic/auth_basic.h
@@ -93,14 +93,6 @@ public:
     virtual void authenticate(HttpRequest * request, ConnStateData *conn, http_hdr_type type);
     virtual int module_direction();
     virtual void module_start(RH *, void *);
-    virtual AuthUser *user() {return _theUser;}
-
-    virtual const AuthUser *user() const {return _theUser;}
-
-    virtual void user (AuthUser *aUser) {_theUser=dynamic_cast<BasicUser *>(aUser);}
-
-private:
-    BasicUser *_theUser;
 };
 
 MEMPROXY_CLASS_INLINE(AuthBasicUserRequest);

diff --git a/src/auth/digest/auth_digest.cc b/src/auth/digest/auth_digest.cc
index daf9a008faf086b9d1ae9329cb6c02020fe5b993..4546641ebf87ba5f9d518b28bbec17e523c62bba 100644 (file)
--- a/src/auth/digest/auth_digest.cc
+++ b/src/auth/digest/auth_digest.cc
@@ -573,7 +573,6 @@ AuthDigestUserRequest::authenticated() const
 void
 AuthDigestUserRequest::authenticate(HttpRequest * request, ConnStateData * conn, http_hdr_type type)
 {
-    AuthUser *auth_user;
     AuthDigestUserRequest *digest_request;
     digest_user_h *digest_user;
 
@@ -581,11 +580,10 @@ AuthDigestUserRequest::authenticate(HttpRequest * request, ConnStateData * conn,
     HASHHEX HA2 = "";
     HASHHEX Response;
 
-    assert(authUser() != NULL);
-    auth_user = authUser();
-
-    digest_user = dynamic_cast < digest_user_h * >(auth_user);
+    assert(user() != NULL);
+    AuthUser *auth_user = user();
 
+    digest_user = dynamic_cast<digest_user_h*>(auth_user);
     assert(digest_user != NULL);
 
     /* if the check has corrupted the user, just return */
@@ -1333,11 +1331,10 @@ AuthDigestConfig::decode(char const *proxy_auth)
     /*link the request and the user */
     assert(digest_request != NULL);
 
-    digest_request->authUser (digest_user);
-
+    digest_user->lock();
     digest_request->user(digest_user);
 
-    digest_user->addRequest (digest_request);
+    digest_user->addRequest(digest_request);
 
     debugs(29, 9, "username = '" << digest_user->username() << "'\nrealm = '" <<
            digest_request->realm << "'\nqop = '" << digest_request->qop <<
@@ -1385,20 +1382,6 @@ AuthDigestUserRequest::module_start(RH * handler, void *data)
 DigestUser::DigestUser (AuthConfig *aConfig) : AuthUser (aConfig), HA1created (0)
 {}
 
-AuthUser *
-AuthDigestUserRequest::authUser() const
-{
-    return const_cast<AuthUser *>(user());
-}
-
-void
-AuthDigestUserRequest::authUser(AuthUser *aUser)
-{
-    assert(!authUser());
-    user(aUser);
-    user()->lock();
-}
-
 AuthDigestUserRequest::CredentialsState
 AuthDigestUserRequest::credentials() const
 {
@@ -1414,7 +1397,7 @@ AuthDigestUserRequest::credentials(CredentialsState newCreds)
 AuthDigestUserRequest::AuthDigestUserRequest() : nonceb64(NULL) ,cnonce(NULL) ,realm(NULL),
         pszPass(NULL) ,algorithm(NULL) ,pszMethod(NULL),
         qop(NULL) ,uri(NULL) ,response(NULL),
-        nonce(NULL), _theUser (NULL) ,
+        nonce(NULL),
         credentials_ok (Unchecked)
 {}
 

diff --git a/src/auth/digest/auth_digest.h b/src/auth/digest/auth_digest.h
index 3bf9db8707a51a427d4b582d00998d3a9ef4c21d..5e94f480ee3f64e50e2843821da3b94bd69658f5 100644 (file)
--- a/src/auth/digest/auth_digest.h
+++ b/src/auth/digest/auth_digest.h
@@ -70,17 +70,12 @@ public:
 #endif
 
     virtual void module_start(RH *, void *);
-    virtual AuthUser *user() {return _theUser;}
-
-    virtual const AuthUser *user() const {return _theUser;}
-
-    virtual void user(AuthUser *aUser) {_theUser=dynamic_cast<DigestUser *>(aUser);}
 
     CredentialsState credentials() const;
     void credentials(CredentialsState);
 
-    void authUser(AuthUser *);
-    AuthUser *authUser() const;
+//    void authUser(AuthUser *);
+//    AuthUser *authUser() const;
 
     char *nonceb64;            /* "dcd98b7102dd2f0e8b11d0f600bfb0c093" */
     char *cnonce;              /* "0a4f113b" */
@@ -101,7 +96,6 @@ public:
     digest_nonce_h *nonce;
 
 private:
-    DigestUser *_theUser;
     CredentialsState credentials_ok;
 };
 

diff --git a/src/auth/negotiate/auth_negotiate.cc b/src/auth/negotiate/auth_negotiate.cc
index 14594acae6d15cb4b75eb240b5b082de5a43e8d1..4e9c3ccf1e9fe0b048ab85aac83407cf308d7ff9 100644 (file)
--- a/src/auth/negotiate/auth_negotiate.cc
+++ b/src/auth/negotiate/auth_negotiate.cc
@@ -764,8 +764,7 @@ AuthNegotiateUserRequest::authenticate(HttpRequest * aRequest, ConnStateData * c
 }
 
 AuthNegotiateUserRequest::AuthNegotiateUserRequest() :
-        /*conn(NULL),*/ auth_state(AUTHENTICATE_STATE_NONE),
-        _theUser(NULL)
+        /*conn(NULL),*/ auth_state(AUTHENTICATE_STATE_NONE)
 {
     waiting=0;
     client_blob=0;

diff --git a/src/auth/negotiate/auth_negotiate.h b/src/auth/negotiate/auth_negotiate.h
index 7b3707c3b134dbec8088987b020fb3018e6e293b..fd92061be4afd88c2e3b9a432f27fbaebc195486 100644 (file)
--- a/src/auth/negotiate/auth_negotiate.h
+++ b/src/auth/negotiate/auth_negotiate.h
@@ -72,14 +72,9 @@ public:
     virtual int module_direction();
     virtual void onConnectionClose(ConnStateData *);
     virtual void module_start(RH *, void *);
-    virtual AuthUser *user() {return _theUser;}
-
-    virtual const AuthUser *user() const {return _theUser;}
 
     virtual void addHeader(HttpReply * rep, int accel);
 
-    virtual void user (AuthUser *aUser) {_theUser=dynamic_cast<NegotiateUser *>(aUser);}
-
     virtual const char * connLastHeader();
 
     /*we need to store the helper server between requests */
@@ -102,10 +97,6 @@ public:
 
     /* need access to the request flags to mess around on pconn failure */
     HttpRequest *request;
-
-private:
-    /* the user */
-    NegotiateUser * _theUser;
 };
 
 MEMPROXY_CLASS_INLINE(AuthNegotiateUserRequest);

diff --git a/src/auth/ntlm/auth_ntlm.cc b/src/auth/ntlm/auth_ntlm.cc
index 3eaade7d6aa192db6f0f9720ed37115e74a28a43..766f4ea57a1d9a6557a40c5034b6b7e9db648a38 100644 (file)
--- a/src/auth/ntlm/auth_ntlm.cc
+++ b/src/auth/ntlm/auth_ntlm.cc
@@ -682,8 +682,7 @@ AuthNTLMUserRequest::authenticate(HttpRequest * aRequest, ConnStateData * conn,
 }
 
 AuthNTLMUserRequest::AuthNTLMUserRequest() :
-        /*conn(NULL),*/ auth_state(AUTHENTICATE_STATE_NONE),
-        _theUser(NULL)
+        /*conn(NULL),*/ auth_state(AUTHENTICATE_STATE_NONE)
 {
     waiting=0;
     client_blob=0;

diff --git a/src/auth/ntlm/auth_ntlm.h b/src/auth/ntlm/auth_ntlm.h
index 949c882a269c6782a87f438c06f22a82f1a8640e..8ac709b799ae13a90f3f7f55383131e25c662852 100644 (file)
--- a/src/auth/ntlm/auth_ntlm.h
+++ b/src/auth/ntlm/auth_ntlm.h
@@ -60,11 +60,6 @@ public:
     virtual int module_direction();
     virtual void onConnectionClose(ConnStateData *);
     virtual void module_start(RH *, void *);
-    virtual AuthUser *user() {return _theUser;}
-
-    virtual const AuthUser *user() const {return _theUser;}
-
-    virtual void user (AuthUser *aUser) {_theUser=dynamic_cast<NTLMUser *>(aUser);}
 
     virtual const char * connLastHeader();
 
@@ -88,10 +83,6 @@ public:
 
     /* need access to the request flags to mess around on pconn failure */
     HttpRequest *request;
-
-private:
-    /* the user */
-    NTLMUser * _theUser;
 };
 
 MEMPROXY_CLASS_INLINE(AuthNTLMUserRequest);