From: Alex Rousskov Date: Fri, 30 Jun 2017 06:37:58 +0000 (+1200) Subject: Minimize direct comparisons with ACCESS_ALLOWED and ACCESS_DENIED. X-Git-Tag: SQUID_4_0_21~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4745827a5ee16f6d0ed9c3ea8b194340b37cace3;p=thirdparty%2Fsquid.git 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 aa54fe4e66..50321ce045 100644 --- 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 301a220ff6..0808249023 100644 --- a/src/FwdState.cc +++ b/src/FwdState.cc @@ -324,7 +324,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); @@ -1180,7 +1180,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. @@ -1232,7 +1232,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; } @@ -1244,7 +1244,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; } @@ -1295,7 +1295,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 fba8a45268..7c734e0631 100644 --- 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; @@ -478,7 +478,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 0550d8ed99..1043096a6b 100644 --- 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 7b648f9795..bf2d5d2ec2 100644 --- 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; @@ -702,7 +702,7 @@ HttpRequest::manager(const CbcPointer &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 4cc9130985..c2f37ad729 100644 --- a/src/Notes.cc +++ b/src/Notes.cc @@ -47,10 +47,10 @@ Note::match(HttpRequest *request, HttpReply *reply, const AccessLogEntry::Pointe HTTPMSGLOCK(ch.reply); for (VLI i = values.begin(); i != values.end(); ++i ) { - const int ret= ch.fastCheck((*i)->aclList); + const auto ret= ch.fastCheck((*i)->aclList); debugs(93, 5, HERE << "Check for header name: " << key << ": " << (*i)->value <<", HttpRequest: " << request << " HttpReply: " << reply << " matched: " << ret); - if (ret == ACCESS_ALLOWED) { + if (ret.allowed()) { if (al != NULL && (*i)->valueFormat != NULL) { static MemBuf mb; mb.reset(); diff --git a/src/acl/Acl.h b/src/acl/Acl.h index 9ed1b19c30..57300d6c87 100644 --- a/src/acl/Acl.h +++ b/src/acl/Acl.h @@ -16,6 +16,7 @@ #include "dlink.h" #include "sbuf/forward.h" +#include #include 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 d243354cc3..bb2fdc4983 100644 --- 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 diff --git a/src/adaptation/AccessCheck.cc b/src/adaptation/AccessCheck.cc index 6ce6d91975..97242dbe6a 100644 --- 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 a1cdde35e1..da4ab8d401 100644 --- a/src/adaptation/icap/Launcher.cc +++ b/src/adaptation/icap/Launcher.cc @@ -147,7 +147,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/client_side.cc b/src/client_side.cc index 108955eb66..758cfa30b0 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -461,7 +461,7 @@ ClientHttpRequest::logRequest() statsCheck.reply = al->reply; HTTPMSGLOCK(statsCheck.reply); } - updatePerformanceCounters = (statsCheck.fastCheck() == ACCESS_ALLOWED); + updatePerformanceCounters = statsCheck.fastCheck().allowed(); } if (updatePerformanceCounters) { @@ -1526,7 +1526,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; } @@ -1580,7 +1580,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 @@ -1825,7 +1825,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; @@ -2445,7 +2445,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 @@ -2473,7 +2473,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 */ @@ -2705,7 +2705,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(answer.kind); } else { @@ -2861,7 +2861,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; @@ -2884,7 +2884,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; } @@ -3169,7 +3169,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 b1810ed49c..ff9b5b0dc6 100644 --- a/src/client_side_reply.cc +++ b/src/client_side_reply.cc @@ -873,7 +873,7 @@ clientReplyContext::blockedHit() const std::unique_ptr chl(clientAclChecklistCreate(Config.accessList.sendHit, http)); chl->reply = const_cast(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 @@ -2096,7 +2096,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 4b3140fe50..388407d085 100644 --- 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(""); #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(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 807ae3b3b3..ab32dad78f 100644 --- a/src/clients/Client.cc +++ b/src/clients/Client.cc @@ -539,7 +539,7 @@ Client::blockCaching() ACLFilledChecklist ch(acl, originalRequest(), NULL); ch.reply = const_cast(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 aa8e1ca542..2455dde99d 100644 --- 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 f560af5c74..a7c791b98e 100644 --- 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 7ebc44fcaf..d199c0b7ae 100644 --- 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 eb17139b0d..09bbf4e620 100644 --- a/src/http.cc +++ b/src/http.cc @@ -807,7 +807,7 @@ HttpStateData::handle1xx(HttpReply *reply) ACLFilledChecklist ch(Config.accessList.reply, originalRequest(), NULL); 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(), NULL); - if (ch.fastCheck() != ACCESS_ALLOWED) { + if (!ch.fastCheck().allowed()) { debugs(11, 5, HERE << "didn't match brokenPosts"); return false; } diff --git a/src/icp_v2.cc b/src/icp_v2.cc index d9ed9f0df2..4cb551eb58 100644 --- 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 ee16f6a65b..0bb1b063e7 100644 --- 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 ce1bc0f8ed..c6cdb1adbb 100644 --- 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 15e5d7c784..4aec20c415 100644 --- 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 ca9f71d615..5968695c1c 100644 --- 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 b9a72aa6bb..d92a48af81 100644 --- a/src/servers/Http1Server.cc +++ b/src/servers/Http1Server.cc @@ -250,7 +250,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 4965278c72..e6ee612a31 100644 --- 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 1fbffd56a9..917b4bfc1c 100644 --- 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(answer.kind): checkForPeekAndSpliceGuess(); checkForPeekAndSpliceMatched(finalAction); diff --git a/src/ssl/support.cc b/src/ssl/support.cc index 3bd7bcb3f3..f90812715b 100644 --- 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 c3f9fc3210..ec023ac731 100644 --- a/src/tunnel.cc +++ b/src/tunnel.cc @@ -1084,7 +1084,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;