From: Amos Jeffries Date: Fri, 14 Aug 2009 05:15:16 +0000 (+1200) Subject: Author: Henrik Nordstrom X-Git-Tag: SQUID_3_0_STABLE19~13 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=029ee83d4a97edeb1214cd4aa7fc287c3e2d4339;p=thirdparty%2Fsquid.git Author: Henrik Nordstrom Bug 2648: stateful helpers stuck in reserved if client disconnects while helper busy Note: May depend on the other stateful helper cleanups. --- diff --git a/src/auth/negotiate/auth_negotiate.cc b/src/auth/negotiate/auth_negotiate.cc index da6b2b84b4..a03af00596 100644 --- a/src/auth/negotiate/auth_negotiate.cc +++ b/src/auth/negotiate/auth_negotiate.cc @@ -367,13 +367,12 @@ NegotiateUser::~NegotiateUser() debugs(29, 5, "NegotiateUser::~NegotiateUser: doing nothing to clearNegotiate scheme data for '" << this << "'"); } -static stateful_helper_callback_t +static void authenticateNegotiateHandleReply(void *data, void *lastserver, char *reply) { authenticateStateData *r = static_cast(data); int valid; - stateful_helper_callback_t result = S_HELPER_UNKNOWN; char *blob, *arg = NULL; AuthUserRequest *auth_user_request; @@ -385,11 +384,10 @@ authenticateNegotiateHandleReply(void *data, void *lastserver, char *reply) valid = cbdataReferenceValid(r->data); if (!valid) { - debugs(29, 1, "authenticateNegotiateHandleReply: invalid callback data. Releasing helper '" << lastserver << "'."); + debugs(29, 1, "authenticateNegotiateHandleReply: invalid callback data. helper '" << lastserver << "'."); cbdataReferenceDone(r->data); authenticateStateFree(r); - debugs(29, 9, "authenticateNegotiateHandleReply: telling stateful helper : " << S_HELPER_RELEASE); - return S_HELPER_RELEASE; + return; } if (!reply) { @@ -433,18 +431,16 @@ authenticateNegotiateHandleReply(void *data, void *lastserver, char *reply) if (arg) *arg++ = '\0'; safe_free(negotiate_request->server_blob); - negotiate_request->request->flags.must_keepalive = 1; - if (negotiate_request->request->flags.proxy_keepalive) { - negotiate_request->server_blob = xstrdup(blob); - negotiate_request->auth_state = AUTHENTICATE_STATE_IN_PROGRESS; - auth_user_request->denyMessage("Authentication in progress"); - debugs(29, 4, "authenticateNegotiateHandleReply: Need to challenge the client with a server blob '" << blob << "'"); - result = S_HELPER_RESERVE; - } else { - negotiate_request->auth_state = AUTHENTICATE_STATE_FAILED; - auth_user_request->denyMessage("NTLM authentication requires a persistent connection"); - result = S_HELPER_RELEASE; - } + negotiate_request->request->flags.must_keepalive = 1; + if (negotiate_request->request->flags.proxy_keepalive) { + negotiate_request->server_blob = xstrdup(blob); + negotiate_request->auth_state = AUTHENTICATE_STATE_IN_PROGRESS; + auth_user_request->denyMessage("Authentication in progress"); + debugs(29, 4, "authenticateNegotiateHandleReply: Need to challenge the client with a server blob '" << blob << "'"); + } else { + negotiate_request->auth_state = AUTHENTICATE_STATE_FAILED; + auth_user_request->denyMessage("NTLM authentication requires a persistent connection"); + } } else if (strncasecmp(reply, "AF ", 3) == 0 && arg != NULL) { /* we're finished, release the helper */ @@ -463,8 +459,6 @@ authenticateNegotiateHandleReply(void *data, void *lastserver, char *reply) negotiate_request->auth_state = AUTHENTICATE_STATE_DONE; - result = S_HELPER_RELEASE; - debugs(29, 4, "authenticateNegotiateHandleReply: Successfully validated user via Negotiate. Username '" << blob << "'"); /* connection is authenticated */ @@ -510,8 +504,6 @@ authenticateNegotiateHandleReply(void *data, void *lastserver, char *reply) authenticateNegotiateReleaseServer(negotiate_request); - result = S_HELPER_RELEASE; - debugs(29, 4, "authenticateNegotiateHandleReply: Failed validating user via Negotiate. Error returned '" << blob << "'"); } else if (strncasecmp(reply, "BH ", 3) == 0) { /* TODO kick off a refresh process. This can occur after a YR or after @@ -523,7 +515,6 @@ authenticateNegotiateHandleReply(void *data, void *lastserver, char *reply) negotiate_request->auth_state = AUTHENTICATE_STATE_FAILED; safe_free(negotiate_request->server_blob); authenticateNegotiateReleaseServer(negotiate_request); - result = S_HELPER_RELEASE; debugs(29, 1, "authenticateNegotiateHandleReply: Error validating user via Negotiate. Error returned '" << reply << "'"); } else { /* protocol error */ @@ -537,8 +528,6 @@ authenticateNegotiateHandleReply(void *data, void *lastserver, char *reply) r->handler(r->data, NULL); cbdataReferenceDone(r->data); authenticateStateFree(r); - debugs(29, 9, "authenticateNegotiateHandleReply: telling stateful helper : " << result); - return result; } static void diff --git a/src/auth/ntlm/auth_ntlm.cc b/src/auth/ntlm/auth_ntlm.cc index 165633578f..8c1117e263 100644 --- a/src/auth/ntlm/auth_ntlm.cc +++ b/src/auth/ntlm/auth_ntlm.cc @@ -329,13 +329,12 @@ NTLMUser::~NTLMUser() debugs(29, 5, "NTLMUser::~NTLMUser: doing nothing to clearNTLM scheme data for '" << this << "'"); } -static stateful_helper_callback_t +static void authenticateNTLMHandleReply(void *data, void *lastserver, char *reply) { authenticateStateData *r = static_cast(data); int valid; - stateful_helper_callback_t result = S_HELPER_UNKNOWN; char *blob; AuthUserRequest *auth_user_request; @@ -347,11 +346,10 @@ authenticateNTLMHandleReply(void *data, void *lastserver, char *reply) valid = cbdataReferenceValid(r->data); if (!valid) { - debugs(29, 1, "authenticateNTLMHandleReply: invalid callback data. Releasing helper '" << lastserver << "'."); + debugs(29, 1, "authenticateNTLMHandleReply: invalid callback data. helper '" << lastserver << "'."); cbdataReferenceDone(r->data); authenticateStateFree(r); - debugs(29, 9, "authenticateNTLMHandleReply: telling stateful helper : " << S_HELPER_RELEASE); - return S_HELPER_RELEASE; + return; } if (!reply) { @@ -389,25 +387,22 @@ authenticateNTLMHandleReply(void *data, void *lastserver, char *reply) if (strncasecmp(reply, "TT ", 3) == 0) { /* we have been given a blob to send to the client */ safe_free(ntlm_request->server_blob); - ntlm_request->request->flags.must_keepalive = 1; - if (ntlm_request->request->flags.proxy_keepalive) { - ntlm_request->server_blob = xstrdup(blob); - ntlm_request->auth_state = AUTHENTICATE_STATE_IN_PROGRESS; - auth_user_request->denyMessage("Authentication in progress"); - debugs(29, 4, "authenticateNTLMHandleReply: Need to challenge the client with a server blob '" << blob << "'"); - result = S_HELPER_RESERVE; - } else { - ntlm_request->auth_state = AUTHENTICATE_STATE_FAILED; - auth_user_request->denyMessage("NTLM authentication requires a persistent connection"); - result = S_HELPER_RELEASE; - } + ntlm_request->request->flags.must_keepalive = 1; + if (ntlm_request->request->flags.proxy_keepalive) { + ntlm_request->server_blob = xstrdup(blob); + ntlm_request->auth_state = AUTHENTICATE_STATE_IN_PROGRESS; + auth_user_request->denyMessage("Authentication in progress"); + debugs(29, 4, "authenticateNTLMHandleReply: Need to challenge the client with a server blob '" << blob << "'"); + } else { + ntlm_request->auth_state = AUTHENTICATE_STATE_FAILED; + auth_user_request->denyMessage("NTLM authentication requires a persistent connection"); + } } else if (strncasecmp(reply, "AF ", 3) == 0) { /* we're finished, release the helper */ ntlm_user->username(blob); auth_user_request->denyMessage("Login successful"); safe_free(ntlm_request->server_blob); - result = S_HELPER_RELEASE; debugs(29, 4, "authenticateNTLMHandleReply: Successfully validated user via NTLM. Username '" << blob << "'"); /* connection is authenticated */ debugs(29, 4, "AuthNTLMUserRequest::authenticate: authenticated user " << ntlm_user->username()); @@ -441,7 +436,6 @@ authenticateNTLMHandleReply(void *data, void *lastserver, char *reply) ntlm_request->auth_state = AUTHENTICATE_STATE_FAILED; safe_free(ntlm_request->server_blob); authenticateNTLMReleaseServer(ntlm_request); - result = S_HELPER_RELEASE; debugs(29, 4, "authenticateNTLMHandleReply: Failed validating user via NTLM. Error returned '" << blob << "'"); } else if (strncasecmp(reply, "BH ", 3) == 0) { /* TODO kick off a refresh process. This can occur after a YR or after @@ -453,7 +447,6 @@ authenticateNTLMHandleReply(void *data, void *lastserver, char *reply) ntlm_request->auth_state = AUTHENTICATE_STATE_FAILED; safe_free(ntlm_request->server_blob); authenticateNTLMReleaseServer(ntlm_request); - result = S_HELPER_RELEASE; debugs(29, 1, "authenticateNTLMHandleReply: Error validating user via NTLM. Error returned '" << reply << "'"); } else { /* protocol error */ @@ -467,8 +460,6 @@ authenticateNTLMHandleReply(void *data, void *lastserver, char *reply) r->handler(r->data, NULL); cbdataReferenceDone(r->data); authenticateStateFree(r); - debugs(29, 9, "authenticateNTLMHandleReply: telling stateful helper : " << result); - return result; } static void diff --git a/src/helper.cc b/src/helper.cc index 959edaf6ed..269fb3d0a7 100644 --- a/src/helper.cc +++ b/src/helper.cc @@ -991,6 +991,7 @@ helperStatefulHandleRead(int fd, char *buf, size_t len, comm_err_t flag, int xer if ((t = strchr(srv->rbuf, '\n'))) { /* end of reply found */ + int called = 1; debugs(84, 3, "helperStatefulHandleRead: end of reply found"); if (t > srv->rbuf && t[-1] == '\r') @@ -999,36 +1000,10 @@ helperStatefulHandleRead(int fd, char *buf, size_t len, comm_err_t flag, int xer *t = '\0'; if (r && cbdataReferenceValid(r->data)) { - switch ((r->callback(r->data, srv, srv->rbuf))) { /*if non-zero reserve helper */ - - case S_HELPER_UNKNOWN: - fatal("helperStatefulHandleRead: either a non-state aware callback was give to the stateful helper routines, or an uninitialised callback response was received.\n"); - break; - - case S_HELPER_RELEASE: /* helper finished with */ - - srv->flags.reserved = 0; - - if ((srv->parent->OnEmptyQueue != NULL) && (srv->data)) - srv->parent->OnEmptyQueue(srv->data); - - debugs(84, 5, "StatefulHandleRead: releasing " << hlp->id_name << " #" << srv->index + 1); - - break; - - case S_HELPER_RESERVE: /* 'pin' this helper for the caller */ - - srv->flags.reserved = 1; - debugs(84, 5, "StatefulHandleRead: reserving " << hlp->id_name << " #" << srv->index + 1); - - break; - - default: - fatal("helperStatefulHandleRead: unknown stateful helper callback result.\n"); - } - + r->callback(r->data, srv, srv->rbuf); } else { debugs(84, 1, "StatefulHandleRead: no callback data registered"); + called = 0; } srv->flags.busy = 0; @@ -1042,7 +1017,10 @@ helperStatefulHandleRead(int fd, char *buf, size_t len, comm_err_t flag, int xer tvSubMsec(srv->dispatch_time, current_time), hlp->stats.replies, REDIRECT_AV_FACTOR); - helperStatefulServerDone(srv); + if (called) + helperStatefulServerDone(srv); + else + helperStatefulReleaseServer(srv); } if (srv->rfd != -1) @@ -1309,6 +1287,7 @@ helperStatefulDispatch(helper_stateful_server * srv, helper_stateful_request * r if (!cbdataReferenceValid(r->data)) { debugs(84, 1, "helperStatefulDispatch: invalid callback data"); helperStatefulRequestFree(r); + helperStatefulReleaseServer(srv); return; } @@ -1331,6 +1310,7 @@ helperStatefulDispatch(helper_stateful_server * srv, helper_stateful_request * r } srv->flags.busy = 1; + srv->flags.reserved = 1; srv->request = r; srv->dispatch_time = current_time; comm_write(srv->wfd, diff --git a/src/helper.h b/src/helper.h index 8de49dd716..150c19b1ae 100644 --- a/src/helper.h +++ b/src/helper.h @@ -53,7 +53,7 @@ typedef struct _helper_flags helper_flags; typedef struct _helper_stateful_flags helper_stateful_flags; -typedef stateful_helper_callback_t HLPSCB(void *, void *lastserver, char *buf); +typedef void HLPSCB(void *, void *lastserver, char *buf); struct _helper {