From: Amos Jeffries Date: Wed, 29 Jul 2009 09:07:56 +0000 (+1200) Subject: Cleanup: shuffle authenticate*ReleaseServer into member methods. X-Git-Tag: SQUID_3_2_0_1~829 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d945258c8d740b0caf489af793a7027dba75394c;p=thirdparty%2Fsquid.git Cleanup: shuffle authenticate*ReleaseServer into member methods. No code change. Just removes some more global functions and castings. Simplifies the duplicate case checking a bit too. --- diff --git a/src/auth/negotiate/auth_negotiate.cc b/src/auth/negotiate/auth_negotiate.cc index 2d78603c07..81dcc9298f 100644 --- a/src/auth/negotiate/auth_negotiate.cc +++ b/src/auth/negotiate/auth_negotiate.cc @@ -61,9 +61,6 @@ // AYJ: must match re-definition in helpers/negotiate_auth/squid_kerb_auth/squid_kerb_auth.c #define MAX_AUTHTOKEN_LEN 32768 -static void -authenticateNegotiateReleaseServer(AuthUserRequest * auth_user_request); - /// \ingroup AuthNegotiateInternal static void @@ -474,7 +471,7 @@ authenticateNegotiateHandleReply(void *data, void *lastserver, char *reply) negotiate_request->server_blob = xstrdup(blob); - authenticateNegotiateReleaseServer(negotiate_request); + negotiate_request->releaseAuthServer(); negotiate_request->auth_state = AUTHENTICATE_STATE_DONE; @@ -506,7 +503,7 @@ authenticateNegotiateHandleReply(void *data, void *lastserver, char *reply) /* set these to now because this is either a new login from an * existing user or a new user */ local_auth_user->expiretime = current_time.tv_sec; - authenticateNegotiateReleaseServer(negotiate_request); + negotiate_request->releaseAuthServer(); negotiate_request->auth_state = AUTHENTICATE_STATE_DONE; } else if (strncasecmp(reply, "NA ", 3) == 0 && arg != NULL) { @@ -523,7 +520,7 @@ authenticateNegotiateHandleReply(void *data, void *lastserver, char *reply) negotiate_request->server_blob = xstrdup(blob); - authenticateNegotiateReleaseServer(negotiate_request); + negotiate_request->releaseAuthServer(); result = S_HELPER_RELEASE; @@ -537,7 +534,7 @@ authenticateNegotiateHandleReply(void *data, void *lastserver, char *reply) auth_user_request->denyMessage(blob); negotiate_request->auth_state = AUTHENTICATE_STATE_FAILED; safe_free(negotiate_request->server_blob); - authenticateNegotiateReleaseServer(negotiate_request); + negotiate_request->releaseAuthServer(); result = S_HELPER_RELEASE; debugs(29, 1, "authenticateNegotiateHandleReply: Error validating user via Negotiate. Error returned '" << reply << "'"); } else { @@ -605,25 +602,20 @@ AuthNegotiateUserRequest::module_start(RH * handler, void *data) helperStatefulSubmit(negotiateauthenticators, buf, authenticateNegotiateHandleReply, r, authserver); } -/* clear the Negotiate helper of being reserved for future requests */ -static void -authenticateNegotiateReleaseServer(AuthUserRequest * auth_user_request) +/** + * Atomic action: properly release the Negotiate auth helpers which may have been reserved + * for this request connections use. + */ +void +AuthNegotiateUserRequest::releaseAuthServer() { - AuthNegotiateUserRequest *negotiate_request; - assert(auth_user_request->user()->auth_type == AUTH_NEGOTIATE); - negotiate_request = dynamic_cast< AuthNegotiateUserRequest *>(auth_user_request); - assert(negotiate_request != NULL); - debugs(29, 9, "authenticateNegotiateReleaseServer: releasing server '" << negotiate_request->authserver << "'"); - /* is it possible for the server to be NULL? hno seems to think so. - * Let's see what happens, might segfault in helperStatefulReleaseServer - * if it does. I leave it like this not to cover possibly problematic - * code-paths. Kinkie */ - /* DPW 2007-05-07 - * yes, it is possible */ - if (negotiate_request->authserver) { - helperStatefulReleaseServer(negotiate_request->authserver); - negotiate_request->authserver = NULL; + if (authserver) { + debugs(29, 6, HERE << "releasing Negotiate auth server '" << authserver << "'"); + helperStatefulReleaseServer(authserver); + authserver = NULL; } + else + debugs(29, 6, HERE << "No Negotiate auth server to release."); } /* clear any connection related authentication details */ @@ -639,8 +631,7 @@ AuthNegotiateUserRequest::onConnectionClose(ConnStateData *conn) return; } - if (authserver != NULL) - authenticateNegotiateReleaseServer(this); + releaseAuthServer(); /* unlock the connection based lock */ debugs(29, 9, "AuthNegotiateUserRequest::onConnectionClose: Unlocking auth_user from the connection '" << conn << "'."); diff --git a/src/auth/negotiate/auth_negotiate.h b/src/auth/negotiate/auth_negotiate.h index b3888c4b5a..e28f518e52 100644 --- a/src/auth/negotiate/auth_negotiate.h +++ b/src/auth/negotiate/auth_negotiate.h @@ -84,6 +84,8 @@ public: /*we need to store the helper server between requests */ helper_stateful_server *authserver; + void releaseAuthServer(void); ///< Release the authserver helper server properly. + /* what connection is this associated with */ /* ConnStateData * conn;*/ diff --git a/src/auth/ntlm/auth_ntlm.cc b/src/auth/ntlm/auth_ntlm.cc index 6a58c16e5f..0d18a841db 100644 --- a/src/auth/ntlm/auth_ntlm.cc +++ b/src/auth/ntlm/auth_ntlm.cc @@ -50,9 +50,6 @@ #include "wordlist.h" #include "SquidTime.h" -static void -authenticateNTLMReleaseServer(AuthUserRequest * auth_user_request); - static void authenticateStateFree(authenticateStateData * r) @@ -432,14 +429,14 @@ authenticateNTLMHandleReply(void *data, void *lastserver, char *reply) /* set these to now because this is either a new login from an * existing user or a new user */ local_auth_user->expiretime = current_time.tv_sec; - authenticateNTLMReleaseServer(ntlm_request); + ntlm_request->releaseAuthServer(); ntlm_request->auth_state = AUTHENTICATE_STATE_DONE; } else if (strncasecmp(reply, "NA ", 3) == 0) { /* authentication failure (wrong password, etc.) */ auth_user_request->denyMessage(blob); ntlm_request->auth_state = AUTHENTICATE_STATE_FAILED; safe_free(ntlm_request->server_blob); - authenticateNTLMReleaseServer(ntlm_request); + ntlm_request->releaseAuthServer(); result = S_HELPER_RELEASE; debugs(29, 4, "authenticateNTLMHandleReply: Failed validating user via NTLM. Error returned '" << blob << "'"); } else if (strncasecmp(reply, "BH ", 3) == 0) { @@ -451,7 +448,7 @@ authenticateNTLMHandleReply(void *data, void *lastserver, char *reply) auth_user_request->denyMessage(blob); ntlm_request->auth_state = AUTHENTICATE_STATE_FAILED; safe_free(ntlm_request->server_blob); - authenticateNTLMReleaseServer(ntlm_request); + ntlm_request->releaseAuthServer(); result = S_HELPER_RELEASE; debugs(29, 1, "authenticateNTLMHandleReply: Error validating user via NTLM. Error returned '" << reply << "'"); } else { @@ -519,25 +516,20 @@ AuthNTLMUserRequest::module_start(RH * handler, void *data) helperStatefulSubmit(ntlmauthenticators, buf, authenticateNTLMHandleReply, r, authserver); } -/* clear the NTLM helper of being reserved for future requests */ -static void -authenticateNTLMReleaseServer(AuthUserRequest * auth_user_request) +/** + * Atomic action: properly release the NTLM auth helpers which may have been reserved + * for this request connections use. + */ +void +AuthNTLMUserRequest::releaseAuthServer() { - AuthNTLMUserRequest *ntlm_request; - assert(auth_user_request->user()->auth_type == AUTH_NTLM); - ntlm_request = dynamic_cast< AuthNTLMUserRequest *>(auth_user_request); - debugs(29, 9, "authenticateNTLMReleaseServer: releasing server '" << ntlm_request->authserver << "'"); - /* is it possible for the server to be NULL? hno seems to think so. - * Let's see what happens, might segfault in helperStatefulReleaseServer - * if it does. I leave it like this not to cover possibly problematic - * code-paths. Kinkie */ - /* DPW 2007-05-07 - * yes, it is possible */ - assert(ntlm_request != NULL); - if (ntlm_request->authserver) { - helperStatefulReleaseServer(ntlm_request->authserver); - ntlm_request->authserver = NULL; + if (authserver) { + debugs(29, 6, HERE << "releasing NTLM auth server '" << authserver << "'"); + helperStatefulReleaseServer(authserver); + authserver = NULL; } + else + debugs(29, 6, HERE << "No NTLM auth server to release."); } /* clear any connection related authentication details */ @@ -553,8 +545,8 @@ AuthNTLMUserRequest::onConnectionClose(ConnStateData *conn) return; } - if (authserver != NULL) - authenticateNTLMReleaseServer(this); + // unlock / un-reserve the helpers + releaseAuthServer(); /* unlock the connection based lock */ debugs(29, 9, "AuthNTLMUserRequest::onConnectionClose: Unlocking auth_user from the connection '" << conn << "'."); @@ -720,11 +712,8 @@ AuthNTLMUserRequest::~AuthNTLMUserRequest() safe_free(server_blob); safe_free(client_blob); - if (authserver != NULL) { - debugs(29, 9, "AuthNTLMUserRequest::~AuthNTLMUserRequest: releasing server '" << authserver << "'"); - helperStatefulReleaseServer(authserver); - authserver = NULL; - } + releaseAuthServer(); + if (request) { HTTPMSGUNLOCK(request); request = NULL; @@ -753,4 +742,3 @@ AuthNTLMUserRequest::connLastHeader() { return NULL; } - diff --git a/src/auth/ntlm/auth_ntlm.h b/src/auth/ntlm/auth_ntlm.h index 9086682350..e26cba6018 100644 --- a/src/auth/ntlm/auth_ntlm.h +++ b/src/auth/ntlm/auth_ntlm.h @@ -68,8 +68,10 @@ public: virtual const char * connLastHeader(); - /*we need to store the helper server between requests */ + /* we need to store the helper server between requests */ helper_stateful_server *authserver; + void releaseAuthServer(void); ///< Release authserver NTLM helpers properly when finished or abandoning. + /* what connection is this associated with */ // ConnStateData * conn;