On 11/01/2017 05:27 μμ, Amos Jeffries wrote:
On 10/01/2017 12:49 a.m., Christos Tsantilas wrote:
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 doesn't 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 is 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 retries 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 patch fixes the problems described above.

This is a Measurement Factory project


Thank you for this long overdue fix.

+1. Though if possible I would like one extra change...

Please add the below method to class external_acl and use it to
de-duplicate the complex if-conditions in external_acl_cache_touch and
external_acl_cache_add about whether a response is non-cacheable:

bool
external_acl::maybeCacheable(const allow_t &result)
{
    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;
}

The patch applied to squid-5 branch as r15005.
I am also attaching the patch for squid-3.5



Could you also mention for the squid-dev record how much testing this
patch has had.

Well, the fixes inside squid should be enough tested. It is tested with custom helpers and used various scenarios.

The  fixes in ext_ldap_group_acl helper should be enough tested too.

The other helpers which fixed with this patch did not tested, but the fixes looks safe.

I also checked a little the ext_kerberos_ldap_group_acl helper. Looks that it is ok, so no changes are required, but I did not run tests with this helper.



Cheers
Amos

_______________________________________________
squid-dev mailing list
[email protected]
http://lists.squid-cache.org/listinfo/squid-dev


External ACL helpers error handling & 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 doesn't 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 is 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 retries 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

=== modified file 'helpers/defines.h'
--- helpers/defines.h	2016-01-01 00:14:27 +0000
+++ helpers/defines.h	2017-01-09 11:21:11 +0000
@@ -34,28 +34,31 @@
  * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
  * POSSIBILITY OF SUCH DAMAGE.
  */
 
 #ifndef __SQUID_HELPERS_DEFINES_H
 #define __SQUID_HELPERS_DEFINES_H
 
 /*
  * This file contains several macro definitions which are
  * useful and shared between helpers.
  */
 
 #define HELPER_INPUT_BUFFER 8196
 
 /* send OK result to Squid with a string parameter. */
 #define SEND_OK(x)  fprintf(stdout, "OK %s\n",x)
 
 /* 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)
 
 #endif /* __SQUID_HELPERS_DEFINES_H */
 

=== modified file 'helpers/external_acl/AD_group/ext_ad_group_acl.cc'
--- helpers/external_acl/AD_group/ext_ad_group_acl.cc	2016-01-01 00:14:27 +0000
+++ helpers/external_acl/AD_group/ext_ad_group_acl.cc	2017-01-09 11:23:59 +0000
@@ -802,58 +802,58 @@
             DefaultDomain = xstrdup(machinedomain);
     }
     debug("%s " VERSION " " SQUID_BUILD_INFO " starting up...\n", argv[0]);
     if (use_global)
         debug("Domain Global group mode enabled using '%s' as default domain.\n", DefaultDomain);
     if (use_case_insensitive_compare)
         debug("Warning: running in case insensitive mode !!!\n");
 
     atexit(CloseCOM);
 
     /* Main Loop */
     while (fgets(buf, HELPER_INPUT_BUFFER, stdin)) {
         if (NULL == strchr(buf, '\n')) {
             /* too large message received.. skip and deny */
             fprintf(stderr, "%s: ERROR: Too large: %s\n", argv[0], buf);
             while (fgets(buf, HELPER_INPUT_BUFFER, stdin)) {
                 fprintf(stderr, "%s: ERROR: Too large..: %s\n", argv[0], buf);
                 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)
             *p = '\0';      /* strip \n */
         if ((p = strchr(buf, '\r')) != NULL)
             *p = '\0';      /* strip \r */
 
         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, " ");
         for (n = 0; (group = strtok(NULL, " ")) != NULL; ++n) {
             rfc1738_unescape(group);
             groups[n] = group;
         }
         groups[n] = NULL;
         numberofgroups = n;
 
         if (NULL == username) {
-            SEND_ERR("Invalid Request. No Username.");
+            SEND_BH(HLP_MSG("Invalid Request. No Username."));
             continue;
         }
         rfc1738_unescape(username);
 
         if ((use_global ? Valid_Global_Groups(username, groups) : Valid_Local_Groups(username, groups))) {
             SEND_OK("");
         } else {
             SEND_ERR("");
         }
         err = 0;
     }
     return 0;
 }
 

=== modified file 'helpers/external_acl/LDAP_group/ext_ldap_group_acl.cc'
--- helpers/external_acl/LDAP_group/ext_ldap_group_acl.cc	2016-01-13 16:29:59 +0000
+++ helpers/external_acl/LDAP_group/ext_ldap_group_acl.cc	2017-01-09 11:29:01 +0000
@@ -449,166 +449,175 @@
         HMODULE WLDAP32Handle;
 
         WLDAP32Handle = GetModuleHandle("wldap32");
         if ((Win32_ldap_start_tls_s = (PFldap_start_tls_s) GetProcAddress(WLDAP32Handle, LDAP_START_TLS_S)) == NULL) {
             fprintf(stderr, PROGRAM_NAME ": FATAL: TLS (-Z) not supported on this platform.\n");
             exit(1);
         }
     }
 #endif
 
     while (fgets(buf, HELPER_INPUT_BUFFER, stdin) != NULL) {
         int found = 0;
         if (!strchr(buf, '\n')) {
             /* too large message received.. skip and deny */
             fprintf(stderr, "%s: ERROR: Input Too large: %s\n", argv[0], buf);
             while (fgets(buf, sizeof(buf), stdin)) {
                 fprintf(stderr, "%s: ERROR: Input Too large..: %s\n", argv[0], buf);
                 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);
         if (strip_nt_domain) {
             char *u = strrchr(user, '\\');
             if (!u)
                 u = strrchr(user, '/');
             if (!u)
                 u = strrchr(user, '+');
             if (u && u[1])
                 user = u + 1;
         }
         if (strip_kerberos_realm) {
             char *u = strchr(user, '@');
             if (u != NULL) {
                 *u = '\0';
             }
         }
         if (use_extension_dn) {
             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);
 
 recover:
             if (ld == NULL) {
 #if HAS_URI_SUPPORT
                 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;
                     }
                 } else
 #endif
 #if NETSCAPE_SSL
                     if (sslpath) {
                         if (!sslinit && (ldapssl_client_init(sslpath, NULL) != LDAP_SUCCESS)) {
                             fprintf(stderr, "FATAL: Unable to initialise SSL with cert path %s\n", sslpath);
                             exit(1);
                         } else {
                             ++sslinit;
                         }
                         if ((ld = ldapssl_init(ldapServer, port, 1)) == NULL) {
                             fprintf(stderr, "FATAL: Unable to connect to SSL LDAP server: %s port:%d\n",
                                     ldapServer, port);
                             exit(1);
                         }
                     } 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)
                     squid_ldap_set_connect_timeout(ld, connect_timeout);
 
 #ifdef LDAP_VERSION3
                 if (version == -1) {
                     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;
                 }
                 if (use_tls) {
 #ifdef LDAP_OPT_X_TLS
                     if (version != LDAP_VERSION3) {
                         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;
                     }
 #else
                     fprintf(stderr, "FATAL: TLS not supported with your LDAP library\n");
                     exit(1);
 #endif
                 }
 #endif
                 squid_ldap_set_timelimit(ld, timelimit);
                 squid_ldap_set_referrals(ld, !noreferrals);
                 squid_ldap_set_aliasderef(ld, aliasderef);
                 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;
                     }
                 }
                 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("");
         }
 
         if (ld != NULL) {
             if (!persistent || (squid_ldap_errno(ld) != LDAP_SUCCESS && squid_ldap_errno(ld) != LDAP_INVALID_CREDENTIALS)) {
                 ldap_unbind(ld);
                 ld = NULL;
             } else {
                 tryagain = 1;
             }
         }
     }
     if (ld)
         ldap_unbind(ld);
     return 0;
 }
 
 static int
 ldap_escape_value(char *escaped, int size, const char *src)
@@ -696,60 +705,60 @@
     return 0;
 }
 
 static int
 searchLDAPGroup(LDAP * ld, char *group, char *member, char *extension_dn)
 {
     char filter[256];
     static char searchbase[256];
     LDAPMessage *res = NULL;
     LDAPMessage *entry;
     int rc;
     char *searchattr[] = {(char *) LDAP_NO_ATTRS, NULL};
 
     if (extension_dn && *extension_dn)
         snprintf(searchbase, sizeof(searchbase), "%s,%s", extension_dn, basedn);
     else
         snprintf(searchbase, sizeof(searchbase), "%s", basedn);
 
     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);
 
     rc = ldap_search_s(ld, searchbase, searchscope, filter, searchattr, 1, &res);
     if (rc != LDAP_SUCCESS) {
         if (noreferrals && rc == LDAP_PARTIAL_RESULTS) {
             /* Everything is fine. This is expected when referrals
              * are disabled.
              */
         } else {
             fprintf(stderr, PROGRAM_NAME ": WARNING: LDAP search error '%s'\n", ldap_err2string(rc));
 #if defined(NETSCAPE_SSL)
             if (sslpath && ((rc == LDAP_SERVER_DOWN) || (rc == LDAP_CONNECT_ERROR))) {
                 int sslerr = PORT_GetError();
                 fprintf(stderr, PROGRAM_NAME ": WARNING: SSL error %d (%s)\n", sslerr, ldapssl_err2string(sslerr));
             }
 #endif
             ldap_msgfree(res);
-            return 1;
+            return -1;
         }
     }
     entry = ldap_first_entry(ld, res);
     if (!entry) {
         ldap_msgfree(res);
         return 1;
     }
     ldap_msgfree(res);
     return 0;
 }
 
 static int
 searchLDAP(LDAP * ld, char *group, char *login, char *extension_dn)
 {
 
     if (usersearchfilter) {
         char filter[8192];
         char searchbase[8192];
         char escaped_login[1024];
         LDAPMessage *res = NULL;
@@ -762,41 +771,41 @@
         else
             snprintf(searchbase, sizeof(searchbase), "%s", userbasedn ? userbasedn : basedn);
         ldap_escape_value(escaped_login, sizeof(escaped_login), login);
         snprintf(filter, sizeof(filter), usersearchfilter, escaped_login, escaped_login, escaped_login, escaped_login, escaped_login, escaped_login, escaped_login, escaped_login, escaped_login, escaped_login, escaped_login, escaped_login, escaped_login, escaped_login, escaped_login, escaped_login);
         debug("user filter '%s', searchbase '%s'\n", filter, searchbase);
         rc = ldap_search_s(ld, searchbase, searchscope, filter, searchattr, 1, &res);
         if (rc != LDAP_SUCCESS) {
             if (noreferrals && rc == LDAP_PARTIAL_RESULTS) {
                 /* Everything is fine. This is expected when referrals
                  * are disabled.
                  */
             } else {
                 fprintf(stderr, PROGRAM_NAME ": WARNING: LDAP search error '%s'\n", ldap_err2string(rc));
 #if defined(NETSCAPE_SSL)
                 if (sslpath && ((rc == LDAP_SERVER_DOWN) || (rc == LDAP_CONNECT_ERROR))) {
                     int sslerr = PORT_GetError();
                     fprintf(stderr, PROGRAM_NAME ": WARNING: SSL error %d (%s)\n", sslerr, ldapssl_err2string(sslerr));
                 }
 #endif
                 ldap_msgfree(res);
-                return 1;
+                return -1;
             }
         }
         entry = ldap_first_entry(ld, res);
         if (!entry) {
             fprintf(stderr, PROGRAM_NAME ": WARNING: User '%s' not found in '%s'\n", login, searchbase);
             ldap_msgfree(res);
             return 1;
         }
         userdn = ldap_get_dn(ld, entry);
         rc = searchLDAPGroup(ld, group, userdn, extension_dn);
         squid_ldap_memfree(userdn);
         ldap_msgfree(res);
         return rc;
     } else if (userdnattr) {
         char dn[8192];
         if (extension_dn && *extension_dn)
             snprintf(dn, 8192, "%s=%s, %s, %s", userdnattr, login, extension_dn, userbasedn ? userbasedn : basedn);
         else
             snprintf(dn, 8192, "%s=%s, %s", userdnattr, login, userbasedn ? userbasedn : basedn);
         return searchLDAPGroup(ld, group, dn, extension_dn);

=== modified file 'helpers/external_acl/LM_group/ext_lm_group_acl.cc'
--- helpers/external_acl/LM_group/ext_lm_group_acl.cc	2016-01-01 00:14:27 +0000
+++ helpers/external_acl/LM_group/ext_lm_group_acl.cc	2017-01-09 11:23:59 +0000
@@ -544,56 +544,56 @@
     if (use_global) {
         debug("Domain Global group mode enabled using '%s' as default domain.\n", DefaultDomain);
     }
     if (use_case_insensitive_compare) {
         debug("Warning: running in case insensitive mode !!!\n");
     }
     if (use_PDC_only) {
         debug("Warning: using only PDCs for group validation !!!\n");
     }
 
     /* Main Loop */
     while (fgets(buf, HELPER_INPUT_BUFFER, stdin)) {
         if (NULL == strchr(buf, '\n')) {
             /* too large message received.. skip and deny */
             debug("%s: ERROR: Too large: %s\n", argv[0], buf);
             while (fgets(buf, HELPER_INPUT_BUFFER, stdin)) {
                 debug("%s: ERROR: Too large..: %s\n", argv[0], buf);
                 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)
             *p = '\0';      /* strip \n */
         if ((p = strchr(buf, '\r')) != NULL)
             *p = '\0';      /* strip \r */
 
         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, " ");
         for (n = 0; (group = strtok(NULL, " ")) != NULL; ++n) {
             rfc1738_unescape(group);
             groups[n] = group;
         }
         groups[n] = NULL;
 
         if (NULL == username) {
-            SEND_ERR("Invalid Request. No Username.");
+            SEND_BH(HLP_MSG("Invalid Request. No Username."));
             continue;
         }
         rfc1738_unescape(username);
 
         if ((use_global ? Valid_Global_Groups(username, groups) : Valid_Local_Groups(username, groups))) {
             SEND_OK("");
         } else {
             SEND_ERR("");
         }
     }
     return 0;
 }
 

=== modified file 'helpers/external_acl/SQL_session/ext_sql_session_acl.pl.in'
--- helpers/external_acl/SQL_session/ext_sql_session_acl.pl.in	2016-01-01 00:14:27 +0000
+++ helpers/external_acl/SQL_session/ext_sql_session_acl.pl.in	2017-01-09 11:23:59 +0000
@@ -178,31 +178,31 @@
 	close_db();
 	open_db() || return undef;
 	$sth->execute($uid) || return undef;;
     }
     return $sth;
 }
 my $status;
 
 $|=1;
 while (<>) {
     my $string = $_;
     $string =~ m/^(\d+)\s(.*)$/;
     my ($cid, $uid) = ($1, $2);
 
     $status = "ERR";
     $cid =~ s/%(..)/pack("H*", $1)/ge;
     $uid =~ s/%(..)/pack("H*", $1)/ge;
 
     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."'\"";
     my $row = $sth->fetchrow_hashref() || next;
     $status = $cid . " OK" . ($row->{'user'} ne "" ? " user=" . $row->{'user'} : "" ) . ($row->{'tag'} ne "" ? " tag=" . $row->{'tag'} : "" );
     $sth->finish();
 } continue {
     close_db() if (!$persist);
     print $status . "\n";
 }

=== modified file 'helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc'
--- helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc	2016-01-01 00:14:27 +0000
+++ helpers/external_acl/eDirectory_userip/ext_edirectory_userip_acl.cc	2017-01-09 11:23:59 +0000
@@ -1741,41 +1741,41 @@
             edui_elap = 0;
         k = strlen(bufa);
         /* BINARY DEBUGGING *
                     local_printfx("while() -> bufa[%" PRIuSIZE "]: %s", k, bufa);
                     for (i = 0; i < k; ++i)
                       local_printfx("%02X", bufa[i]);
                     local_printfx("\n");
         * BINARY DEBUGGING */
         /* Check for CRLF */
         p = strchr(bufa, '\n');
         if (p != NULL)
             *p = '\0';
         p = strchr(bufa, '\r');
         if (p != NULL)
             *p = '\0';
         p = strchr(bufa, ' ');
 
         /* 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;
 
         /* Open LDAP connection */
         if (!(edui_ldap.status & LDAP_INIT_S)) {
             InitLDAP(&edui_ldap);
             debug("InitLDAP() -> %s\n", ErrLDAP(LDAP_ERR_SUCCESS));
             if (edui_conf.mode & EDUI_MODE_PERSIST)                 /* Setup persistant mode */
                 edui_ldap.status |= LDAP_PERSIST_S;
         }
         if ((edui_ldap.status & LDAP_IDLE_S) && (edui_elap > 0)) {
             edui_ldap.idle_time = edui_ldap.idle_time + edui_elap;
         }
         if ((edui_ldap.status & LDAP_PERSIST_S) && (edui_ldap.status & LDAP_IDLE_S) && (edui_ldap.idle_time > edui_conf.persist_timeout)) {
             debug("while() -> Connection timed out after %d seconds\n", (int)(edui_ldap.idle_time));
             x = CloseLDAP(&edui_ldap);
             debug("CloseLDAP(-) -> %s\n", ErrLDAP(x));
         }
         edui_ldap.err = -1;
@@ -1784,161 +1784,167 @@
             if (x != LDAP_ERR_SUCCESS) {
                 /* Failed to connect */
                 debug("OpenLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err));
             } else {
                 debug("OpenLDAP(-, %s, %d) -> %s\n", edui_conf.host, edui_conf.port, ErrLDAP(x));
                 x = SetVerLDAP(&edui_ldap, edui_conf.ver);
                 if (x != LDAP_ERR_SUCCESS) {
                     /* Failed to set version */
                     debug("SetVerLDAP() -> %s (LDAP: %s)\n", ErrLDAP(x), ldap_err2string(edui_ldap.err));
                 } else
                     debug("SetVerLDAP(-, %d) -> %s\n", edui_conf.ver, ErrLDAP(x));
             }
         }
         edui_ldap.err = -1;
         if (!(edui_ldap.status & LDAP_BIND_S) && (edui_conf.mode & EDUI_MODE_TLS)) {
             /* TLS binding */
             x = BindLDAP(&edui_ldap, edui_conf.dn, edui_conf.passwd, LDAP_AUTH_TLS);
             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));
         } else if (!(edui_ldap.status & LDAP_BIND_S)) {
             if (edui_conf.dn[0] != '\0') {
                 /* Simple binding - using dn / passwd for authorization */
                 x = BindLDAP(&edui_ldap, edui_conf.dn, edui_conf.passwd, LDAP_AUTH_SIMPLE);
                 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));
             } else {
                 /* Anonymous binding */
                 x = BindLDAP(&edui_ldap, edui_conf.dn, edui_conf.passwd, LDAP_AUTH_NONE);
                 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));
             }
         }
         edui_ldap.err = -1;
         if (edui_ldap.status & LDAP_PERSIST_S) {
             x = ResetLDAP(&edui_ldap);
             if (x != LDAP_ERR_SUCCESS) {
                 /* Unable to reset */
                 debug("ResetLDAP() -> %s\n", ErrLDAP(x));
             } else
                 debug("ResetLDAP() -> %s\n", ErrLDAP(x));
         }
         if (x != LDAP_ERR_SUCCESS) {
             /* 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;
         /* If we got a group string, split it */
         if (p != NULL) {
             /* Split string */
             debug("StringSplit(%s, ' ', %s, %" PRIuSIZE ")\n", bufa, bufb, sizeof(bufb));
             i = StringSplit(bufa, ' ', bufb, sizeof(bufb));
             if (i > 0) {
                 debug("StringSplit(%s, %s) done.  Result: %" PRIuSIZE "\n", bufa, bufb, i);
                 /* Got a group to match against */
                 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;
                         debug("SearchFilterLDAP(-, %s) -> Length: %u\n", bufa, 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(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=<userid> */
+                            } 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 */
                         memset(bufc, '\0', sizeof(bufc));
                     }
                 }
             } 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=<userid> */
+                        } 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));
                         }
                     }
                 }
                 /* Clear for next query */
                 memset(bufc, '\0', sizeof(bufc));
             }
         }
 
         /* Clear buffer and close for next data, if not persistent */
         edui_ldap.err = -1;
         memset(bufa, '\0', sizeof(bufa));
         if (!(edui_ldap.status & LDAP_PERSIST_S)) {
             x = CloseLDAP(&edui_ldap);
             debug("CloseLDAP(-) -> %s\n", ErrLDAP(x));
         }
     }
 
     debug("Terminating.\n");
     return 1;
 }

=== modified file 'helpers/external_acl/file_userip/ext_file_userip_acl.cc'
--- helpers/external_acl/file_userip/ext_file_userip_acl.cc	2016-01-01 00:14:27 +0000
+++ helpers/external_acl/file_userip/ext_file_userip_acl.cc	2017-01-09 11:30:56 +0000
@@ -248,46 +248,46 @@
         fprintf(stderr, "%s: FATAL: No Filename configured.", program_name);
         usage(program_name);
         exit(1);
     }
     FILE *FH = fopen(filename, "r");
     if (!FH) {
         fprintf(stderr, "%s: FATAL: Unable to open file '%s': %s", program_name, filename, xstrerror());
         exit(1);
     }
     current_entry = load_dict(FH);
 
     while (fgets(line, HELPER_INPUT_BUFFER, stdin)) {
         if ((cp = strchr (line, '\n')) == NULL) {
             /* too large message received.. skip and deny */
             fprintf(stderr, "%s: ERROR: Input Too Large: %s\n", program_name, line);
             while (fgets(line, sizeof(line), stdin)) {
                 fprintf(stderr, "%s: ERROR: Input Too Large..: %s\n", program_name, line);
                 if (strchr(line, '\n') != NULL)
                     break;
             }
-            SEND_ERR("Input Too Large.");
+            SEND_BH(HLP_MSG("Input Too Large."));
             continue;
         }
         *cp = '\0';
         address = strtok(line, " \t");
         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);
         rfc1738_unescape(username);
         int result = dict_lookup(current_entry, username, address);
         debug("%s: result: %d\n", program_name, result);
         if (result != 0) {
             SEND_OK("");
         } else {
             SEND_ERR("");
         }
     }
 
     fclose (FH);
     return 0;
 }
 

=== modified file 'helpers/external_acl/time_quota/ext_time_quota_acl.cc'
--- helpers/external_acl/time_quota/ext_time_quota_acl.cc	2016-01-01 00:14:27 +0000
+++ helpers/external_acl/time_quota/ext_time_quota_acl.cc	2017-01-09 11:23:59 +0000
@@ -435,30 +435,30 @@
         }
     }
 
     log_info("Starting %s\n", __FILE__);
     setbuf(stdout, NULL);
 
     init_db();
 
     if ( optind + 1 != argc ) {
         usage();
         exit(1);
     } else {
         readConfig(argv[optind]);
     }
 
     log_info("Waiting for requests...\n");
     while (fgets(request, HELPER_INPUT_BUFFER, stdin)) {
         // 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);
     }
     log_info("Ending %s\n", __FILE__);
     shutdown_db();
     return 0;
 }
 

=== modified file 'helpers/external_acl/unix_group/check_group.cc'
--- helpers/external_acl/unix_group/check_group.cc	2016-01-01 00:14:27 +0000
+++ helpers/external_acl/unix_group/check_group.cc	2017-01-09 11:31:40 +0000
@@ -181,46 +181,46 @@
         default:
             usage(argv[0]);
             exit(1);
         }
     }
     if (optind < argc) {
         fprintf(stderr, "FATAL: Unknown option '%s'\n", argv[optind]);
         usage(argv[0]);
         exit(1);
     }
     while (fgets(buf, HELPER_INPUT_BUFFER, stdin)) {
         j = 0;
         if ((p = strchr(buf, '\n')) == NULL) {
             /* too large message received.. skip and deny */
             fprintf(stderr, "ERROR: %s: Too large: %s\n", argv[0], buf);
             while (fgets(buf, sizeof(buf), stdin)) {
                 fprintf(stderr, "ERROR: %s: Too large..: %s\n", argv[0], buf);
                 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;
             rfc1738_unescape(user);
             if (strip_dm) {
                 suser = strchr(user, '\\');
                 if (!suser) suser = strchr(user, '/');
                 if (suser && suser[1]) user = suser + 1;
             }
             /* check groups supplied by Squid */
             while ((p = strtok(NULL, " ")) != NULL) {
                 rfc1738_unescape(p);
                 if (check_pw == 1)
                     j += validate_user_pw(user, p);
                 j += validate_user_gr(user, p);
             }
         }
 
         /* check groups supplied on the command line */
         for (i = 0; i < ngroups; ++i) {

=== modified file 'src/external_acl.cc'
--- src/external_acl.cc	2016-10-25 08:23:49 +0000
+++ src/external_acl.cc	2017-01-11 18:58:45 +0000
@@ -81,40 +81,42 @@
     Format::ByteCode_t type;
     external_acl_format::Pointer next;
     char *header;
     char *member;
     char separator;
     http_hdr_type header_id;
 };
 
 MEMPROXY_CLASS_INLINE(external_acl_format);
 
 class external_acl
 {
 
 public:
     external_acl *next;
 
     void add(const ExternalACLEntryPointer &);
 
     void trimCache();
 
+    bool maybeCacheable(const allow_t &) const;
+
     int ttl;
 
     int negative_ttl;
 
     int grace;
 
     char *name;
 
     external_acl_format::Pointer format;
 
     wordlist *cmdline;
 
     Helper::ChildConfig children;
 
     helper *theHelper;
 
     hash_table *cache;
 
     dlink_list lru_list;
 
@@ -594,40 +596,55 @@
     trimCache();
     assert(anEntry != NULL);
     assert (anEntry->def == NULL);
     anEntry->def = this;
     ExternalACLEntry *e = const_cast<ExternalACLEntry *>(anEntry.getRaw()); // XXX: make hash a std::map of Pointer.
     hash_join(cache, e);
     dlinkAdd(e, &e->lru, &lru_list);
     e->lock(); //cbdataReference(e); // lock it on behalf of the hash
     ++cache_entries;
 }
 
 void
 external_acl::trimCache()
 {
     if (cache_size && cache_entries >= cache_size) {
         ExternalACLEntryPointer e(static_cast<ExternalACLEntry *>(lru_list.tail->data));
         external_acl_cache_delete(this, e);
     }
 }
 
+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
  */
 
 struct _external_acl_data {
     external_acl *def;
     const char *name;
     wordlist *arguments;
 };
 
 CBDATA_TYPE(external_acl_data);
 static void
 free_external_acl_data(void *data)
 {
     external_acl_data *p = static_cast<external_acl_data *>(data);
     safe_free(p->name);
     wordlistDestroy(&p->arguments);
     cbdataReferenceDone(p->def);
 }
 
@@ -857,41 +874,41 @@
     SBufList rv;
     rv.push_back(SBuf(acl->def->name));
 
     for (wordlist *arg = acl->arguments; arg; arg = arg->next) {
         SBuf s;
         s.Printf(" %s", arg->key);
         rv.push_back(s);
     }
 
     return rv;
 }
 
 /******************************************************************
  * external_acl cache
  */
 
 static void
 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);
     ExternalACLEntry *e = const_cast<ExternalACLEntry *>(entry.getRaw()); // XXX: make hash a std::map of Pointer.
     dlinkAdd(e, &entry->lru, &def->lru_list);
 }
 
 #if USE_OPENSSL
 static const char *
 external_acl_ssl_get_user_attribute(const ACLFilledChecklist &ch, const char *attr)
 {
     if (ch.conn() != NULL && Comm::IsConnOpen(ch.conn()->clientConnection)) {
         if (SSL *ssl = fd_table[ch.conn()->clientConnection->fd].ssl)
             return sslGetUserAttribute(ssl, attr);
     }
     return NULL;
 }
 #endif
 
 static char *
@@ -1206,73 +1223,77 @@
             if (!first)
                 mb.append(" ", 1);
 
             if (acl_data->def->quote == external_acl::QUOTE_METHOD_URL) {
                 const char *quoted = rfc1738_escape(arg->key);
                 mb.append(quoted, strlen(quoted));
             } else {
                 strwordquote(&mb, arg->key);
             }
 
             first = 0;
         }
     }
 
     return mb.buf;
 }
 
 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;
 }
 
 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)
         return 1;
     else
         return 0;
 }
 
 static ExternalACLEntryPointer
 external_acl_cache_add(external_acl * def, const char *key, ExternalACLEntryData const & data)
 {
     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<ExternalACLEntry *>(hash_lookup(def->cache, key));
+            if (oldentry != NULL)
+                external_acl_cache_delete(def, oldentry);
+        }
         entry = new ExternalACLEntry;
         entry->key = xstrdup(key);
         entry->update(data);
         entry->def = def;
         return entry;
     }
 
     entry = static_cast<ExternalACLEntry *>(hash_lookup(def->cache, key));
     debugs(82, 2, "external_acl_cache_add: Adding '" << key << "' = " << data.result);
 
     if (entry != NULL) {
         debugs(82, 3, "updating existing entry");
         entry->update(data);
         external_acl_cache_touch(def, entry);
         return entry;
     }
 
     entry = new ExternalACLEntry;
     entry->key = xstrdup(key);
     entry->update(data);
@@ -1330,88 +1351,81 @@
  * Keywords:
  *
  *   user=      The users name (login)
  *   message=   Message describing the reason
  *   tag=   A string tag to be applied to the request that triggered the acl match.
  *          applies to both OK and ERR responses.
  *          Won't override existing request tags.
  *   log=   A string to be used in access logging
  *
  * Other keywords may be added to the protocol later
  *
  * value needs to be URL-encoded or enclosed in double quotes (")
  * with \-escaping on any whitespace, quotes, or slashes (\).
  */
 static void
 externalAclHandleReply(void *data, const Helper::Reply &reply)
 {
     externalAclState *state = static_cast<externalAclState *>(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.
 
     entryData.notes.append(&reply.notes);
 
     const char *label = reply.notes.findFirst("tag");
     if (label != NULL && *label != '\0')
         entryData.tag = label;
 
     label = reply.notes.findFirst("message");
     if (label != NULL && *label != '\0')
         entryData.message = label;
 
     label = reply.notes.findFirst("log");
     if (label != NULL && *label != '\0')
         entryData.log = label;
 
 #if USE_AUTH
     label = reply.notes.findFirst("user");
     if (label != NULL && *label != '\0')
         entryData.user = label;
 
     label = reply.notes.findFirst("password");
     if (label != NULL && *label != '\0')
         entryData.password = label;
 #endif
 
     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<ExternalACLEntry *>(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;
         cbdataReferenceDone(state->def);
 
         if (state->callback && cbdataReferenceValidDone(state->callback_data, &cbdata))
             state->callback(cbdata, entry);
 
         next = state->queue;
 
         cbdataFree(state);
 
         state = next;
     } while (state);
 }
 
 void
 ACLExternal::ExternalAclLookup(ACLChecklist *checklist, ACLExternal * me)
 {
     ExternalACLLookup::Start(checklist, me->data, false);

_______________________________________________
squid-dev mailing list
[email protected]
http://lists.squid-cache.org/listinfo/squid-dev

Reply via email to