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 2d78603c070b37eb54431f61e6a4e7a0d1690a70..81dcc9298f2c6812e574ef6acbae33e2a92b1b88 100644 (file)
--- 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 b3888c4b5a96f182beb61540c8aae3aa31761de2..e28f518e521af314c1c29ad81df90dcf9b0e162f 100644 (file)
--- 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 6a58c16e5f31822261dc0712813a3c8874bcde86..0d18a841db971df67e6741bdbb047044dc135ad8 100644 (file)
--- 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 9086682350d96d449f779ba6ed13ce43a61492b4..e26cba601809b872cf205a8f912173fb3cc87f78 100644 (file)
--- 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;