From edda124411f237a843788fd4c79176600b4c29b8 Mon Sep 17 00:00:00 2001 From: Amos Jeffries Date: Fri, 18 Dec 2009 17:15:55 +1300 Subject: [PATCH] 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. --- src/auth/User.cc | 2 +- src/auth/UserRequest.cc | 19 ++++++++++++------ src/auth/basic/auth_basic.cc | 2 +- src/auth/basic/auth_basic.h | 8 -------- src/auth/digest/auth_digest.cc | 29 ++++++---------------------- src/auth/digest/auth_digest.h | 10 ++-------- src/auth/negotiate/auth_negotiate.cc | 3 +-- src/auth/negotiate/auth_negotiate.h | 9 --------- src/auth/ntlm/auth_ntlm.cc | 3 +-- src/auth/ntlm/auth_ntlm.h | 9 --------- 10 files changed, 25 insertions(+), 69 deletions(-) diff --git a/src/auth/User.cc b/src/auth/User.cc index 5c242c72e4..90f8d70de5 100644 --- 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 417f381263..4a6303f422 100644 --- 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(link->data)->getRaw() != this ); - dlinkNodeDelete(link); + if (static_cast(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 3e30f6345d..b45aa5de27 100644 --- 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 4f4df1f9bd..7884c3da64 100644 --- 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(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 daf9a008fa..4546641ebf 100644 --- 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(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(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 3bf9db8707..5e94f480ee 100644 --- 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(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 14594acae6..4e9c3ccf1e 100644 --- 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 7b3707c3b1..fd92061be4 100644 --- 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(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 3eaade7d6a..766f4ea57a 100644 --- 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 949c882a26..8ac709b799 100644 --- 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(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); -- 2.47.3