------------------------------------------------------------ revno: 14134 revision-id: squid3@treenet.co.nz-20170127022604-1j0lnbc7lu1839z0 parent: squid3@treenet.co.nz-20170124201432-vskcudplmy2elpib author: Christos Tsantilas committer: Amos Jeffries branch nick: 3.5 timestamp: Fri 2017-01-27 15:26:04 +1300 message: Update External ACL helpers error handling and caching The helper protocol for external ACLs [1] defines three possible return values: OK - Success. ACL test matches. ERR - Success. ACL test fails to match. BH - Failure. The helper encountered a problem. The external acl helpers distributed with squid currently do not follow this definition. For example, upon connection error, ERR is returned: $ ext_ldap_group_acl ... -d ext_ldap_group_acl: WARNING: could not bind to binddn 'Can't contact LDAP server' ERR This does not allow to distinguish "no match" and "error" either and therefore negative caches "ERR", also in the case of an error. Moreover there are multiple problems inside squid when trying to handle BH responses: - Squid-5 and Squid-4 retry requests for BH responses but crashes after the maximum retry number (currently 2) is reached. - If an external acl helper return always BH (eg because the LDAP server is down) squid sends infinitely new request to the helper. This is a Measurement Factory project ------------------------------------------------------------ # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: squid3@treenet.co.nz-20170127022604-1j0lnbc7lu1839z0 # target_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 # testament_sha1: 64af3fea6c6a855a88a93069e8243344ecb9f39d # timestamp: 2017-01-27 02:51:14 +0000 # source_branch: http://bzr.squid-cache.org/bzr/squid3/3.5 # base_revision_id: squid3@treenet.co.nz-20170124201432-\ # vskcudplmy2elpib # # Begin patch === modified file 'helpers/defines.h' --- helpers/defines.h 2017-01-01 00:16:45 +0000 +++ helpers/defines.h 2017-01-27 02:26:04 +0000 @@ -51,9 +51,12 @@ /* send ERR result to Squid with a string parameter. */ #define SEND_ERR(x) fprintf(stdout, "ERR %s\n",x) -/* send ERR result to Squid with a string parameter. */ +/* send BH result to Squid with a string parameter. */ #define SEND_BH(x) fprintf(stdout, "BH %s\n",x) +/* constructs a message to Squid. */ +#define HLP_MSG(text) "message=\"" text "\"" + /* send TT result to Squid with a string parameter. */ #define SEND_TT(x) fprintf(stdout, "TT %s\n",x) === modified file 'helpers/external_acl/AD_group/ext_ad_group_acl.cc' --- helpers/external_acl/AD_group/ext_ad_group_acl.cc 2017-01-01 00:16:45 +0000 +++ helpers/external_acl/AD_group/ext_ad_group_acl.cc 2017-01-27 02:26:04 +0000 @@ -819,7 +819,7 @@ if (strchr(buf, '\n') != NULL) break; } - SEND_ERR("Invalid Request. Too Long."); + SEND_BH(HLP_MSG("Invalid Request. Too Long.")); continue; } if ((p = strchr(buf, '\n')) != NULL) @@ -830,7 +830,7 @@ debug("Got '%s' from Squid (length: %d).\n", buf, strlen(buf)); if (buf[0] == '\0') { - SEND_ERR("Invalid Request. No Input."); + SEND_BH(HLP_MSG("Invalid Request. No Input.")); continue; } username = strtok(buf, " "); @@ -842,7 +842,7 @@ numberofgroups = n; if (NULL == username) { - SEND_ERR("Invalid Request. No Username."); + SEND_BH(HLP_MSG("Invalid Request. No Username.")); continue; } rfc1738_unescape(username); === modified file 'helpers/external_acl/LDAP_group/ext_ldap_group_acl.cc' --- helpers/external_acl/LDAP_group/ext_ldap_group_acl.cc 2017-01-01 00:16:45 +0000 +++ helpers/external_acl/LDAP_group/ext_ldap_group_acl.cc 2017-01-27 02:26:04 +0000 @@ -466,13 +466,13 @@ if (strchr(buf, '\n') != NULL) break; } - SEND_ERR(""); + SEND_BH(HLP_MSG("Input too large")); continue; } user = strtok(buf, " \n"); if (!user) { debug("%s: Invalid request: No Username given\n", argv[0]); - SEND_ERR("Invalid request. No Username"); + SEND_BH(HLP_MSG("Invalid request. No Username")); continue; } rfc1738_unescape(user); @@ -495,11 +495,12 @@ extension_dn = strtok(NULL, " \n"); if (!extension_dn) { debug("%s: Invalid request: Extension DN configured, but none sent.\n", argv[0]); - SEND_ERR("Invalid Request. Extension DN required."); + SEND_BH(HLP_MSG("Invalid Request. Extension DN required")); continue; } rfc1738_unescape(extension_dn); } + const char *broken = nullptr; while (!found && user && (group = strtok(NULL, " \n")) != NULL) { rfc1738_unescape(group); @@ -509,6 +510,7 @@ if (strstr(ldapServer, "://") != NULL) { rc = ldap_initialize(&ld, ldapServer); if (rc != LDAP_SUCCESS) { + broken = HLP_MSG("Unable to connect to LDAP server"); fprintf(stderr, "%s: ERROR: Unable to connect to LDAPURI:%s\n", argv[0], ldapServer); break; } @@ -530,7 +532,8 @@ } else #endif if ((ld = ldap_init(ldapServer, port)) == NULL) { - fprintf(stderr, "ERROR: Unable to connect to LDAP server:%s port:%d\n", ldapServer, port); + broken = HLP_MSG("Unable to connect to LDAP server"); + fprintf(stderr, "ERROR: %s:%s port:%d\n", broken, ldapServer, port); break; } if (connect_timeout) @@ -541,8 +544,8 @@ version = LDAP_VERSION3; } if (ldap_set_option(ld, LDAP_OPT_PROTOCOL_VERSION, &version) != LDAP_SUCCESS) { - fprintf(stderr, "ERROR: Could not set LDAP_OPT_PROTOCOL_VERSION %d\n", - version); + broken = HLP_MSG("Could not set LDAP_OPT_PROTOCOL_VERSION"); + fprintf(stderr, "ERROR: %s %d\n", broken, version); ldap_unbind(ld); ld = NULL; break; @@ -553,7 +556,8 @@ fprintf(stderr, "FATAL: TLS requires LDAP version 3\n"); exit(1); } else if (ldap_start_tls_s(ld, NULL, NULL) != LDAP_SUCCESS) { - fprintf(stderr, "ERROR: Could not Activate TLS connection\n"); + broken = HLP_MSG("Could not Activate TLS connection"); + fprintf(stderr, "ERROR: %s\n", broken); ldap_unbind(ld); ld = NULL; break; @@ -570,7 +574,8 @@ if (binddn && bindpasswd && *binddn && *bindpasswd) { rc = ldap_simple_bind_s(ld, binddn, bindpasswd); if (rc != LDAP_SUCCESS) { - fprintf(stderr, PROGRAM_NAME ": WARNING: could not bind to binddn '%s'\n", ldap_err2string(rc)); + broken = HLP_MSG("could not bind"); + fprintf(stderr, PROGRAM_NAME ": WARNING: %s to binddn '%s'\n", broken, ldap_err2string(rc)); ldap_unbind(ld); ld = NULL; break; @@ -578,20 +583,24 @@ } debug("Connected OK\n"); } - if (searchLDAP(ld, group, user, extension_dn) == 0) { + int searchResult = searchLDAP(ld, group, user, extension_dn); + if (searchResult == 0) { found = 1; break; - } else { + } else if (searchResult < 0){ if (tryagain) { tryagain = 0; ldap_unbind(ld); ld = NULL; goto recover; } + broken = HLP_MSG("LDAP search error"); } } if (found) SEND_OK(""); + else if (broken) + SEND_BH(broken); else { SEND_ERR(""); } @@ -713,7 +722,7 @@ if (build_filter(filter, sizeof(filter), searchfilter, member, group) != 0) { fprintf(stderr, PROGRAM_NAME ": ERROR: Failed to construct LDAP search filter. filter=\"%s\", user=\"%s\", group=\"%s\"\n", filter, member, group); - return 1; + return -1; } debug("group filter '%s', searchbase '%s'\n", filter, searchbase); @@ -732,7 +741,7 @@ } #endif ldap_msgfree(res); - return 1; + return -1; } } entry = ldap_first_entry(ld, res); @@ -779,7 +788,7 @@ } #endif ldap_msgfree(res); - return 1; + return -1; } } entry = ldap_first_entry(ld, res); === modified file 'helpers/external_acl/LM_group/ext_lm_group_acl.cc' --- helpers/external_acl/LM_group/ext_lm_group_acl.cc 2017-01-01 00:16:45 +0000 +++ helpers/external_acl/LM_group/ext_lm_group_acl.cc 2017-01-27 02:26:04 +0000 @@ -561,7 +561,7 @@ if (strchr(buf, '\n') != NULL) break; } - SEND_ERR("Input Too Long."); + SEND_BH(HLP_MSG("Input Too Long.")); continue; } if ((p = strchr(buf, '\n')) != NULL) @@ -572,7 +572,7 @@ debug("Got '%s' from Squid (length: %d).\n", buf, strlen(buf)); if (buf[0] == '\0') { - SEND_ERR("Invalid Request."); + SEND_BH(HLP_MSG("Invalid Request.")); continue; } username = strtok(buf, " "); @@ -583,7 +583,7 @@ groups[n] = NULL; if (NULL == username) { - SEND_ERR("Invalid Request. No Username."); + SEND_BH(HLP_MSG("Invalid Request. No Username.")); continue; } rfc1738_unescape(username); === modified file 'helpers/external_acl/SQL_session/ext_sql_session_acl.pl.in' --- helpers/external_acl/SQL_session/ext_sql_session_acl.pl.in 2017-01-01 00:16:45 +0000 +++ helpers/external_acl/SQL_session/ext_sql_session_acl.pl.in 2017-01-27 02:26:04 +0000 @@ -195,7 +195,7 @@ print(stderr "Received: Channel=".$cid.", UID='".$uid."'\n") if ($debug); - $status = $cid . " ERR message=\"database error\""; + $status = $cid . " BH message=\"database error\""; my $sth = query_db($uid) || next; print(stderr "Rows: ". $sth->rows()."\n") if ($debug); $status = $cid . " ERR message=\"unknown UID '".$uid."'\""; === modified file 'helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc' --- helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc 2017-01-01 00:16:45 +0000 +++ helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc 2017-01-27 02:26:04 +0000 @@ -1758,7 +1758,7 @@ /* No space given, but group string is required --> ERR */ if ((edui_conf.mode & EDUI_MODE_GROUP) && (p == NULL)) { debug("while() -> Search group is missing. (required)\n"); - local_printfx("ERR message=\"(Search Group Required)\"\n"); + local_printfx("BH message=\"(Search Group Required)\"\n"); continue; } x = 0; @@ -1801,7 +1801,7 @@ if (x != LDAP_ERR_SUCCESS) { /* Unable to bind */ debug("BindLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); - local_printfx("ERR message=\"(BindLDAP: %s - %s)\"\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); + local_printfx("BH message=\"(BindLDAP: %s - %s)\"\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); continue; } else debug("BindLDAP(-, %s, %s, (LDAP_AUTH_TLS)) -> %s\n", edui_conf.dn, edui_conf.passwd, ErrLDAP(x)); @@ -1812,7 +1812,7 @@ if (x != LDAP_ERR_SUCCESS) { /* Unable to bind */ debug("BindLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); - local_printfx("ERR message=\"(BindLDAP: %s - %s)\"\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); + local_printfx("BH message=\"(BindLDAP: %s - %s)\"\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); continue; } else debug("BindLDAP(-, %s, %s, (LDAP_AUTH_SIMPLE)) -> %s\n", edui_conf.dn, edui_conf.passwd, ErrLDAP(x)); @@ -1822,7 +1822,7 @@ if (x != LDAP_ERR_SUCCESS) { /* Unable to bind */ debug("BindLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); - local_printfx("ERR message=\"(BindLDAP: %s - %s)\"\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); + local_printfx("BH message=\"(BindLDAP: %s - %s)\"\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); continue; } else debug("BindLDAP(-, -, -, (LDAP_AUTH_NONE)) -> %s\n", ErrLDAP(x)); @@ -1841,7 +1841,7 @@ /* Everything failed --> ERR */ debug("while() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); CloseLDAP(&edui_ldap); - local_printfx("ERR message=\"(General Failure: %s)\"\n", ErrLDAP(x)); + local_printfx("BH message=\"(General Failure: %s)\"\n", ErrLDAP(x)); continue; } edui_ldap.err = -1; @@ -1856,14 +1856,14 @@ x = ConvertIP(&edui_ldap, bufb); if (x < 0) { debug("ConvertIP() -> %s\n", ErrLDAP(x)); - local_printfx("ERR message=\"(ConvertIP: %s)\"\n", ErrLDAP(x)); + local_printfx("BH message=\"(ConvertIP: %s)\"\n", ErrLDAP(x)); } else { edui_ldap.err = -1; debug("ConvertIP(-, %s) -> Result[%d]: %s\n", bufb, x, edui_ldap.search_ip); x = SearchFilterLDAP(&edui_ldap, bufa); if (x < 0) { debug("SearchFilterLDAP() -> %s\n", ErrLDAP(x)); - local_printfx("ERR message=\"(SearchFilterLDAP: %s)\"\n", ErrLDAP(x)); + local_printfx("BH message=\"(SearchFilterLDAP: %s)\"\n", ErrLDAP(x)); } else { /* Do Search */ edui_ldap.err = -1; @@ -1871,17 +1871,20 @@ x = SearchLDAP(&edui_ldap, edui_ldap.scope, edui_ldap.search_filter, (char **) &search_attrib); if (x != LDAP_ERR_SUCCESS) { debug("SearchLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); - local_printfx("ERR message=\"(SearchLDAP: %s)\"\n", ErrLDAP(x)); + local_printfx("BH message=\"(SearchLDAP: %s)\"\n", ErrLDAP(x)); } else { edui_ldap.err = -1; debug("SearchLDAP(-, %d, %s, -) -> %s\n", edui_conf.scope, edui_ldap.search_filter, ErrLDAP(x)); x = SearchIPLDAP(&edui_ldap); - if (x != LDAP_ERR_SUCCESS) { - debug("SearchIPLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); + if (x == LDAP_ERR_NOTFOUND) { + debug("SearchIPLDAP() -> %s\n", ErrLDAP(x)); local_printfx("ERR message=\"(SearchIPLDAP: %s)\"\n", ErrLDAP(x)); - } else { + } else if (x == LDAP_ERR_SUCCESS) { debug("SearchIPLDAP(-, %s) -> %s\n", edui_ldap.userid, ErrLDAP(x)); local_printfx("OK user=%s\n", edui_ldap.userid); /* Got userid --> OK user= */ + } else { + debug("SearchIPLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); + local_printfx("BH message=\"(SearchIPLDAP: %s)\"\n", ErrLDAP(x)); } } /* Clear for next query */ @@ -1890,38 +1893,41 @@ } } else { debug("StringSplit() -> Error: %" PRIuSIZE "\n", i); - local_printfx("ERR message=\"(StringSplit Error %" PRIuSIZE ")\"\n", i); + local_printfx("BH message=\"(StringSplit Error %" PRIuSIZE ")\"\n", i); } } else { /* No group to match against, only an IP */ x = ConvertIP(&edui_ldap, bufa); if (x < 0) { debug("ConvertIP() -> %s\n", ErrLDAP(x)); - local_printfx("ERR message=\"(ConvertIP: %s)\"\n", ErrLDAP(x)); + local_printfx("BH message=\"(ConvertIP: %s)\"\n", ErrLDAP(x)); } else { debug("ConvertIP(-, %s) -> Result[%d]: %s\n", bufa, x, edui_ldap.search_ip); /* Do search */ x = SearchFilterLDAP(&edui_ldap, NULL); if (x < 0) { debug("SearchFilterLDAP() -> %s\n", ErrLDAP(x)); - local_printfx("ERR message=\"(SearchFilterLDAP: %s)\"\n", ErrLDAP(x)); + local_printfx("BH message=\"(SearchFilterLDAP: %s)\"\n", ErrLDAP(x)); } else { edui_ldap.err = -1; debug("SearchFilterLDAP(-, NULL) -> Length: %u\n", x); x = SearchLDAP(&edui_ldap, edui_ldap.scope, edui_ldap.search_filter, (char **) &search_attrib); if (x != LDAP_ERR_SUCCESS) { debug("SearchLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(x)); - local_printfx("ERR message=\"(SearchLDAP: %s)\"\n", ErrLDAP(x)); + local_printfx("BH message=\"(SearchLDAP: %s)\"\n", ErrLDAP(x)); } else { edui_ldap.err = -1; debug("SearchLDAP(-, %d, %s, -) -> %s\n", edui_conf.scope, edui_ldap.search_filter, ErrLDAP(x)); x = SearchIPLDAP(&edui_ldap); - if (x != LDAP_ERR_SUCCESS) { - debug("SearchIPLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); + if (x == LDAP_ERR_NOTFOUND) { + debug("SearchIPLDAP() -> %s\n", ErrLDAP(x)); local_printfx("ERR message=\"(SearchIPLDAP: %s)\"\n", ErrLDAP(x)); - } else { + } else if (x == LDAP_ERR_SUCCESS) { debug("SearchIPLDAP(-, %s) -> %s\n", edui_ldap.userid, ErrLDAP(x)); local_printfx("OK user=%s\n", edui_ldap.userid); /* Got a userid --> OK user= */ + } else if (x != LDAP_ERR_SUCCESS) { + debug("SearchIPLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err)); + local_printfx("BH message=\"(SearchIPLDAP: %s)\"\n", ErrLDAP(x)); } } } === modified file 'helpers/external_acl/file_userip/ext_file_userip_acl.cc' --- helpers/external_acl/file_userip/ext_file_userip_acl.cc 2017-01-01 00:16:45 +0000 +++ helpers/external_acl/file_userip/ext_file_userip_acl.cc 2017-01-27 02:26:04 +0000 @@ -265,7 +265,7 @@ if (strchr(line, '\n') != NULL) break; } - SEND_ERR("Input Too Large."); + SEND_BH(HLP_MSG("Input Too Large.")); continue; } *cp = '\0'; @@ -273,7 +273,7 @@ username = strtok(NULL, " \t"); if (!address || !username) { debug("%s: unable to read tokens\n", program_name); - SEND_ERR("Invalid Input."); + SEND_BH(HLP_MSG("Invalid Input.")); continue; } rfc1738_unescape(address); === modified file 'helpers/external_acl/time_quota/ext_time_quota_acl.cc' --- helpers/external_acl/time_quota/ext_time_quota_acl.cc 2017-01-01 00:16:45 +0000 +++ helpers/external_acl/time_quota/ext_time_quota_acl.cc 2017-01-27 02:26:04 +0000 @@ -452,7 +452,7 @@ // we expect the following line syntax: %LOGIN const char *user_key = strtok(request, " \n"); if (!user_key) { - SEND_BH("message=\"User name missing\""); + SEND_BH(HLP_MSG("User name missing")); continue; } processActivity(user_key); === modified file 'helpers/external_acl/unix_group/check_group.cc' --- helpers/external_acl/unix_group/check_group.cc 2017-01-01 00:16:45 +0000 +++ helpers/external_acl/unix_group/check_group.cc 2017-01-27 02:26:04 +0000 @@ -198,12 +198,12 @@ if (strchr(buf, '\n') != NULL) break; } - SEND_ERR("Username Input too large."); + SEND_BH(HLP_MSG("Username Input too large.")); continue; } *p = '\0'; if ((p = strtok(buf, " ")) == NULL) { - SEND_ERR("No username given."); + SEND_BH(HLP_MSG("No username given.")); continue; } else { user = p; === modified file 'src/external_acl.cc' --- src/external_acl.cc 2017-01-01 00:16:45 +0000 +++ src/external_acl.cc 2017-01-27 02:26:04 +0000 @@ -98,6 +98,8 @@ void trimCache(); + bool maybeCacheable(const allow_t &) const; + int ttl; int negative_ttl; @@ -611,6 +613,21 @@ } } +bool +external_acl::maybeCacheable(const allow_t &result) const +{ + if (cache_size <= 0) + return false; // cache is disabled + + if (result == ACCESS_DUNNO) + return false; // non-cacheable response + + if ((result == ACCESS_ALLOWED ? ttl : negative_ttl) <= 0) + return false; // not caching this type of response + + return true; +} + /****************************************************************** * external acl type */ @@ -874,7 +891,7 @@ external_acl_cache_touch(external_acl * def, const ExternalACLEntryPointer &entry) { // this must not be done when nothing is being cached. - if (def->cache_size <= 0 || (def->ttl <= 0 && entry->result == 1) || (def->negative_ttl <= 0 && entry->result != 1)) + if (!def->maybeCacheable(entry->result)) return; dlinkDelete(&entry->lru, &def->lru_list); @@ -1223,10 +1240,10 @@ static int external_acl_entry_expired(external_acl * def, const ExternalACLEntryPointer &entry) { - if (def->cache_size <= 0) + if (def->cache_size <= 0 || entry->result == ACCESS_DUNNO) return 1; - if (entry->date + (entry->result == 1 ? def->ttl : def->negative_ttl) < squid_curtime) + if (entry->date + (entry->result == ACCESS_ALLOWED ? def->ttl : def->negative_ttl) < squid_curtime) return 1; else return 0; @@ -1235,11 +1252,11 @@ static int external_acl_grace_expired(external_acl * def, const ExternalACLEntryPointer &entry) { - if (def->cache_size <= 0) + if (def->cache_size <= 0 || entry->result == ACCESS_DUNNO) return 1; int ttl; - ttl = entry->result == 1 ? def->ttl : def->negative_ttl; + ttl = entry->result == ACCESS_ALLOWED ? def->ttl : def->negative_ttl; ttl = (ttl * (100 - def->grace)) / 100; if (entry->date + ttl <= squid_curtime) @@ -1253,9 +1270,13 @@ { ExternalACLEntryPointer entry; - // do not bother caching this result if TTL is going to expire it immediately - if (def->cache_size <= 0 || (def->ttl <= 0 && data.result == 1) || (def->negative_ttl <= 0 && data.result != 1)) { + if (!def->maybeCacheable(data.result)) { debugs(82,6, HERE); + if (data.result == ACCESS_DUNNO) { + const ExternalACLEntryPointer oldentry = static_cast(hash_lookup(def->cache, key)); + if (oldentry != NULL) + external_acl_cache_delete(def, oldentry); + } entry = new ExternalACLEntry; entry->key = xstrdup(key); entry->update(data); @@ -1347,13 +1368,15 @@ externalAclState *state = static_cast(data); externalAclState *next; ExternalACLEntryData entryData; - entryData.result = ACCESS_DENIED; debugs(82, 2, HERE << "reply=" << reply); if (reply.result == Helper::Okay) entryData.result = ACCESS_ALLOWED; - // XXX: handle other non-DENIED results better + else if (reply.result == Helper::Error) + entryData.result = ACCESS_DENIED; + else //BrokenHelper,TimedOut or Unknown. Should not cached. + entryData.result = ACCESS_DUNNO; // XXX: make entryData store a proper Helper::Reply object instead of copying. @@ -1384,17 +1407,8 @@ dlinkDelete(&state->list, &state->def->queue); ExternalACLEntryPointer entry; - if (cbdataReferenceValid(state->def)) { - // only cache OK and ERR results. - if (reply.result == Helper::Okay || reply.result == Helper::Error) - entry = external_acl_cache_add(state->def, state->key, entryData); - else { - const ExternalACLEntryPointer oldentry = static_cast(hash_lookup(state->def->cache, state->key)); - - if (oldentry != NULL) - external_acl_cache_delete(state->def, oldentry); - } - } + if (cbdataReferenceValid(state->def)) + entry = external_acl_cache_add(state->def, state->key, entryData); do { void *cbdata;