Revert revno11955 fix for bug 3444

diff --git a/src/ClientRequestContext.h b/src/ClientRequestContext.h
index 6290eaf9b293a02adfc902362bda0282352e5632..4ea052eba050ec288a56e97e2bde875dd448229c 100644 (file)
--- a/src/ClientRequestContext.h
+++ b/src/ClientRequestContext.h
@@ -47,7 +47,7 @@ public:
      */
     bool sslBumpAccessCheck();
     /// The callback function for ssl-bump access check list
-    void sslBumpAccessCheckDone(const allow_t &answer);
+    void sslBumpAccessCheckDone(bool doSslBump);
 #endif
 
     ClientHttpRequest *http;
@@ -69,11 +69,7 @@ public:
     bool sslBumpCheckDone;
 #endif
 
-    /// Send authentication response (challenge or error) if ACL result indicates one is needed
-    /// \return true if an error page of any kind has been sent back to the client.
-    // NP: public only until ACLChecklist::nonBlockingCheck() takes Async::Pointer to a call
-    bool maybeSendAuthChallenge(const allow_t &answer);
-
+private:
     CBDATA_CLASS(ClientRequestContext);
 };
 

diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index 5baaa82cbb818d4186893f29782900a6d9cfebe6..422a4124986c7fa685e75e2abfc41eabd9c306fc 100644 (file)
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -740,12 +740,17 @@ clientAccessCheckDoneWrapper(allow_t answer, void *data)
     calloutContext->clientAccessCheckDone(answer);
 }
 
-bool
-ClientRequestContext::maybeSendAuthChallenge(const allow_t &answer)
+void
+ClientRequestContext::clientAccessCheckDone(const allow_t &answer)
 {
     acl_checklist = NULL;
     err_type page_id;
     http_status status;
+    debugs(85, 2, "The request " <<
+           RequestMethodStr(http->request->method) << " " <<
+           http->uri << " is " << answer <<
+           ", because it matched '" <<
+           (AclMatchedName ? AclMatchedName : "NO ACL's") << "'" );
 
 #if USE_AUTH
     char const *proxy_auth_msg = "<null>";
@@ -755,102 +760,75 @@ ClientRequestContext::maybeSendAuthChallenge(const allow_t &answer)
         proxy_auth_msg = http->request->auth_user_request->denyMessage("<null>");
 #endif
 
-    bool auth_challenge = false;
-    switch (answer) {
-    case ACCESS_ALLOWED:
-    case ACCESS_AUTH_EXPIRED_OK:
-        // No authentication challenge on these ACL results
-        return auth_challenge;
-
-    case ACCESS_DENIED:
-    case ACCESS_DUNNO:
-        // MAYBE challenge on these ACL results
-        auth_challenge |= aclIsProxyAuth(AclMatchedName);
-        break;
-
-    case ACCESS_AUTH_REQUIRED:
-    case ACCESS_AUTH_EXPIRED_BAD:
-        // Send an auth challenge or error
-        auth_challenge = true;
-    }
+    if (answer != ACCESS_ALLOWED && answer != ACCESS_AUTH_EXPIRED_OK) {
+        // auth has a grace period where credentials can be expired but okay not to challenge.
 
-    // auth has a grace period where credentials can be expired but okay not to challenge.
-    debugs(85, 5, "Access Denied: " << http->uri);
-    debugs(85, 5, "AclMatchedName = " << (AclMatchedName ? AclMatchedName : "<null>"));
+        /* Send an auth challenge or error */
+        // XXX: do we still need aclIsProxyAuth() ?
+        bool auth_challenge = (answer == ACCESS_AUTH_REQUIRED || answer == ACCESS_AUTH_EXPIRED_BAD || aclIsProxyAuth(AclMatchedName));
+        debugs(85, 5, "Access Denied: " << http->uri);
+        debugs(85, 5, "AclMatchedName = " << (AclMatchedName ? AclMatchedName : "<null>"));
 #if USE_AUTH
-    if (auth_challenge)
-        debugs(33, 5, "Proxy Auth Message = " << (proxy_auth_msg ? proxy_auth_msg : "<null>"));
+        if (auth_challenge)
+            debugs(33, 5, "Proxy Auth Message = " << (proxy_auth_msg ? proxy_auth_msg : "<null>"));
 #endif
 
-    /*
-     * NOTE: get page_id here, based on AclMatchedName because if
-     * USE_DELAY_POOLS is enabled, then AclMatchedName gets clobbered in
-     * the clientCreateStoreEntry() call just below.  Pedro Ribeiro
-     * <pribeiro@isel.pt>
-     */
-    page_id = aclGetDenyInfoPage(&Config.denyInfoList, AclMatchedName, auth_challenge);
+        /*
+         * NOTE: get page_id here, based on AclMatchedName because if
+         * USE_DELAY_POOLS is enabled, then AclMatchedName gets clobbered in
+         * the clientCreateStoreEntry() call just below.  Pedro Ribeiro
+         * <pribeiro@isel.pt>
+         */
+        page_id = aclGetDenyInfoPage(&Config.denyInfoList, AclMatchedName, answer != ACCESS_AUTH_REQUIRED);
 
-    http->logType = LOG_TCP_DENIED;
+        http->logType = LOG_TCP_DENIED;
 
-    if (auth_challenge) {
+        if (auth_challenge) {
 #if USE_AUTH
-        if (http->request->flags.sslBumped) {
-            /*SSL Bumped request, authentication is not possible*/
-            status = HTTP_FORBIDDEN;
-        } else if (!http->flags.accel) {
-            /* Proxy authorisation needed */
-            status = HTTP_PROXY_AUTHENTICATION_REQUIRED;
-        } else {
-            /* WWW authorisation needed */
-            status = HTTP_UNAUTHORIZED;
-        }
+            if (http->request->flags.sslBumped) {
+                /*SSL Bumped request, authentication is not possible*/
+                status = HTTP_FORBIDDEN;
+            } else if (!http->flags.accel) {
+                /* Proxy authorisation needed */
+                status = HTTP_PROXY_AUTHENTICATION_REQUIRED;
+            } else {
+                /* WWW authorisation needed */
+                status = HTTP_UNAUTHORIZED;
+            }
 #else
-        // need auth, but not possible to do.
-        status = HTTP_FORBIDDEN;
+            // need auth, but not possible to do.
+            status = HTTP_FORBIDDEN;
 #endif
-        if (page_id == ERR_NONE)
-            page_id = ERR_CACHE_ACCESS_DENIED;
-    } else {
-        status = HTTP_FORBIDDEN;
+            if (page_id == ERR_NONE)
+                page_id = ERR_CACHE_ACCESS_DENIED;
+        } else {
+            status = HTTP_FORBIDDEN;
 
-        if (page_id == ERR_NONE)
-            page_id = ERR_ACCESS_DENIED;
-    }
+            if (page_id == ERR_NONE)
+                page_id = ERR_ACCESS_DENIED;
+        }
 
-    clientStreamNode *node = (clientStreamNode *)http->client_stream.tail->prev->data;
-    clientReplyContext *repContext = dynamic_cast<clientReplyContext *>(node->data.getRaw());
-    assert (repContext);
-    Ip::Address tmpnoaddr;
-    tmpnoaddr.SetNoAddr();
-    repContext->setReplyToError(page_id, status,
-                                http->request->method,
-                                NULL,
-                                http->getConn() != NULL ? http->getConn()->clientConnection->remote : tmpnoaddr,
-                                http->request,
-                                NULL,
+        clientStreamNode *node = (clientStreamNode *)http->client_stream.tail->prev->data;
+        clientReplyContext *repContext = dynamic_cast<clientReplyContext *>(node->data.getRaw());
+        assert (repContext);
+        Ip::Address tmpnoaddr;
+        tmpnoaddr.SetNoAddr();
+        repContext->setReplyToError(page_id, status,
+                                    http->request->method, NULL,
+                                    http->getConn() != NULL ? http->getConn()->clientConnection->remote : tmpnoaddr,
+                                    http->request,
+                                    NULL,
 #if USE_AUTH
-                                http->getConn() != NULL && http->getConn()->auth_user_request != NULL ?
-                                http->getConn()->auth_user_request : http->request->auth_user_request);
+                                    http->getConn() != NULL && http->getConn()->auth_user_request != NULL ?
+                                    http->getConn()->auth_user_request : http->request->auth_user_request);
 #else
-                                NULL);
+                                    NULL);
 #endif
-    http->getConn()->flags.readMore = true; // resume any pipeline reads.
-    node = (clientStreamNode *)http->client_stream.tail->data;
-    clientStreamRead(node, http, node->readBuffer);
-    return true;
-}
-
-void
-ClientRequestContext::clientAccessCheckDone(const allow_t &answer)
-{
-    debugs(85, 2, "The request " <<
-           RequestMethodStr(http->request->method) << " " <<
-           http->uri << " is " << answer <<
-           ", because it matched '" <<
-           (AclMatchedName ? AclMatchedName : "NO ACL's") << "'" );
-
-    if (maybeSendAuthChallenge(answer))
+        http->getConn()->flags.readMore = true; // resume any pipeline reads.
+        node = (clientStreamNode *)http->client_stream.tail->data;
+        clientStreamRead(node, http, node->readBuffer);
         return;
+    }
 
     /* ACCESS_ALLOWED (or auth in grace period ACCESS_AUTH_EXPIRED_OK) continues here ... */
     safe_free(http->uri);
@@ -896,12 +874,11 @@ static void
 clientRedirectAccessCheckDone(allow_t answer, void *data)
 {
     ClientRequestContext *context = (ClientRequestContext *)data;
-
-    if (context->maybeSendAuthChallenge(answer))
-        return;
+    ClientHttpRequest *http = context->http;
+    context->acl_checklist = NULL;
 
     if (answer == ACCESS_ALLOWED)
-        redirectStart(context->http, clientRedirectDoneWrapper, context);
+        redirectStart(http, clientRedirectDoneWrapper, context);
     else
         context->clientRedirectDone(NULL);
 }
@@ -1323,19 +1300,16 @@ static void
 sslBumpAccessCheckDoneWrapper(allow_t answer, void *data)
 {
     ClientRequestContext *calloutContext = static_cast<ClientRequestContext *>(data);
-    calloutContext->sslBumpAccessCheckDone(answer);
+
+    if (!calloutContext->httpStateIsValid())
+        return;
+    calloutContext->sslBumpAccessCheckDone(answer == ACCESS_ALLOWED);
 }
 
 void
-ClientRequestContext::sslBumpAccessCheckDone(const allow_t &answer)
+ClientRequestContext::sslBumpAccessCheckDone(bool doSslBump)
 {
-    if (!httpStateIsValid())
-        return;
-
-    if (maybeSendAuthChallenge(answer))
-        return;
-
-    http->sslBumpNeeded(answer == ACCESS_ALLOWED);
+    http->sslBumpNeeded(doSslBump);
     http->doCallouts();
 }
 #endif