Enable non-caching of external ACL results

Admin configure ttl=0 and/or negative_ttl=0 to prevent Squid storing the
ACL lookup results. The problem is that results still get cached and
re-used for the grace= period or one second, whichever is larger.

Also, in the event where two or more requests with identical details
needing to be looked up at the same time there is an optimization
which will merge and share one lookup result for all these requests.

In most situations this result sharing is beneficial, however when a
unique result is wanted it can cause problems.

This patch makes ttl=0 and negative_ttl=0 prevent their respective OK and
ERR results from being stored into the helper result cache. Sharing is
still performed for overlapping duplicate requests.

When cache=0 is configured, no caching or sharing of results is performed
at all.

diff --git a/src/external_acl.cc b/src/external_acl.cc
index aef254110139d993eeec3cf3c4f36d312db0a99f..862c058a1ba9c29097bec16c1e1bde32a2cad9b8 100644 (file)
--- a/src/external_acl.cc
+++ b/src/external_acl.cc
@@ -726,15 +726,20 @@ aclMatchExternal(external_acl_data *acl, ACLFilledChecklist *ch)
     int result;
     external_acl_entry *entry;
     const char *key = "";
-    debugs(82, 9, "aclMatchExternal: acl=\"" << acl->def->name << "\"");
+    debugs(82, 9, HERE << "acl=\"" << acl->def->name << "\"");
     entry = ch->extacl_entry;
 
     if (entry) {
-        if (cbdataReferenceValid(entry) && entry->def == acl->def &&
-                strcmp((char *)entry->key, key) == 0) {
+        if (cbdataReferenceValid(entry) && entry->def == acl->def) {
             /* Ours, use it.. */
         } else {
             /* Not valid, or not ours.. get rid of it */
+            debugs(82, 9, HERE << "entry " << entry << " not valid or not ours. Discarded.");
+            if (entry) {
+                debugs(82, 9, HERE << "entry def=" << entry->def << ", our def=" << acl->def);
+                key = makeExternalAclKey(ch, acl);
+                debugs(82, 9, HERE << "entry key='" << (char *)entry->key << "', our key='" << key << "'");
+            }
             cbdataReferenceDone(ch->extacl_entry);
             entry = NULL;
         }
@@ -743,6 +748,7 @@ aclMatchExternal(external_acl_data *acl, ACLFilledChecklist *ch)
     external_acl_message = "MISSING REQUIRED INFORMATION";
 
     if (!entry) {
+        debugs(82, 9, HERE << "No helper entry available");
         if (acl->def->require_auth) {
             int ti;
             /* Make sure the user is authenticated */
@@ -850,6 +856,10 @@ ACLExternal::dump() const
 static void
 external_acl_cache_touch(external_acl * def, external_acl_entry * 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))
+        return;
+
     dlinkDelete(&entry->lru, &def->lru_list);
     dlinkAdd(entry, &entry->lru, &def->lru_list);
 }
@@ -1091,6 +1101,9 @@ makeExternalAclKey(ACLFilledChecklist * ch, external_acl_data * acl_data)
 static int
 external_acl_entry_expired(external_acl * def, external_acl_entry * entry)
 {
+    if (def->cache_size <= 0)
+        return 1;
+
     if (entry->date + (entry->result == 1 ? def->ttl : def->negative_ttl) < squid_curtime)
         return 1;
     else
@@ -1100,6 +1113,9 @@ external_acl_entry_expired(external_acl * def, external_acl_entry * entry)
 static int
 external_acl_grace_expired(external_acl * def, external_acl_entry * entry)
 {
+    if (def->cache_size <= 0)
+        return 1;
+
     int ttl;
     ttl = entry->result == 1 ? def->ttl : def->negative_ttl;
     ttl = (ttl * (100 - def->grace)) / 100;
@@ -1113,12 +1129,24 @@ external_acl_grace_expired(external_acl * def, external_acl_entry * entry)
 static external_acl_entry *
 external_acl_cache_add(external_acl * def, const char *key, ExternalACLEntryData const & data)
 {
-    ExternalACLEntry *entry = static_cast<ExternalACLEntry *>(hash_lookup(def->cache, key));
+    ExternalACLEntry *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)) {
+        debugs(82,6, HERE);
+        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) {
         debugs(82, 3, "ExternalACLEntry::update: updating existing entry");
-        entry->update (data);
+        entry->update(data);
         external_acl_cache_touch(def, entry);
 
         return entry;
@@ -1126,7 +1154,7 @@ external_acl_cache_add(external_acl * def, const char *key, ExternalACLEntryData
 
     entry = new ExternalACLEntry;
     entry->key = xstrdup(key);
-    entry->update (data);
+    entry->update(data);
 
     def->add(entry);
 
@@ -1136,7 +1164,7 @@ external_acl_cache_add(external_acl * def, const char *key, ExternalACLEntryData
 static void
 external_acl_cache_delete(external_acl * def, external_acl_entry * entry)
 {
-    assert (entry->def == def);
+    assert(def->cache_size > 0 && entry->def == def);
     hash_remove_link(def->cache, entry);
     dlinkDelete(&entry->lru, &def->lru_list);
     def->cache_entries -= 1;
@@ -1200,7 +1228,7 @@ externalAclHandleReply(void *data, char *reply)
     char *status;
     char *token;
     char *value;
-    char *t;
+    char *t = NULL;
     ExternalACLEntryData entryData;
     entryData.result = 0;
     external_acl_entry *entry = NULL;
@@ -1313,12 +1341,15 @@ ACLExternal::ExternalAclLookup(ACLChecklist *checklist, ACLExternal * me, EAH *
         entry = NULL;
 
     /* Check for a pending lookup to hook into */
-    for (node = def->queue.head; node; node = node->next) {
-        externalAclState *oldstatetmp = static_cast<externalAclState *>(node->data);
+    // only possible if we are caching results.
+    if (def->cache_size > 0) {
+        for (node = def->queue.head; node; node = node->next) {
+            externalAclState *oldstatetmp = static_cast<externalAclState *>(node->data);
 
-        if (strcmp(key, oldstatetmp->key) == 0) {
-            oldstate = oldstatetmp;
-            break;
+            if (strcmp(key, oldstatetmp->key) == 0) {
+                oldstate = oldstatetmp;
+                break;
+            }
         }
     }