Minimize direct comparisons with ACCESS_ALLOWED and ACCESS_DENIED.

No functionality changes expected.

Added allow_t API to avoid direct comparisons with ACCESS_ALLOWED and
ACCESS_DENIED. Developers using direct comparisons eventually mishandle
exceptional ACCESS_DUNNO and ACCESS_AUTH_REQUIRED cases where neither
"allow" nor "deny" rule matched. The new API cannot fully prevent such
bugs, but should either led the developer to the right choice (usually
.allowed()) or alert the reviewer about an unusual choice (i.e.,
denied()).

The vast majority of checks use allowed(), but we could not eliminate
the remaining denied() cases ("miss_access" and "cache" directives) for
backward compatibility reasons -- previously "working" deployments may
suddenly start blocking cache misses and/or stop caching:
http://lists.squid-cache.org/pipermail/squid-dev/2017-May/008576.html

diff --git a/src/DelayId.cc b/src/DelayId.cc
index aa54fe4e6649e4254ef0093428c6f837ae7c2da1..50321ce04590935540ff57f78e4cc9eab0d1203e 100644 (file)
--- a/src/DelayId.cc
+++ b/src/DelayId.cc
@@ -101,7 +101,7 @@ DelayId::DelayClient(ClientHttpRequest * http, HttpReply *reply)
         if (http->getConn() != NULL)
             ch.conn(http->getConn());
 
-        if (DelayPools::delay_data[pool].theComposite().getRaw() && ch.fastCheck() == ACCESS_ALLOWED) {
+        if (DelayPools::delay_data[pool].theComposite().getRaw() && ch.fastCheck().allowed()) {
 
             DelayId result (pool + 1);
             CompositePoolNode::CompositeSelectionDetails details;

diff --git a/src/FwdState.cc b/src/FwdState.cc
index 631bf8e0d897013eb95c48470a821b71ced8b7fa..206cbdc7b96c492f88b68fd5241034397d2d8849 100644 (file)
--- a/src/FwdState.cc
+++ b/src/FwdState.cc
@@ -332,7 +332,7 @@ FwdState::Start(const Comm::ConnectionPointer &clientConn, StoreEntry *entry, Ht
          */
         ACLFilledChecklist ch(Config.accessList.miss, request, NULL);
         ch.src_addr = request->client_addr;
-        if (ch.fastCheck() == ACCESS_DENIED) {
+        if (ch.fastCheck().denied()) {
             err_type page_id;
             page_id = aclGetDenyInfoPage(&Config.denyInfoList, AclMatchedName, 1);
 
@@ -1218,7 +1218,7 @@ FwdState::pconnPop(const Comm::ConnectionPointer &dest, const char *domain)
     bool retriable = checkRetriable();
     if (!retriable && Config.accessList.serverPconnForNonretriable) {
         ACLFilledChecklist ch(Config.accessList.serverPconnForNonretriable, request, NULL);
-        retriable = (ch.fastCheck() == ACCESS_ALLOWED);
+        retriable = ch.fastCheck().allowed();
     }
     // always call shared pool first because we need to close an idle
     // connection there if we have to use a standby connection.
@@ -1270,7 +1270,7 @@ tos_t
 aclMapTOS(acl_tos * head, ACLChecklist * ch)
 {
     for (acl_tos *l = head; l; l = l->next) {
-        if (!l->aclList || ch->fastCheck(l->aclList) == ACCESS_ALLOWED)
+        if (!l->aclList || ch->fastCheck(l->aclList).allowed())
             return l->tos;
     }
 
@@ -1282,7 +1282,7 @@ nfmark_t
 aclMapNfmark(acl_nfmark * head, ACLChecklist * ch)
 {
     for (acl_nfmark *l = head; l; l = l->next) {
-        if (!l->aclList || ch->fastCheck(l->aclList) == ACCESS_ALLOWED)
+        if (!l->aclList || ch->fastCheck(l->aclList).allowed())
             return l->nfmark;
     }
 
@@ -1333,7 +1333,7 @@ getOutgoingAddress(HttpRequest * request, Comm::ConnectionPointer conn)
         if (conn->remote.isIPv4() != l->addr.isIPv4()) continue;
 
         /* check ACLs for this outgoing address */
-        if (!l->aclList || ch.fastCheck(l->aclList) == ACCESS_ALLOWED) {
+        if (!l->aclList || ch.fastCheck(l->aclList).allowed()) {
             conn->local = l->addr;
             return;
         }

diff --git a/src/HttpHeaderTools.cc b/src/HttpHeaderTools.cc
index c1669042e9bc66d002cb20ff3a2596f6b6d588cc..a5bea3dfec2c270ba5cc57865182d686a363c1b5 100644 (file)
--- a/src/HttpHeaderTools.cc
+++ b/src/HttpHeaderTools.cc
@@ -289,7 +289,7 @@ httpHdrMangle(HttpHeaderEntry * e, HttpRequest * request, HeaderManglers *hms)
 
     ACLFilledChecklist checklist(hm->access_list, request, NULL);
 
-    if (checklist.fastCheck() == ACCESS_ALLOWED) {
+    if (checklist.fastCheck().allowed()) {
         /* aclCheckFast returns true for allow. */
         debugs(66, 7, "checklist for mangler is positive. Mangle");
         retval = 1;
@@ -479,7 +479,7 @@ httpHdrAdd(HttpHeader *heads, HttpRequest *request, const AccessLogEntryPointer
     ACLFilledChecklist checklist(NULL, request, NULL);
 
     for (HeaderWithAclList::const_iterator hwa = headersAdd.begin(); hwa != headersAdd.end(); ++hwa) {
-        if (!hwa->aclList || checklist.fastCheck(hwa->aclList) == ACCESS_ALLOWED) {
+        if (!hwa->aclList || checklist.fastCheck(hwa->aclList).allowed()) {
             const char *fieldValue = NULL;
             MemBuf mb;
             if (hwa->quoted) {

diff --git a/src/HttpReply.cc b/src/HttpReply.cc
index 2d69207be0b7f0ebcc3c047c7d749a93021f9b75..8b5c222519f6e078cac8d330863a041972386d9f 100644 (file)
--- a/src/HttpReply.cc
+++ b/src/HttpReply.cc
@@ -517,7 +517,7 @@ HttpReply::calcMaxBodySize(HttpRequest& request) const
     HTTPMSGLOCK(ch.reply);
     for (AclSizeLimit *l = Config.ReplyBodySize; l; l = l -> next) {
         /* if there is no ACL list or if the ACLs listed match use this size value */
-        if (!l->aclList || ch.fastCheck(l->aclList) == ACCESS_ALLOWED) {
+        if (!l->aclList || ch.fastCheck(l->aclList).allowed()) {
             debugs(58, 4, HERE << "bodySizeMax=" << bodySizeMax);
             bodySizeMax = l->size; // may be -1
             break;

diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc
index 6daf43885e45691c2c88558a5bf670484276a5cb..00c0fdb3883c4915402d80d1dd4d57606d4be4af 100644 (file)
--- a/src/HttpRequest.cc
+++ b/src/HttpRequest.cc
@@ -609,7 +609,7 @@ HttpRequest::getRangeOffsetLimit()
 
     for (AclSizeLimit *l = Config.rangeOffsetLimit; l; l = l -> next) {
         /* if there is no ACL list or if the ACLs listed match use this limit value */
-        if (!l->aclList || ch.fastCheck(l->aclList) == ACCESS_ALLOWED) {
+        if (!l->aclList || ch.fastCheck(l->aclList).allowed()) {
             debugs(58, 4, HERE << "rangeOffsetLimit=" << rangeOffsetLimit);
             rangeOffsetLimit = l->size; // may be -1
             break;
@@ -724,7 +724,7 @@ HttpRequest::manager(const CbcPointer<ConnStateData> &aMgr, const AccessLogEntry
             if (Config.accessList.spoof_client_ip) {
                 ACLFilledChecklist *checklist = new ACLFilledChecklist(Config.accessList.spoof_client_ip, this, clientConnection->rfc931);
                 checklist->al = al;
-                flags.spoofClientIp = (checklist->fastCheck() == ACCESS_ALLOWED);
+                flags.spoofClientIp = checklist->fastCheck().allowed();
                 delete checklist;
             } else
                 flags.spoofClientIp = true;

diff --git a/src/Notes.cc b/src/Notes.cc
index c1b25bc679f4f1b3aaf5341448cbf73200888c81..7d6d1fc079b28c6b449069503413e926cea761a7 100644 (file)
--- a/src/Notes.cc
+++ b/src/Notes.cc
@@ -75,10 +75,10 @@ Note::match(HttpRequest *request, HttpReply *reply, const AccessLogEntry::Pointe
 
     for (auto v: values) {
         assert(v->aclList);
-        const int ret = ch.fastCheck(v->aclList);
+        const auto ret = ch.fastCheck(v->aclList);
         debugs(93, 5, "Check for header name: " << theKey << ": " << v->value() <<
                ", HttpRequest: " << request << " HttpReply: " << reply << " matched: " << ret);
-        if (ret == ACCESS_ALLOWED) {
+        if (ret.allowed()) {
             matched = v->format(al);
             return true;
         }

diff --git a/src/acl/Acl.h b/src/acl/Acl.h
index 9ed1b19c30c90b01f44314b5af8e8e117007b042..57300d6c873a424a16823b4d60a238c41c3aadbc 100644 (file)
--- a/src/acl/Acl.h
+++ b/src/acl/Acl.h
@@ -16,6 +16,7 @@
 #include "dlink.h"
 #include "sbuf/forward.h"
 
+#include <algorithm>
 #include <ostream>
 
 class ConfigParser;
@@ -133,6 +134,22 @@ public:
         return code;
     }
 
+    /// Whether an "allow" rule matched. If in doubt, use this popular method.
+    /// Also use this method to treat exceptional ACCESS_DUNNO and
+    /// ACCESS_AUTH_REQUIRED outcomes as if a "deny" rule matched.
+    /// See also: denied().
+    bool allowed() const { return code == ACCESS_ALLOWED; }
+
+    /// Whether a "deny" rule matched. Avoid this rarely used method.
+    /// Use this method (only) to treat exceptional ACCESS_DUNNO and
+    /// ACCESS_AUTH_REQUIRED outcomes as if an "allow" rule matched.
+    /// See also: allowed().
+    bool denied() const { return code == ACCESS_DENIED; }
+
+    /// whether there was either a default rule, a rule without any ACLs, or a
+    /// a rule with ACLs that all matched
+    bool someRuleMatched() const { return allowed() || denied(); }
+
     aclMatchCode code; ///< ACCESS_* code
     int kind; ///< which custom access list verb matched
 };

diff --git a/src/acl/Tree.h b/src/acl/Tree.h
index a554f26d88885178930e7e6b1ad8ab985e5cbe7f..34f4796833f294fe76d0dd41044226cf90bcfd68 100644 (file)
--- a/src/acl/Tree.h
+++ b/src/acl/Tree.h
@@ -52,7 +52,7 @@ protected:
 inline const char *
 AllowOrDeny(const allow_t &action)
 {
-    return action == ACCESS_ALLOWED ? "allow" : "deny";
+    return action.allowed() ? "allow" : "deny";
 }
 
 template <class ActionToStringConverter>

diff --git a/src/adaptation/AccessCheck.cc b/src/adaptation/AccessCheck.cc
index 6ce6d91975710d6c1af7d75f3c93d1bfbb88a8d4..97242dbe6ab62cc5c0b18f61b75627d5672a5748 100644 (file)
--- a/src/adaptation/AccessCheck.cc
+++ b/src/adaptation/AccessCheck.cc
@@ -174,7 +174,7 @@ Adaptation::AccessCheck::noteAnswer(allow_t answer)
     Must(!candidates.empty()); // the candidate we were checking must be there
     debugs(93,5, HERE << topCandidate() << " answer=" << answer);
 
-    if (answer == ACCESS_ALLOWED) { // the rule matched
+    if (answer.allowed()) { // the rule matched
         ServiceGroupPointer g = topGroup();
         if (g != NULL) { // the corresponding group found
             callBack(g);

diff --git a/src/adaptation/icap/Launcher.cc b/src/adaptation/icap/Launcher.cc
index 4b9c51d213814a3f1c24c86d8c1af8a9979c0242..0b19e966182410bccaa06122929abd393366e099 100644 (file)
--- a/src/adaptation/icap/Launcher.cc
+++ b/src/adaptation/icap/Launcher.cc
@@ -145,7 +145,7 @@ bool Adaptation::Icap::Launcher::canRepeat(Adaptation::Icap::XactAbortInfo &info
     cl->reply = info.icapReply;
     HTTPMSGLOCK(cl->reply);
 
-    bool result = cl->fastCheck() == ACCESS_ALLOWED;
+    bool result = cl->fastCheck().allowed();
     delete cl;
     return result;
 }

diff --git a/src/auth/UserRequest.cc b/src/auth/UserRequest.cc
index 9f6970a79e535c1c8a1a70ecbdf3516b8037240a..195cfb8559868035b569832853f8ce833d1ca8a4 100644 (file)
--- a/src/auth/UserRequest.cc
+++ b/src/auth/UserRequest.cc
@@ -469,7 +469,7 @@ schemesConfig(HttpRequest *request, HttpReply *rep)
         ch.reply = rep;
         HTTPMSGLOCK(ch.reply);
         const allow_t answer = ch.fastCheck(Auth::TheConfig.schemeAccess);
-        if (answer == ACCESS_ALLOWED)
+        if (answer.allowed())
             return Auth::TheConfig.schemeLists.at(answer.kind).authConfigs;
     }
     return Auth::TheConfig.schemes;

diff --git a/src/client_side.cc b/src/client_side.cc
index 2ec8da96aeaece118402120ee769dba3fed84b42..700c54bcca050afb99f9910939330986e7ad527a 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -462,7 +462,7 @@ ClientHttpRequest::logRequest()
             statsCheck.reply = al->reply;
             HTTPMSGLOCK(statsCheck.reply);
         }
-        updatePerformanceCounters = (statsCheck.fastCheck() == ACCESS_ALLOWED);
+        updatePerformanceCounters = statsCheck.fastCheck().allowed();
     }
 
     if (updatePerformanceCounters) {
@@ -1527,7 +1527,7 @@ bool ConnStateData::serveDelayedError(Http::Stream *context)
             if (Config.ssl_client.cert_error) {
                 ACLFilledChecklist check(Config.ssl_client.cert_error, request, dash_str);
                 check.sslErrors = new Security::CertErrors(Security::CertError(SQUID_X509_V_ERR_DOMAIN_MISMATCH, srvCert));
-                allowDomainMismatch = (check.fastCheck() == ACCESS_ALLOWED);
+                allowDomainMismatch = check.fastCheck().allowed();
                 delete check.sslErrors;
                 check.sslErrors = NULL;
             }
@@ -1581,7 +1581,7 @@ clientTunnelOnError(ConnStateData *conn, Http::StreamPointer &context, HttpReque
         checklist.my_addr = conn->clientConnection->local;
         checklist.conn(conn);
         allow_t answer = checklist.fastCheck();
-        if (answer == ACCESS_ALLOWED && answer.kind == 1) {
+        if (answer.allowed() && answer.kind == 1) {
             debugs(33, 3, "Request will be tunneled to server");
             if (context) {
                 assert(conn->pipeline.front() == context); // XXX: still assumes HTTP/1 semantics
@@ -1826,7 +1826,7 @@ ConnStateData::proxyProtocolValidateClient()
     ch.my_addr = clientConnection->local;
     ch.conn(this);
 
-    if (ch.fastCheck() != ACCESS_ALLOWED)
+    if (!ch.fastCheck().allowed())
         return proxyProtocolError("PROXY client not permitted by ACLs");
 
     return true;
@@ -2446,7 +2446,7 @@ ConnStateData::whenClientIpKnown()
         ACLFilledChecklist identChecklist(Ident::TheConfig.identLookup, NULL, NULL);
         identChecklist.src_addr = clientConnection->remote;
         identChecklist.my_addr = clientConnection->local;
-        if (identChecklist.fastCheck() == ACCESS_ALLOWED)
+        if (identChecklist.fastCheck().allowed())
             Ident::Start(clientConnection, clientIdentDone, this);
     }
 #endif
@@ -2474,7 +2474,7 @@ ConnStateData::whenClientIpKnown()
             if (pools[pool]->access) {
                 ch.changeAcl(pools[pool]->access);
                 allow_t answer = ch.fastCheck();
-                if (answer == ACCESS_ALLOWED) {
+                if (answer.allowed()) {
 
                     /*  request client information from db after we did all checks
                         this will save hash lookup if client failed checks */
@@ -2706,7 +2706,7 @@ httpsSslBumpAccessCheckDone(allow_t answer, void *data)
     if (!connState->isOpen())
         return;
 
-    if (answer == ACCESS_ALLOWED) {
+    if (answer.allowed()) {
         debugs(33, 2, "sslBump action " << Ssl::bumpMode(answer.kind) << "needed for " << connState->clientConnection);
         connState->sslBumpMode = static_cast<Ssl::BumpMode>(answer.kind);
     } else {
@@ -2862,7 +2862,7 @@ void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &cer
                     (ca->alg == Ssl::algSetValidBefore && certProperties.setValidBefore) )
                 continue;
 
-            if (ca->aclList && checklist.fastCheck(ca->aclList) == ACCESS_ALLOWED) {
+            if (ca->aclList && checklist.fastCheck(ca->aclList).allowed()) {
                 const char *alg = Ssl::CertAdaptAlgorithmStr[ca->alg];
                 const char *param = ca->param;
 
@@ -2885,7 +2885,7 @@ void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &cer
 
         certProperties.signAlgorithm = Ssl::algSignEnd;
         for (sslproxy_cert_sign *sg = Config.ssl_client.cert_sign; sg != NULL; sg = sg->next) {
-            if (sg->aclList && checklist.fastCheck(sg->aclList) == ACCESS_ALLOWED) {
+            if (sg->aclList && checklist.fastCheck(sg->aclList).allowed()) {
                 certProperties.signAlgorithm = (Ssl::CertSignAlgorithm)sg->alg;
                 break;
             }
@@ -3170,7 +3170,7 @@ void httpsSslBumpStep2AccessCheckDone(allow_t answer, void *data)
     debugs(33, 5, "Answer: " << answer << " kind:" << answer.kind);
     assert(connState->serverBump());
     Ssl::BumpMode bumpAction;
-    if (answer == ACCESS_ALLOWED) {
+    if (answer.allowed()) {
         bumpAction = (Ssl::BumpMode)answer.kind;
     } else
         bumpAction = Ssl::bumpSplice;

diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc
index 81fc88dfc1ef7eb189bf537c318bc4956df2c49e..875118d22095a42d8e673e7caa8c3a99bfe46b83 100644 (file)
--- a/src/client_side_reply.cc
+++ b/src/client_side_reply.cc
@@ -873,7 +873,7 @@ clientReplyContext::blockedHit() const
         std::unique_ptr<ACLFilledChecklist> chl(clientAclChecklistCreate(Config.accessList.sendHit, http));
         chl->reply = const_cast<HttpReply*>(rep); // ACLChecklist API bug
         HTTPMSGLOCK(chl->reply);
-        return chl->fastCheck() != ACCESS_ALLOWED; // when in doubt, block
+        return !chl->fastCheck().allowed(); // when in doubt, block
     }
 
     // This does not happen, I hope, because we are called from CacheHit, which
@@ -2097,7 +2097,7 @@ clientReplyContext::processReplyAccessResult(const allow_t &accessAllowed)
            << ' ' << http->uri << " is " << accessAllowed << ", because it matched "
            << (AclMatchedName ? AclMatchedName : "NO ACL's"));
 
-    if (accessAllowed != ACCESS_ALLOWED) {
+    if (!accessAllowed.allowed()) {
         ErrorState *err;
         err_type page_id;
         page_id = aclGetDenyInfoPage(&Config.denyInfoList, AclMatchedName, 1);

diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index d4628cbcc2f09cd4bc914b3a79ae1098498f827e..2822d1a7a8ea80bd7a7c94dc7b8f05332278ae7d 100644 (file)
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -449,13 +449,7 @@ clientFollowXForwardedForCheck(allow_t answer, void *data)
     ClientHttpRequest *http = calloutContext->http;
     HttpRequest *request = http->request;
 
-    /*
-     * answer should be be ACCESS_ALLOWED or ACCESS_DENIED if we are
-     * called as a result of ACL checks, or -1 if we are called when
-     * there's nothing left to do.
-     */
-    if (answer == ACCESS_ALLOWED &&
-            request->x_forwarded_for_iterator.size () != 0) {
+    if (answer.allowed() && request->x_forwarded_for_iterator.size() != 0) {
 
         /*
          * Remove the last comma-delimited element from the
@@ -497,8 +491,7 @@ clientFollowXForwardedForCheck(allow_t answer, void *data)
             calloutContext->acl_checklist->nonBlockingCheck(clientFollowXForwardedForCheck, data);
             return;
         }
-    } /*if (answer == ACCESS_ALLOWED &&
-        request->x_forwarded_for_iterator.size () != 0)*/
+    }
 
     /* clean up, and pass control to clientAccessCheck */
     if (Config.onoff.log_uses_indirect_client) {
@@ -513,7 +506,7 @@ clientFollowXForwardedForCheck(allow_t answer, void *data)
     request->x_forwarded_for_iterator.clean();
     request->flags.done_follow_x_forwarded_for = true;
 
-    if (answer != ACCESS_ALLOWED && answer != ACCESS_DENIED) {
+    if (!answer.someRuleMatched()) {
         debugs(28, DBG_CRITICAL, "ERROR: Processing X-Forwarded-For. Stopping at IP address: " << request->indirect_client_addr );
     }
 
@@ -769,7 +762,7 @@ ClientRequestContext::clientAccessCheckDone(const allow_t &answer)
         proxy_auth_msg = http->request->auth_user_request->denyMessage("<null>");
 #endif
 
-    if (answer != ACCESS_ALLOWED) {
+    if (!answer.allowed()) {
         // auth has a grace period where credentials can be expired but okay not to challenge.
 
         /* Send an auth challenge or error */
@@ -880,7 +873,7 @@ clientRedirectAccessCheckDone(allow_t answer, void *data)
     ClientHttpRequest *http = context->http;
     context->acl_checklist = NULL;
 
-    if (answer == ACCESS_ALLOWED)
+    if (answer.allowed())
         redirectStart(http, clientRedirectDoneWrapper, context);
     else {
         Helper::Reply const nilReply(Helper::Error);
@@ -911,7 +904,7 @@ clientStoreIdAccessCheckDone(allow_t answer, void *data)
     ClientHttpRequest *http = context->http;
     context->acl_checklist = NULL;
 
-    if (answer == ACCESS_ALLOWED)
+    if (answer.allowed())
         storeIdStart(http, clientStoreIdDoneWrapper, context);
     else {
         debugs(85, 3, "access denied expected ERR reply handling: " << answer);
@@ -1397,7 +1390,7 @@ void
 ClientRequestContext::checkNoCacheDone(const allow_t &answer)
 {
     acl_checklist = NULL;
-    if (answer == ACCESS_DENIED) {
+    if (answer.denied()) {
         http->request->flags.noCache = true; // dont read reply from cache
         http->request->flags.cachable = false; // dont store reply into cache
     }
@@ -1496,7 +1489,7 @@ ClientRequestContext::sslBumpAccessCheckDone(const allow_t &answer)
     if (!httpStateIsValid())
         return;
 
-    const Ssl::BumpMode bumpMode = answer == ACCESS_ALLOWED ?
+    const Ssl::BumpMode bumpMode = answer.allowed() ?
                                    static_cast<Ssl::BumpMode>(answer.kind) : Ssl::bumpSplice;
     http->sslBumpNeed(bumpMode); // for processRequest() to bump if needed
     http->al->ssl.bumpMode = bumpMode; // for logging

diff --git a/src/clients/Client.cc b/src/clients/Client.cc
index 24a3b9bf33e958ada08a7258979fd30b9e829013..6448b35223145bc89f1a0237cade5b162355040e 100644 (file)
--- a/src/clients/Client.cc
+++ b/src/clients/Client.cc
@@ -522,7 +522,7 @@ Client::blockCaching()
         ACLFilledChecklist ch(acl, originalRequest().getRaw());
         ch.reply = const_cast<HttpReply*>(entry->getReply()); // ACLFilledChecklist API bug
         HTTPMSGLOCK(ch.reply);
-        if (ch.fastCheck() != ACCESS_ALLOWED) { // when in doubt, block
+        if (!ch.fastCheck().allowed()) { // when in doubt, block
             debugs(20, 3, "store_miss prohibits caching");
             return true;
         }

diff --git a/src/clients/FtpClient.cc b/src/clients/FtpClient.cc
index aa8e1ca5428ca66466b95c26f4818b5d0cd2db5a..2455dde99de04cbcff2d01e5b0841d44035bd1c3 100644 (file)
--- a/src/clients/FtpClient.cc
+++ b/src/clients/FtpClient.cc
@@ -705,7 +705,7 @@ Ftp::Client::sendPassive()
         bool doEpsv = true;
         if (Config.accessList.ftp_epsv) {
             ACLFilledChecklist checklist(Config.accessList.ftp_epsv, fwd->request, NULL);
-            doEpsv = (checklist.fastCheck() == ACCESS_ALLOWED);
+            doEpsv = checklist.fastCheck().allowed();
         }
         if (!doEpsv) {
             debugs(9, 5, "EPSV support manually disabled. Sending PASV for FTP Channel (" << ctrl.conn->remote <<")");

diff --git a/src/external_acl.cc b/src/external_acl.cc
index f560af5c7496641eb03b7b9801bc3f69ec914f7e..a7c791b98e87e598cf01ad7a67b7cbf1aea70f50 100644 (file)
--- a/src/external_acl.cc
+++ b/src/external_acl.cc
@@ -456,7 +456,7 @@ external_acl::maybeCacheable(const allow_t &result) const
     if (result == ACCESS_DUNNO)
         return false; // non-cacheable response
 
-    if ((result == ACCESS_ALLOWED ? ttl : negative_ttl) <= 0)
+    if ((result.allowed() ? ttl : negative_ttl) <= 0)
         return false; // not caching this type of response
 
     return true;
@@ -615,7 +615,7 @@ aclMatchExternal(external_acl_data *acl, ACLFilledChecklist *ch)
             /* Make sure the user is authenticated */
             debugs(82, 3, HERE << acl->def->name << " check user authenticated.");
             const allow_t ti = AuthenticateAcl(ch);
-            if (ti != ACCESS_ALLOWED) {
+            if (!ti.allowed()) {
                 debugs(82, 2, HERE << acl->def->name << " user not authenticated (" << ti << ")");
                 return ti;
             }
@@ -802,7 +802,7 @@ external_acl_entry_expired(external_acl * def, const ExternalACLEntryPointer &en
     if (def->cache_size <= 0 || entry->result == ACCESS_DUNNO)
         return 1;
 
-    if (entry->date + (entry->result == ACCESS_ALLOWED ? def->ttl : def->negative_ttl) < squid_curtime)
+    if (entry->date + (entry->result.allowed() ? def->ttl : def->negative_ttl) < squid_curtime)
         return 1;
     else
         return 0;
@@ -815,7 +815,7 @@ external_acl_grace_expired(external_acl * def, const ExternalACLEntryPointer &en
         return 1;
 
     int ttl;
-    ttl = entry->result == ACCESS_ALLOWED ? def->ttl : def->negative_ttl;
+    ttl = entry->result.allowed() ? def->ttl : def->negative_ttl;
     ttl = (ttl * (100 - def->grace)) / 100;
 
     if (entry->date + ttl <= squid_curtime)

diff --git a/src/htcp.cc b/src/htcp.cc
index 7ebc44fcaf5942fdf6e0af7c1ed184bb74b1cc38..d199c0b7ae4804ffec05776791247ec4cc76b6df 100644 (file)
--- a/src/htcp.cc
+++ b/src/htcp.cc
@@ -775,7 +775,7 @@ htcpAccessAllowed(acl_access * acl, const htcpSpecifier::Pointer &s, Ip::Address
     ACLFilledChecklist checklist(acl, s->request.getRaw(), nullptr);
     checklist.src_addr = from;
     checklist.my_addr.setNoAddr();
-    return (checklist.fastCheck() == ACCESS_ALLOWED);
+    return checklist.fastCheck().allowed();
 }
 
 static void

diff --git a/src/http.cc b/src/http.cc
index 1b0605fbc696914fee4b3e7c14cee936cf6830fd..e8b5fbeda2b223987d935fe04e2c9b36ed4ce113 100644 (file)
--- a/src/http.cc
+++ b/src/http.cc
@@ -807,7 +807,7 @@ HttpStateData::handle1xx(HttpReply *reply)
         ACLFilledChecklist ch(Config.accessList.reply, originalRequest().getRaw());
         ch.reply = reply;
         HTTPMSGLOCK(ch.reply);
-        if (ch.fastCheck() != ACCESS_ALLOWED) { // TODO: support slow lookups?
+        if (!ch.fastCheck().allowed()) { // TODO: support slow lookups?
             debugs(11, 3, HERE << "ignoring denied 1xx");
             proceedAfter1xx();
             return;
@@ -2318,7 +2318,7 @@ HttpStateData::finishingBrokenPost()
     }
 
     ACLFilledChecklist ch(Config.accessList.brokenPosts, originalRequest().getRaw());
-    if (ch.fastCheck() != ACCESS_ALLOWED) {
+    if (!ch.fastCheck().allowed()) {
         debugs(11, 5, HERE << "didn't match brokenPosts");
         return false;
     }

diff --git a/src/http/Stream.cc b/src/http/Stream.cc
index 3c13c868fa167d0ac87735fa3d1ac14cfa162d5e..b4c8dc55641203b4fe8cc4318b4d51b318a71cd9 100644 (file)
--- a/src/http/Stream.cc
+++ b/src/http/Stream.cc
@@ -296,7 +296,7 @@ Http::Stream::sendStartOfMessage(HttpReply *rep, StoreIOBuffer bodyData)
             chl->reply = rep;
             HTTPMSGLOCK(chl->reply);
             const allow_t answer = chl->fastCheck();
-            if (answer == ACCESS_ALLOWED) {
+            if (answer.allowed()) {
                 writeQuotaHandler = pool->createBucket();
                 fd_table[clientConnection->fd].writeQuotaHandler = writeQuotaHandler;
                 break;

diff --git a/src/icp_v2.cc b/src/icp_v2.cc
index d9ed9f0df27647d3571ca62e84d8fe7af1bffdac..4cb551eb587240e194be7c5a3fe31eaf1bb9895f 100644 (file)
--- a/src/icp_v2.cc
+++ b/src/icp_v2.cc
@@ -417,7 +417,7 @@ icpAccessAllowed(Ip::Address &from, HttpRequest * icp_request)
     ACLFilledChecklist checklist(Config.accessList.icp, icp_request, NULL);
     checklist.src_addr = from;
     checklist.my_addr.setNoAddr();
-    return (checklist.fastCheck() == ACCESS_ALLOWED);
+    return checklist.fastCheck().allowed();
 }
 
 char const *

diff --git a/src/log/access_log.cc b/src/log/access_log.cc
index ee16f6a65b98e01de361e251ab65767cd250b851..0bb1b063e7aa0325a1e0c0accaf9d6aae1e2515f 100644 (file)
--- a/src/log/access_log.cc
+++ b/src/log/access_log.cc
@@ -84,7 +84,7 @@ accessLogLogTo(CustomLog* log, AccessLogEntry::Pointer &al, ACLChecklist * check
         xstrncpy(al->hier.host, dash_str, SQUIDHOSTNAMELEN);
 
     for (; log; log = log->next) {
-        if (log->aclList && checklist && checklist->fastCheck(log->aclList) != ACCESS_ALLOWED)
+        if (log->aclList && checklist && !checklist->fastCheck(log->aclList).allowed())
             continue;
 
         // The special-case "none" type has no logfile object set

diff --git a/src/neighbors.cc b/src/neighbors.cc
index 2087b688c18dcc78d3c65a117332f9b96ddb59d0..65ae2ef59077b934702b38a395cc174665a8bebd 100644 (file)
--- a/src/neighbors.cc
+++ b/src/neighbors.cc
@@ -168,7 +168,7 @@ peerAllowedToUse(const CachePeer * p, HttpRequest * request)
 
     ACLFilledChecklist checklist(p->access, request, NULL);
 
-    return (checklist.fastCheck() == ACCESS_ALLOWED);
+    return checklist.fastCheck().allowed();
 }
 
 /* Return TRUE if it is okay to send an ICP request to this CachePeer.   */

diff --git a/src/security/PeerConnector.cc b/src/security/PeerConnector.cc
index 4b3b6ceaca519989246bc765f35cdba232174a62..0baa345a68e2df991f06a182162c4d2a56c9073e 100644 (file)
--- a/src/security/PeerConnector.cc
+++ b/src/security/PeerConnector.cc
@@ -338,7 +338,7 @@ Security::PeerConnector::sslCrtvdCheckForErrors(Ssl::CertValidationResponse cons
             bool allowed = false;
             if (check) {
                 check->sslErrors = new Security::CertErrors(Security::CertError(i->error_no, i->cert, i->error_depth));
-                if (check->fastCheck() == ACCESS_ALLOWED)
+                if (check->fastCheck().allowed())
                     allowed = true;
             }
             // else the Config.ssl_client.cert_error access list is not defined

diff --git a/src/servers/FtpServer.cc b/src/servers/FtpServer.cc
index ca9f71d6153ef467da7e7cd048d0b9cfec142076..5968695c1caecadc125c0623c42a49081a496478 100644 (file)
--- a/src/servers/FtpServer.cc
+++ b/src/servers/FtpServer.cc
@@ -1547,7 +1547,7 @@ Ftp::Server::handleUploadRequest(String &, String &)
         ClientHttpRequest *http = pipeline.front()->http;
         HttpRequest *request = http->request;
         ACLFilledChecklist bodyContinuationCheck(Config.accessList.forceRequestBodyContinuation, request, NULL);
-        if (bodyContinuationCheck.fastCheck() == ACCESS_ALLOWED) {
+        if (bodyContinuationCheck.fastCheck().allowed()) {
             request->forcedBodyContinuation = true;
             if (checkDataConnPost()) {
                 // Write control Msg

diff --git a/src/servers/Http1Server.cc b/src/servers/Http1Server.cc
index 2cbfbe0f498a7acdd1403772367d11e564eb473d..9f808b0086d7aa6764e63b017db2211db876f7cf 100644 (file)
--- a/src/servers/Http1Server.cc
+++ b/src/servers/Http1Server.cc
@@ -257,7 +257,7 @@ Http::One::Server::processParsedRequest(Http::StreamPointer &context)
 
         if (Config.accessList.forceRequestBodyContinuation) {
             ACLFilledChecklist bodyContinuationCheck(Config.accessList.forceRequestBodyContinuation, request.getRaw(), NULL);
-            if (bodyContinuationCheck.fastCheck() == ACCESS_ALLOWED) {
+            if (bodyContinuationCheck.fastCheck().allowed()) {
                 debugs(33, 5, "Body Continuation forced");
                 request->forcedBodyContinuation = true;
                 //sendControlMsg

diff --git a/src/snmp_core.cc b/src/snmp_core.cc
index e931b55f83c7b621557bd4802a3d3479ec654e52..b6721a48939c3ad2afd92f458068393150e7f318 100644 (file)
--- a/src/snmp_core.cc
+++ b/src/snmp_core.cc
@@ -383,7 +383,6 @@ snmpDecodePacket(SnmpRequest * rq)
     u_char *Community;
     u_char *buf = rq->buf;
     int len = rq->len;
-    allow_t allow = ACCESS_DENIED;
 
     if (!Config.accessList.snmp) {
         debugs(49, DBG_IMPORTANT, "WARNING: snmp_access not configured. agent query DENIED from : " << rq->from);
@@ -402,9 +401,8 @@ snmpDecodePacket(SnmpRequest * rq)
         ACLFilledChecklist checklist(Config.accessList.snmp, NULL, NULL);
         checklist.src_addr = rq->from;
         checklist.snmp_community = (char *) Community;
-        allow = checklist.fastCheck();
 
-        if (allow == ACCESS_ALLOWED && (snmp_coexist_V2toV1(PDU))) {
+        if (checklist.fastCheck().allowed() && (snmp_coexist_V2toV1(PDU))) {
             rq->community = Community;
             rq->PDU = PDU;
             debugs(49, 5, "snmpAgentParse: reqid=[" << PDU->reqid << "]");

diff --git a/src/ssl/PeekingPeerConnector.cc b/src/ssl/PeekingPeerConnector.cc
index 1fbffd56a92a73a2aa66764d21abd0e03833d35a..917b4bfc1ca7833dfb150e1f644da9c3f5020b1a 100644 (file)
--- a/src/ssl/PeekingPeerConnector.cc
+++ b/src/ssl/PeekingPeerConnector.cc
@@ -36,7 +36,7 @@ Ssl::PeekingPeerConnector::cbCheckForPeekAndSpliceDone(allow_t answer, void *dat
 void
 Ssl::PeekingPeerConnector::checkForPeekAndSpliceDone(allow_t answer)
 {
-    const Ssl::BumpMode finalAction = (answer.code == ACCESS_ALLOWED) ?
+    const Ssl::BumpMode finalAction = answer.allowed() ?
                                       static_cast<Ssl::BumpMode>(answer.kind):
                                       checkForPeekAndSpliceGuess();
     checkForPeekAndSpliceMatched(finalAction);

diff --git a/src/ssl/support.cc b/src/ssl/support.cc
index 3bd7bcb3f3e9edb36739a18c2e51a3ade4b0a518..f90812715bed01354a0bdeb61a79609cee8c87f2 100644 (file)
--- a/src/ssl/support.cc
+++ b/src/ssl/support.cc
@@ -329,7 +329,7 @@ ssl_verify_cb(int ok, X509_STORE_CTX * ctx)
                 assert(!filledCheck->sslErrors);
                 filledCheck->sslErrors = new Security::CertErrors(Security::CertError(error_no, broken_cert));
                 filledCheck->serverCert = peer_cert;
-                if (check->fastCheck() == ACCESS_ALLOWED) {
+                if (check->fastCheck().allowed()) {
                     debugs(83, 3, "bypassing SSL error " << error_no << " in " << buffer);
                     ok = 1;
                 } else {

diff --git a/src/tunnel.cc b/src/tunnel.cc
index 42001ea784be08f8841032bcbdf7f81470476212..bb899a49dd8130f0276b418d0d570993d9dcc42e 100644 (file)
--- a/src/tunnel.cc
+++ b/src/tunnel.cc
@@ -1097,7 +1097,7 @@ tunnelStart(ClientHttpRequest * http)
         ACLFilledChecklist ch(Config.accessList.miss, request, NULL);
         ch.src_addr = request->client_addr;
         ch.my_addr = request->my_addr;
-        if (ch.fastCheck() == ACCESS_DENIED) {
+        if (ch.fastCheck().denied()) {
             debugs(26, 4, HERE << "MISS access forbidden.");
             err = new ErrorState(ERR_FORWARDING_DENIED, Http::scForbidden, request);
             http->al->http.code = Http::scForbidden;