Work in progress to fix various ACL bugs.

This project attempts to fix several ACL-related bugs, including:

    # broken when "goodGuys" matches (denies good guys)
    acl_driven_option deny !goodGuys

and

    # broken if badGuys fails to match or mismatch (allows bad guys)
    acl_driven_option allow !badGuys

Fixing the above resulted in significant changes (and more fixes!)
detailed below.

 * Revised ACLChecklist::fastCheck() and nonBlockingCheck() APIs to
   clarify all possible outcomes and to specify that exceptional ACL
   check outcomes (not ALLOW or DENIED) are not ignored/skipped but
   result in the same exceptional final answer. I believe this is the
   right behavior even if it is going to break some [already broken
   IMHO] existing configurations. Skipping failed ACLs is insecure and
   may lead to confusing results.

 * Correctly handle cases where no rules were matched and, hence, the
   keyword/action of the last seen rule (if any) has to be "reversed".

 * Do not ignore non-allow/deny outcomes of rules in fastCheck().

 * Move away from setting the "default" (and usually wrong) "current"
   answer and then sometimes adjusting it. Set the answer only when we
   know what it is. This is done using markFinished() call which now
   accepts the [final] answer value and debugging reason for selecting
   that answer.

 * Streamline and better document ACLChecklist::matchAclList()
   interface.  Use it in a more consistent fashion.

 * Rewrote ACLChecklist::matchAclList() implementation when it comes to
   handling ACLList::matches() outcomes. Better document and restrict
   supported outcomes. Assert on unsupported outcomes (for now).

 * Removed ACLChecklist::lastACLResult(). It was doing nothing but
   duplicating nodeMatched value as far as I could tell.

 * Removed ProxyAuthNeeded class. It is an async state that does not
   perform async operations and, hence, is not needed.

 * Move IdentLookup::checkForAsync() connection check into
   ACLIdent::match() to avoid creating an async state that is not
   needed.

 * Polished aclMatchExternal() and greatly simplified
   ACLExternal::ExternalAclLookup() to avoid creating async state under
   non-async conditions, to avoid checking for the same conditions
   twice, to fix wrong debugging messages, and to simplify (and possibly
   fix) the overall algorithm.

   The aclMatchExternal() call now checks most of the corner cases,
   discards stale cached entries, and schedules either a background
   cache update or a regular external lookup as needed.

   ACLExternal::ExternalAclLookup() code is now
   ExternalACLLookup::Start().  It initiates an external lookup. It does
   not deal with the cached entry at all. It relies on
   aclMatchExternal() to check various preconditions.

   Some of the old code made little sense to me, and this is the biggest
   ACL-specific change in this project, with the highest probability of
   new bugs or unintended side-effects.

   My goal here was to prevent aclMatchExternal() from creating an async
   state where none was needed because new ACLChecklist::matchAclList()
   code prohibited such self-contradictory outcomes. However, I later
   discovered that it is not possible to prohibit them without rewriting
   how Squid DNS cache lookups are working -- ipcache_nbgethostbyname()
   and similar code may call back immediately if the item is in the
   cache. Since I did not want to rewrite DNS code as well, I ended up
   relaxing the ACLChecklist::matchAclList() code requirements, going a
   step back to where we sometimes call ACLList::matches() twice for the
   same ACL node.

   Thus, it is probably possible to undo most of the external_acl.cc
   changes.  I left them in because I think they improve the quality of
   the code and possibly fix a bug or two.

 * Adjusted ACLMaxUserIP::match(), ACLProxyAuth::match(), and
   ACLExternal::match() to explicitly stop ACL processing when an
   exceptional state is discovered instead of just setting the current
   answer and proceeding as if more ACLs could be checked. On the other
   hand, we now do not set the answer if the corresponding internal
   matching code (e.g., AuthenticateAcl()) needs an async operation
   because we do not know the answer yet.

 * Fixed HttpStateData::handle1xx() and
   HttpStateData::finishingBrokenPost() to correctly handle
   fastCheck(void) return values. They were assuming that there are only
   two possible return values (ACCESS_DENIED/ALLOWED), potentially
   subjecting more messages to invasive adaptations than necessary.

TODO: 

 * Rename currentAnswer() to finalAnswer(). We probably never change the
   "current" answer any more.

 * More testing.

 * Trunk port.

=== modified file 'src/ExternalACL.h'
--- src/ExternalACL.h	2009-03-08 19:34:36 +0000
+++ src/ExternalACL.h	2012-05-30 22:34:54 +0000
@@ -19,66 +19,68 @@
  *  the Free Software Foundation; either version 2 of the License, or
  *  (at your option) any later version.
  *
  *  This program is distributed in the hope that it will be useful,
  *  but WITHOUT ANY WARRANTY; without even the implied warranty of
  *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  *  GNU General Public License for more details.
  *
  *  You should have received a copy of the GNU General Public License
  *  along with this program; if not, write to the Free Software
  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111, USA.
  *
  *
  * Copyright (c) 2003, Robert Collins <robertc@squid-cache.org>
  */
 #ifndef SQUID_EXTERNALACL_H
 #define SQUID_EXTERNALACL_H
 
 #include "acl/Checklist.h"
 
-class external_acl;
+/** \todo CLEANUP: kill this typedef. */
+typedef struct _external_acl_data external_acl_data;
 
 class ExternalACLLookup : public ACLChecklist::AsyncState
 {
 
 public:
     static ExternalACLLookup *Instance();
     virtual void checkForAsync(ACLChecklist *)const;
 
+    // If possible, starts an asynchronous lookup of an external ACL.
+    // Otherwise, asserts (or bails if background refresh is requested).
+    static void Start(ACLChecklist *checklist, external_acl_data *acl, bool bg);
+
 private:
     static ExternalACLLookup instance_;
     static void LookupDone(void *data, void *result);
 };
 
-/** \todo CLEANUP: kill this typedef. */
-typedef struct _external_acl_data external_acl_data;
-
 #include "acl/Acl.h"
 
 class ACLExternal : public ACL
 {
 
 public:
     MEMPROXY_CLASS(ACLExternal);
 
-    static void ExternalAclLookup(ACLChecklist * ch, ACLExternal *, EAH * callback, void *callback_data);
+    static void ExternalAclLookup(ACLChecklist * ch, ACLExternal *);
 
 
     ACLExternal(char const *);
     ACLExternal(ACLExternal const &);
     ~ACLExternal();
     ACLExternal&operator=(ACLExternal const &);
 
     virtual ACL *clone()const;
     virtual char const *typeString() const;
     virtual void parse();
     virtual int match(ACLChecklist *checklist);
     /* This really should be dynamic based on the external class defn */
     virtual bool requiresRequest() const {return true;}
 
     /* when requiresRequest is made dynamic, review this too */
     //    virtual bool requiresReply() const {return true;}
     virtual bool isProxyAuth() const;
     virtual wordlist *dump() const;
     virtual bool valid () const;
     virtual bool empty () const;

=== modified file 'src/acl/Acl.cc'
--- src/acl/Acl.cc	2012-04-25 05:29:20 +0000
+++ src/acl/Acl.cc	2012-05-30 22:45:20 +0000
@@ -305,50 +305,56 @@
     if (!checklist->hasRequest() && requiresRequest()) {
         debugs(28, 1, "ACL::checklistMatches WARNING: '" << name << "' ACL is used but there is no HTTP request -- not matching.");
         return 0;
     }
 
     if (!checklist->hasReply() && requiresReply()) {
         debugs(28, 1, "ACL::checklistMatches WARNING: '" << name << "' ACL is used but there is no HTTP reply -- not matching.");
         return 0;
     }
 
     debugs(28, 3, "ACL::checklistMatches: checking '" << name << "'");
     rv= match(checklist);
     debugs(28, 3, "ACL::ChecklistMatches: result for '" << name << "' is " << rv);
     return rv;
 }
 
 bool
 ACLList::matches (ACLChecklist *checklist) const
 {
     assert (_acl);
+    // XXX: AclMatchedName does not contain a matched ACL name when the acl
+    // does not match (or contains stale name if no ACLs are checked). In 
+    // either case, we get misleading debugging and possibly incorrect error
+    // messages. Unfortunately, deny_info's "when none http_access
+    // lines match" exception essentially requires this mess.
+    // TODO: Rework by using an acl-free deny_info for the no-match cases?
     AclMatchedName = _acl->name;
     debugs(28, 3, "ACLList::matches: checking " << (op ? null_string : "!") << _acl->name);
 
     if (_acl->checklistMatches(checklist) != op) {
         debugs(28, 4, "ACLList::matches: result is false");
-        return checklist->lastACLResult(false);
+        return false;
     }
 
     debugs(28, 4, "ACLList::matches: result is true");
-    return checklist->lastACLResult(true);
+    return true;
 }
 
 
 /*********************/
 /* Destroy functions */
 /*********************/
 
 ACL::~ACL()
 {
     debugs(28, 3, "ACL::~ACL: '" << cfgline << "'");
     safe_free(cfgline);
 }
 
 /* to be split into separate files in the future */
 
 CBDATA_CLASS_INIT(acl_access);
 
 void *
 acl_access::operator new (size_t)
 {

=== modified file 'src/acl/Checklist.cc'
--- src/acl/Checklist.cc	2012-05-04 22:20:07 +0000
+++ src/acl/Checklist.cc	2012-05-31 17:12:32 +0000
@@ -25,249 +25,320 @@
  *  but WITHOUT ANY WARRANTY; without even the implied warranty of
  *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  *  GNU General Public License for more details.
  *
  *  You should have received a copy of the GNU General Public License
  *  along with this program; if not, write to the Free Software
  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111, USA.
  *
  * Copyright (c) 2003, Robert Collins <robertc@squid-cache.org>
  */
 
 #include "squid-old.h"
 #include "acl/Checklist.h"
 
 void
 ACLChecklist::matchNonBlocking()
 {
     if (checking())
         return;
 
-    /** Deny if no rules present. */
-    currentAnswer(ACCESS_DENIED);
-
     if (callerGone()) {
-        checkCallback(currentAnswer());
+        checkCallback(ACCESS_DUNNO); // the answer does not really matter
         return;
     }
 
     /** The ACL List should NEVER be NULL when calling this method.
      * Always caller should check for NULL and handle appropriate to its needs first.
      * We cannot select a sensible default for all callers here. */
     if (accessList == NULL) {
         debugs(28, DBG_CRITICAL, "SECURITY ERROR: ACL " << this << " checked with nothing to match against!!");
-        currentAnswer(ACCESS_DENIED);
-        checkCallback(currentAnswer());
+        checkCallback(ACCESS_DUNNO);
         return;
     }
 
+    allow_t lastSeenKeyword = ACCESS_DUNNO;
     /* NOTE: This holds a cbdata reference to the current access_list
      * entry, not the whole list.
      */
     while (accessList != NULL) {
         /** \par
          * If the _acl_access is no longer valid (i.e. its been
-         * freed because of a reconfigure), then bail on this
-         * access check.  For now, return ACCESS_DENIED.
+         * freed because of a reconfigure), then bail with ACCESS_DUNNO.
          */
 
         if (!cbdataReferenceValid(accessList)) {
             cbdataReferenceDone(accessList);
             debugs(28, 4, "ACLChecklist::check: " << this << " accessList is invalid");
-            continue;
+            checkCallback(ACCESS_DUNNO);
+            return;
         }
 
         checking (true);
         checkAccessList();
         checking (false);
 
         if (asyncInProgress()) {
             return;
         }
 
         if (finished()) {
             /** \par
              * Either the request is allowed, denied, requires authentication.
              */
             debugs(28, 3, "ACLChecklist::check: " << this << " match found, calling back with " << currentAnswer());
             cbdataReferenceDone(accessList); /* A */
             checkCallback(currentAnswer());
             /* From here on in, this may be invalid */
             return;
         }
 
+        lastSeenKeyword = accessList->allow;
+
         /*
          * Reference the next access entry
          */
         const acl_access *A = accessList;
 
         assert (A);
 
         accessList = cbdataReference(A->next);
 
         cbdataReferenceDone(A);
     }
 
-    /** If dropped off the end of the list return inversion of last line allow/deny action. */
-    debugs(28, 3, HERE << this << " NO match found, returning " <<
-           (currentAnswer() != ACCESS_DENIED ? ACCESS_DENIED : ACCESS_ALLOWED));
+    calcImplicitAnswer(lastSeenKeyword);
+    checkCallback(currentAnswer());
+}
 
-    checkCallback(currentAnswer() != ACCESS_DENIED ? ACCESS_DENIED : ACCESS_ALLOWED);
+bool
+ACLChecklist::asyncNeeded() const
+{
+    return state_ != NullState::Instance();
 }
 
 bool
 ACLChecklist::asyncInProgress() const
 {
     return async_;
 }
 
 void
 ACLChecklist::asyncInProgress(bool const newAsync)
 {
     assert (!finished() && !(asyncInProgress() && newAsync));
     async_ = newAsync;
     debugs(28, 3, "ACLChecklist::asyncInProgress: " << this <<
            " async set to " << async_);
 }
 
 bool
 ACLChecklist::finished() const
 {
     return finished_;
 }
 
 void
-ACLChecklist::markFinished()
+ACLChecklist::markFinished(const allow_t &finalAnswer, const char *reason)
 {
     assert (!finished() && !asyncInProgress());
     finished_ = true;
-    debugs(28, 3, "ACLChecklist::markFinished: " << this <<
-           " checklist processing finished");
+    allow_ = finalAnswer;
+    debugs(28, 3, HERE << this << " answer " << allow_ << " for " << reason);
 }
 
+/// Called first (and once) by all checks to initialize their state
 void
-ACLChecklist::preCheck()
+ACLChecklist::preCheck(const char *what)
 {
-    debugs(28, 3, "ACLChecklist::preCheck: " << this << " checking '" << accessList->cfgline << "'");
-    /* what is our result on a match? */
-    currentAnswer(accessList->allow);
+    debugs(28, 3, HERE << this << " checking " << what);
+    finished_ = false;
 }
 
 void
 ACLChecklist::checkAccessList()
 {
-    preCheck();
+    debugs(28, 3, HERE << this << " checking '" << accessList->cfgline << "'");
     /* does the current AND clause match */
-    matchAclList(accessList->aclList, false);
+    if (matchAclList(accessList->aclList, false))
+        markFinished(accessList->allow, "first matching rule won");
+
+    // If we are not finished() here, the caller must distinguish between
+    // slow async calls and pure rule mismatches using asyncInProgress().
 }
 
 void
 ACLChecklist::checkForAsync()
 {
     asyncState()->checkForAsync(this);
 }
 
 // ACLFilledChecklist overwrites this to unclock something before we
 // "delete this"
 void
 ACLChecklist::checkCallback(allow_t answer)
 {
     ACLCB *callback_;
     void *cbdata_;
     debugs(28, 3, "ACLChecklist::checkCallback: " << this << " answer=" << answer);
 
     callback_ = callback;
     callback = NULL;
 
     if (cbdataReferenceValidDone(callback_data, &cbdata_))
         callback_(answer, cbdata_);
 
     delete this;
 }
 
-void
+/// An ACLChecklist::matchNodes() wrapper to simplify profiling.
+bool
 ACLChecklist::matchAclList(const ACLList * head, bool const fast)
 {
+    // TODO: remove by using object con/destruction-based PROF_* macros.
     PROF_start(aclMatchAclList);
-    const ACLList *node = head;
+    const bool result = matchNodes(head, fast);
+    PROF_stop(aclMatchAclList);
+    return result;
+}
 
-    finished_ = false;
+/** Returns true if and only if there was a match. If false is returned:
+    finished() indicates an error or exception of some kind, while
+    !finished() means there was a mismatch or an allowed slow async call.
+    If async calls are allowed (i.e. 'fast' was false), then those last
+    two cases can be distinguished using asyncInProgress().
+*/
+bool
+ACLChecklist::matchNodes(const ACLList * head, bool const fast)
+{
+    assert(!finished());
 
-    while (node) {
-        bool nodeMatched = node->matches(this);
+    for (const ACLList *node = head; node; node = node->next) {
 
-        if (fast)
-            changeState(NullState::Instance());
+        const NodeMatchingResult resultBeforeAsync = matchNode(*node, fast);
 
-        if (finished()) {
-            PROF_stop(aclMatchAclList);
-            return;
-        }
+        if (resultBeforeAsync == nmrMatch)
+            continue;
+        
+        if (resultBeforeAsync == nmrMismatch || resultBeforeAsync == nmrFinished)
+            return false;
+
+        assert(resultBeforeAsync == nmrNeedsAsync);
+
+        // Ideally, this should be inside match() itself, but that requires
+        // prohibiting slow ACLs in options that do not support them.
+        // TODO: rename to maybeStartAsync()?
+        checkForAsync();
+
+        // Some match() code claims that an async lookup is needed, but then
+        // fails to start an async lookup when given a chance. We catch such
+        // cases here and call matchNode() again, hoping that some cached data
+        // prevents us from going async again.
+        // This is inefficient and ugly, but fixing all match() code, including
+        // the code it calls, such as ipcache_nbgethostbyname(), takes time.
+        if (!asyncInProgress()) { // failed to start an async operation
+            
+            if (finished()) {
+                debugs(28, 3, HERE << this << " finished after failing to go async: " << currentAnswer());
+                return false; // an exceptional case
+            }
 
-        if (!nodeMatched || state_ != NullState::Instance()) {
+            const NodeMatchingResult resultAfterAsync = matchNode(*node, true);
+            // the second call disables slow checks so we cannot go async again
+            assert(resultAfterAsync != nmrNeedsAsync);
+            if (resultAfterAsync == nmrMatch)
+                continue;
 
-            bool async = state_ != NullState::Instance();
+            assert(resultAfterAsync == nmrMismatch || resultAfterAsync == nmrFinished);
+            return false;
+        }
 
-            checkForAsync();
+        assert(!finished()); // async operation is truly asynchronous
+        debugs(28, 3, HERE << this << " awaiting async operation");
+        return false;
+    }
 
-            bool async_in_progress = asyncInProgress();
-            debugs(28, 3, "aclmatchAclList: async=" << (async ? 1 : 0) <<
-                   " nodeMatched=" << (nodeMatched ? 1 : 0) <<
-                   " async_in_progress=" << (async_in_progress ? 1 : 0) <<
-                   " lastACLResult() = " << (lastACLResult() ? 1 : 0) <<
-                   " finished() = " << finished());
+    debugs(28, 3, HERE << this << " success: all ACLs matched");
+    return true;
+}
 
-            if (finished()) {
-                debugs(28, 3, "aclmatchAclList: " << this << " returning (AND list entry failed to match)");
-                PROF_stop(aclMatchAclList);
-                return;
-            }
 
-            if (async && nodeMatched && !asyncInProgress() && lastACLResult()) {
-                // async acl, but using cached response, and it was a match
-                node = node->next;
-                continue;
-            }
+/// Check whether a single ACL matches, returning NodeMatchingResult
+ACLChecklist::NodeMatchingResult
+ACLChecklist::matchNode(const ACLList &node, bool const fast)
+{
+    const bool nodeMatched = node.matches(this);
+    const bool needsAsync = asyncNeeded();
+    const bool matchFinished = finished();
+
+    debugs(28, 3, HERE << this <<
+           " matched=" << nodeMatched <<
+           " async=" << needsAsync <<
+           " finished=" << matchFinished);
+
+    /* There are eight possible outcomes of the matches() call based on
+       (matched, async, finished) permutations. We support these four:
+       matched,!async,!finished: a match (must check next rule node)
+       !matched,!async,!finished: a mismatch (whole rule fails to match)
+       !matched,!async,finished: error or special condition (propagate)
+       !matched,async,!finished: ACL needs to make an async call (pause)
+     */
 
-            debugs(28, 3, "aclmatchAclList: " << this << " returning (AND list entry awaiting an async lookup)");
-            PROF_stop(aclMatchAclList);
-            return;
-        }
+    if (nodeMatched) {
+        // matches() should return false in all special cases
+        assert(!needsAsync && !matchFinished);
+        return nmrMatch;
+    }
 
-        node = node->next;
+    if (matchFinished) {
+        // we cannot be done and need an async call at the same time
+        assert(!needsAsync);
+        debugs(28, 3, HERE << this << " exception: " << currentAnswer());
+        return nmrFinished;
+     }
+
+    if (!needsAsync) {
+        debugs(28, 3, HERE << this << " simple mismatch");
+        return nmrMismatch;
     }
 
-    debugs(28, 3, "aclmatchAclList: " << this << " returning true (AND list satisfied)");
+    /* we need an async call */
 
-    markFinished();
-    PROF_stop(aclMatchAclList);
+    if (fast) {
+        changeState(NullState::Instance()); // disable async checks
+        markFinished(ACCESS_DUNNO, "async required but prohibited");
+        debugs(28, 3, HERE << this << " DUNNO because cannot async");
+        return nmrFinished;
+    }
+
+    debugs(28, 3, HERE << this << " going async");
+    return nmrNeedsAsync;
 }
 
 ACLChecklist::ACLChecklist() :
         accessList (NULL),
         callback (NULL),
         callback_data (NULL),
         async_(false),
         finished_(false),
         allow_(ACCESS_DENIED),
-        state_(NullState::Instance()),
-        lastACLResult_(false)
+        state_(NullState::Instance())
 {
 }
 
 ACLChecklist::~ACLChecklist()
 {
     assert (!asyncInProgress());
 
     cbdataReferenceDone(accessList);
 
     debugs(28, 4, "ACLChecklist::~ACLChecklist: destroyed " << this);
 }
 
 
 void
 ACLChecklist::AsyncState::changeState (ACLChecklist *checklist, AsyncState *newState) const
 {
     checklist->changeState(newState);
 }
 
 ACLChecklist::NullState *
@@ -291,90 +362,117 @@
      * RBC 02 2003
      */
     assert (state_ == NullState::Instance() || newState == NullState::Instance());
     state_ = newState;
 }
 
 ACLChecklist::AsyncState *
 ACLChecklist::asyncState() const
 {
     return state_;
 }
 
 /**
  * Kick off a non-blocking (slow) ACL access list test
  *
  * NP: this should probably be made Async now.
  */
 void
 ACLChecklist::nonBlockingCheck(ACLCB * callback_, void *callback_data_)
 {
+    preCheck("nonBlocking");
     callback = callback_;
     callback_data = cbdataReference(callback_data_);
     matchNonBlocking();
 }
 
 allow_t const &
 ACLChecklist::fastCheck(const ACLList * list)
 {
     PROF_start(aclCheckFast);
-    currentAnswer(ACCESS_DUNNO);
-    matchAclList(list, true);
-    // assume ALLOWED on matches due to not having an acl_access object
-    if (finished())
-        currentAnswer(ACCESS_ALLOWED);
+
+    preCheck("fast single-rule");
+
+    // assume DENY/ALLOW on mis/matches due to not having acl_access object
+    if (matchAclList(list, true))
+        markFinished(ACCESS_ALLOWED, "all ACLs matched");
+    else
+    if (!finished())
+        markFinished(ACCESS_DENIED, "ACL mismatched");
     PROF_stop(aclCheckFast);
     return currentAnswer();
 }
 
 /* Warning: do not cbdata lock this here - it
  * may be static or on the stack
  */
 allow_t const &
 ACLChecklist::fastCheck()
 {
     PROF_start(aclCheckFast);
-    currentAnswer(ACCESS_DUNNO);
 
+    preCheck("fast multi-rule");
+
+    allow_t lastSeenKeyword = ACCESS_DUNNO;
     debugs(28, 5, "aclCheckFast: list: " << accessList);
     const acl_access *acl = cbdataReference(accessList);
     while (acl != NULL && cbdataReferenceValid(acl)) {
-        matchAclList(acl->aclList, true);
+        // on a match, finish
+        if (matchAclList(acl->aclList, true))
+            markFinished(acl->allow, "first matching rule won");
+
+        // if finished (on a match or in exceptional cases), stop
         if (finished()) {
-            currentAnswer(acl->allow);
-            PROF_stop(aclCheckFast);
             cbdataReferenceDone(acl);
+            PROF_stop(aclCheckFast);
             return currentAnswer();
         }
 
-        /*
-         * Reference the next access entry
-         */
+        // on a mismatch, try the next access rule
+        lastSeenKeyword = acl->allow;
         const acl_access *A = acl;
         acl = cbdataReference(acl->next);
         cbdataReferenceDone(A);
     }
 
-    debugs(28, 5, "aclCheckFast: no matches, returning: " << currentAnswer());
+    // There were no rules to match or no rules matched
+    calcImplicitAnswer(lastSeenKeyword);
     PROF_stop(aclCheckFast);
 
     return currentAnswer();
 }
 
+/// When no rules matched, the answer is the inversion of the last seen rule
+/// action (or ACCESS_DUNNO if the reversal is not possible). The caller 
+/// should set lastSeenAction to ACCESS_DUNNO if there were no rules to see.
+void
+ACLChecklist::calcImplicitAnswer(const allow_t &lastSeenAction)
+{
+    allow_t implicitRuleAnswer = ACCESS_DUNNO;
+    if (lastSeenAction == ACCESS_DENIED) // reverse last seen "deny"
+        implicitRuleAnswer = ACCESS_ALLOWED;
+    else if (lastSeenAction == ACCESS_ALLOWED) // reverse last seen "allow"
+        implicitRuleAnswer = ACCESS_DENIED;
+    // else we saw no rules and will respond with ACCESS_DUNNO
+
+    debugs(28, 3, HERE << this << " NO match found, last action " <<
+           lastSeenAction << " so returning " << implicitRuleAnswer);
+    markFinished(implicitRuleAnswer, "implicit rule won");
+}
 
 bool
 ACLChecklist::checking() const
 {
     return checking_;
 }
 
 void
 ACLChecklist::checking (bool const newValue)
 {
     checking_ = newValue;
 }
 
 bool
 ACLChecklist::callerGone()
 {
     return !cbdataReferenceValid(callback_data);
 }

=== modified file 'src/acl/Checklist.h'
--- src/acl/Checklist.h	2012-05-04 22:20:07 +0000
+++ src/acl/Checklist.h	2012-05-31 17:54:31 +0000
@@ -75,118 +75,155 @@
     };
 
     class NullState : public AsyncState
     {
 
     public:
         static NullState *Instance();
         virtual void checkForAsync(ACLChecklist *) const;
         virtual ~NullState() {}
 
     private:
         static NullState _instance;
     };
 
 
 public:
     ACLChecklist();
     virtual ~ACLChecklist();
 
     /**
-     * Trigger off a non-blocking access check for a set of *_access options..
-     * The callback specified will be called with true/false
-     * when the results of the ACL tests are known.
+     * Start a non-blocking (async) check for a list of allow/deny rules.
+     * Each rule comes with a list of ACLs.
+     *
+     * The callback specified will be called with the result of the check.
+     *
+     * The first rule where all ACLs match wins. If there is such a rule,
+     * the result becomes that rule keyword (ACCESS_ALLOWED or ACCESS_DENIED).
+     *
+     * If there are rules but all ACL lists mismatch, an implicit rule is used.
+     * Its result is the negation of the keyword of the last seen rule.
+     *
+     * Some ACLs may stop the check prematurely by setting an exceptional
+     * check result (e.g., ACCESS_AUTH_REQUIRED) instead of declaring a
+     * match or mismatch.
+     *
+     * If there are no rules to check at all, the result becomes ACCESS_DUNNO.
+     * Calling this method with no rules to check wastes a lot of CPU cycles
+     * and will result in a DBG_CRITICAL debugging message.
      */
     void nonBlockingCheck(ACLCB * callback, void *callback_data);
 
     /**
-     * Trigger a blocking access check for a set of *_access options.
+     * Perform a blocking (immediate) check for a list of allow/deny rules.
+     * Each rule comes with a list of ACLs.
+     *
+     * The first rule where all ACLs match wins. If there is such a rule,
+     * the result becomes that rule keyword (ACCESS_ALLOWED or ACCESS_DENIED).
+     *
+     * If there are rules but all ACL lists mismatch, an implicit rule is used
+     * Its result is the negation of the keyword of the last seen rule.
+     *
+     * Some ACLs may stop the check prematurely by setting an exceptional
+     * check result (e.g., ACCESS_AUTH_REQUIRED) instead of declaring a
+     * match or mismatch.
      *
-     * ACLs which cannot be satisfied directly from available data are ignored.
-     * This means any proxy_auth, external_acl, DNS lookups, Ident lookups etc
-     * which have not already been performed and cached will not be checked.
-     *
-     * If there is no access list to check the default is to return ALLOWED.
-     * However callers should perform their own check and default based on local
-     * knowledge of the ACL usage rather than depend on this default.
-     * That will also save on work setting up ACLChecklist fields for a no-op.
-     *
-     * \retval ACCESS_DUNNO     Unable to determine any result
-     * \retval ACCESS_ALLOWED   Access Allowed
-     * \retval ACCESS_DENIED    Access Denied
+     * Some ACLs may require an async lookup which is prohibited by this
+     * method. In this case, the exceptional check result of ACCESS_DUNNO is
+     * immediately returned.
+     *
+     * If there are no rules to check at all, the result becomes ACCESS_DUNNO.
      */
     allow_t const & fastCheck();
 
     /**
-     * A version of fastCheck() for use when there is a one-line set of ACLs
-     * to be tested and a match determins the result action to be done.
+     * Perform a blocking (immediate) check whether a list of ACLs matches.
+     * This method is meant to be used with squid.conf ACL-driven options that
+     * lack allow/deny keywords and are tested one ACL list at a time. Whether
+     * the checks for other occurrences of the same option continue after this
+     * call is up to the caller and option semantics.
+     *
+     * If all ACLs match, the result becomes ACCESS_ALLOWED.
+     *
+     * If all ACLs mismatch, the result becomes ACCESS_DENIED.
+     *
+     * Some ACLs may stop the check prematurely by setting an exceptional
+     * check result (e.g., ACCESS_AUTH_REQUIRED) instead of declaring a
+     * match or mismatch.
      *
-     * \retval ACCESS_DUNNO     Unable to determine any result
-     * \retval ACCESS_ALLOWED   ACLs all matched
+     * Some ACLs may require an async lookup which is prohibited by this
+     * method. In this case, the exceptional check result of ACCESS_DUNNO is
+     * immediately returned.
+     *
+     * If there are no ACLs to check at all, the result becomes ACCESS_ALLOWED.
      */
     allow_t const & fastCheck(const ACLList * list);
 
+    // whether the last checked ACL of the current rule needs
+    // an async operation to determine whether there was a match
+    bool asyncNeeded() const;
     bool asyncInProgress() const;
     void asyncInProgress(bool const);
 
+    /// whether markFinished() was called
     bool finished() const;
-    void markFinished();
+    /// called when no more ACLs should be checked; sets the final answer and
+    /// prints a debugging message explaining the reason for that answer
+    void markFinished(const allow_t &newAnswer, const char *reason);
 
     const allow_t &currentAnswer() const { return allow_; }
-    void currentAnswer(const allow_t &newAnswer) { allow_ = newAnswer; }
 
     void changeState(AsyncState *);
     AsyncState *asyncState() const;
 
     // XXX: ACLs that need request or reply have to use ACLFilledChecklist and
     // should do their own checks so that we do not have to povide these two
     // for ACL::checklistMatches to use
     virtual bool hasRequest() const = 0;
     virtual bool hasReply() const = 0;
 
 protected:
     virtual void checkCallback(allow_t answer);
 private:
     void checkAccessList();
     void checkForAsync();
 
 public:
     const acl_access *accessList;
 
     ACLCB *callback;
     void *callback_data;
 
     /**
-     * Attempt to check the current checklist against current data.
-     * This is the core routine behind all ACL test routines.
-     * As much as possible of current tests are performed immediately
-     * and the result is maybe delayed to wait for async lookups.
-     *
-     * When all tests are done callback is presented with one of:
-     *  - ACCESS_ALLOWED     Access explicitly Allowed
-     *  - ACCESS_DENIED      Access explicitly Denied
+     * Performs non-blocking check starting with the current rule.
+     * Used by nonBlockingCheck() to initiate the checks and by
+     * async operation callbacks to resume checks after the async
+     * operation updates the current Squid state. See nonBlockingCheck()
+     * for details on final result determination.
      */
     void matchNonBlocking();
 
 private: /* internal methods */
-    void preCheck();
-    void matchAclList(const ACLList * list, bool const fast);
+    /// possible outcomes when trying to match a single ACL node in a list
+    typedef enum { nmrMatch, nmrMismatch, nmrFinished, nmrNeedsAsync }
+        NodeMatchingResult;
+
+    /// prepare for checking ACLs; called once per check
+    void preCheck(const char *what);
+    bool matchAclList(const ACLList * list, bool const fast);
+    bool matchNodes(const ACLList * head, bool const fast);
+    NodeMatchingResult matchNode(const ACLList &node, bool const fast);
+    void calcImplicitAnswer(const allow_t &lastSeenAction);
 
     bool async_;
     bool finished_;
     allow_t allow_;
     AsyncState *state_;
 
     bool checking_;
     bool checking() const;
     void checking (bool const);
 
-    bool lastACLResult_;
     bool callerGone();
-
-public:
-    bool lastACLResult(bool x) { return lastACLResult_ = x; }
-
-    bool lastACLResult() const { return lastACLResult_; }
 };
 
 #endif /* SQUID_ACLCHECKLIST_H */

=== modified file 'src/auth/Acl.cc'
--- src/auth/Acl.cc	2012-01-20 18:55:04 +0000
+++ src/auth/Acl.cc	2012-05-23 21:30:22 +0000
@@ -40,34 +40,37 @@
         /* Proxy authorization on proxy requests */
         headertype = HDR_PROXY_AUTHORIZATION;
     }
 
     /* get authed here */
     /* Note: this fills in auth_user_request when applicable */
     const AuthAclState result = Auth::UserRequest::tryToAuthenticateAndSetAuthUser(
                                     &checklist->auth_user_request, headertype, request,
                                     checklist->conn(), checklist->src_addr);
     switch (result) {
 
     case AUTH_ACL_CANNOT_AUTHENTICATE:
         debugs(28, 4, HERE << "returning " << ACCESS_DENIED << " user authenticated but not authorised.");
         return ACCESS_DENIED;
 
     case AUTH_AUTHENTICATED:
         return ACCESS_ALLOWED;
         break;
 
     case AUTH_ACL_HELPER:
-        debugs(28, 4, HERE << "returning " << ACCESS_DENIED << " sending credentials to helper.");
+        debugs(28, 4, HERE << "returning " << ACCESS_DUNNO << " sending credentials to helper.");
         checklist->changeState(ProxyAuthLookup::Instance());
         return ACCESS_DUNNO; // XXX: break this down into DUNNO, EXPIRED_OK, EXPIRED_BAD states
 
     case AUTH_ACL_CHALLENGE:
-        debugs(28, 4, HERE << "returning " << ACCESS_DENIED << " sending authentication challenge.");
-        checklist->changeState(ProxyAuthNeeded::Instance());
+        debugs(28, 4, HERE << "returning " << ACCESS_AUTH_REQUIRED << " sending authentication challenge.");
+        /* Client is required to resend the request with correct authentication
+         * credentials. (This may be part of a stateful auth protocol.)
+         * The request is denied.
+         */
         return ACCESS_AUTH_REQUIRED;
 
     default:
         fatal("unexpected authenticateAuthenticate reply\n");
         return ACCESS_DENIED;
     }
 }

=== modified file 'src/auth/AclMaxUserIp.cc'
--- src/auth/AclMaxUserIp.cc	2012-05-14 10:37:40 +0000
+++ src/auth/AclMaxUserIp.cc	2012-05-31 17:10:12 +0000
@@ -134,63 +134,60 @@
         /* remove _this_ ip, as it is the culprit for going over the limit */
         authenticateAuthUserRequestRemoveIp(auth_user_request, src_addr);
         debugs(28, 4, "aclMatchUserMaxIP: Denying access in strict mode");
     } else {
         /*
          * non-strict - remove some/all of the cached entries
          * ie to allow the user to move machines easily
          */
         authenticateAuthUserRequestClearIp(auth_user_request);
         debugs(28, 4, "aclMatchUserMaxIP: Denying access in non-strict mode - flushing the user ip cache");
     }
 
     return 1;
 }
 
 int
 ACLMaxUserIP::match(ACLChecklist *cl)
 {
     ACLFilledChecklist *checklist = Filled(cl);
     allow_t answer = AuthenticateAcl(checklist);
-    if (answer != ACCESS_DENIED && answer != ACCESS_ALLOWED) {
-        // If the answer is not allowed or denied (matches/not matches), requires 
-        // authentication (ACCESS_AUTH_*) or the authentication is in progress (ACCESS_DUNNO)
-        // so change the state in the related checklist.
-        checklist->currentAnswer(answer);
-    }
-
     int ti;
 
     // convert to tri-state ACL match 1,0,-1
     switch (answer) {
     case ACCESS_ALLOWED:
         // check for a match
         ti = match(checklist->auth_user_request, checklist->src_addr);
         checklist->auth_user_request = NULL;
         return ti;
 
     case ACCESS_DENIED:
         return 0; // non-match
 
     case ACCESS_DUNNO:
     case ACCESS_AUTH_REQUIRED:
     default:
+        // If the answer is not allowed or denied (matches/not matches) and
+        // async authentication is not needed (asyncNeeded), then we are done.
+        if (!checklist->asyncNeeded())
+            checklist->markFinished(answer, "AuthenticateAcl exception");
         return -1; // other
     }
 }
 
 wordlist *
 ACLMaxUserIP::dump() const
 {
     if (!maximum)
         return NULL;
 
     wordlist *W = NULL;
 
     if (flags.strict)
         wordlistAdd(&W, "-s");
 
     char buf[128];
 
     snprintf(buf, sizeof(buf), "%lu", (unsigned long int) maximum);
 
     wordlistAdd(&W, buf);

=== modified file 'src/auth/AclProxyAuth.cc'
--- src/auth/AclProxyAuth.cc	2012-05-14 10:37:40 +0000
+++ src/auth/AclProxyAuth.cc	2012-05-31 17:10:32 +0000
@@ -63,99 +63,88 @@
     type_ = rhs.type_;
     return *this;
 }
 
 char const *
 ACLProxyAuth::typeString() const
 {
     return type_;
 }
 
 void
 ACLProxyAuth::parse()
 {
     data->parse();
 }
 
 int
 ACLProxyAuth::match(ACLChecklist *checklist)
 {
     allow_t answer = AuthenticateAcl(checklist);
-    if (answer != ACCESS_DENIED && answer != ACCESS_ALLOWED) {
-        // If the answer is not allowed or denied (matches/not matches), requires 
-        // authentication (ACCESS_AUTH_*) or the authentication is in progress (ACCESS_DUNNO)
-        // so change the state in the related checklist.
-        checklist->currentAnswer(answer);
-    }
-
     // convert to tri-state ACL match 1,0,-1
     switch (answer) {
     case ACCESS_ALLOWED:
         // check for a match
         return matchProxyAuth(checklist);
 
     case ACCESS_DENIED:
         return 0; // non-match
 
     case ACCESS_DUNNO:
     case ACCESS_AUTH_REQUIRED:
     default:
+        // If the answer is not allowed or denied (matches/not matches) and
+        // async authentication is not needed (asyncNeeded), then we are done.
+        if (!checklist->asyncNeeded())
+            checklist->markFinished(answer, "AuthenticateAcl exception");
         return -1; // other
     }
 }
 
 wordlist *
 ACLProxyAuth::dump() const
 {
     return data->dump();
 }
 
 bool
 ACLProxyAuth::empty () const
 {
     return data->empty();
 }
 
 bool
 ACLProxyAuth::valid () const
 {
     if (authenticateSchemeCount() == 0) {
         debugs(28, 0, "Can't use proxy auth because no authentication schemes were compiled.");
         return false;
     }
 
     if (authenticateActiveSchemeCount() == 0) {
         debugs(28, 0, "Can't use proxy auth because no authentication schemes are fully configured.");
         return false;
     }
 
     return true;
 }
 
-ProxyAuthNeeded ProxyAuthNeeded::instance_;
-
-ProxyAuthNeeded *
-ProxyAuthNeeded::Instance()
-{
-    return &instance_;
-}
-
 ProxyAuthLookup ProxyAuthLookup::instance_;
 
 ProxyAuthLookup *
 ProxyAuthLookup::Instance()
 {
     return &instance_;
 }
 
 void
 ProxyAuthLookup::checkForAsync(ACLChecklist *cl)const
 {
     ACLFilledChecklist *checklist = Filled(cl);
 
     checklist->asyncInProgress(true);
     debugs(28, 3, HERE << "checking password via authenticator");
 
     /* make sure someone created auth_user_request for us */
     assert(checklist->auth_user_request != NULL);
     assert(checklist->auth_user_request->valid());
     checklist->auth_user_request->start(LookupDone, checklist);
@@ -170,53 +159,40 @@
 
     if (result != NULL)
         fatal("AclLookupProxyAuthDone: Old code floating around somewhere.\nMake clean and if that doesn't work, report a bug to the squid developers.\n");
 
     if (checklist->auth_user_request == NULL || !checklist->auth_user_request->valid() || checklist->conn() == NULL) {
         /* credentials could not be checked either way
          * restart the whole process */
         /* OR the connection was closed, there's no way to continue */
         checklist->auth_user_request = NULL;
 
         if (checklist->conn() != NULL) {
             checklist->conn()->auth_user_request = NULL;
         }
     }
 
     checklist->asyncInProgress(false);
     checklist->changeState (ACLChecklist::NullState::Instance());
     checklist->matchNonBlocking();
 }
 
-void
-ProxyAuthNeeded::checkForAsync(ACLChecklist *checklist) const
-{
-    /* Client is required to resend the request with correct authentication
-     * credentials. (This may be part of a stateful auth protocol.)
-     * The request is denied.
-     */
-    debugs(28, 6, "ACLChecklist::checkForAsync: requiring Proxy Auth header.");
-    checklist->currentAnswer(ACCESS_AUTH_REQUIRED);
-    checklist->changeState (ACLChecklist::NullState::Instance());
-    checklist->markFinished();
-}
-
 ACL *
 ACLProxyAuth::clone() const
 {
     return new ACLProxyAuth(*this);
 }
 
 int
 ACLProxyAuth::matchForCache(ACLChecklist *cl)
 {
     ACLFilledChecklist *checklist = Filled(cl);
     assert (checklist->auth_user_request != NULL);
     return data->match(checklist->auth_user_request->username());
 }
 
 /* aclMatchProxyAuth can return two exit codes:
  * 0 : Authorisation for this ACL failed. (Did not match)
  * 1 : Authorisation OK. (Matched)
  */
 int
 ACLProxyAuth::matchProxyAuth(ACLChecklist *cl)

=== modified file 'src/auth/AclProxyAuth.h'
--- src/auth/AclProxyAuth.h	2011-02-07 10:27:53 +0000
+++ src/auth/AclProxyAuth.h	2012-05-23 21:30:52 +0000
@@ -36,51 +36,40 @@
 #define SQUID_ACLPROXYAUTH_H
 
 #if USE_AUTH
 
 #include "acl/Acl.h"
 #include "acl/Data.h"
 #include "acl/Checklist.h"
 
 class ProxyAuthLookup : public ACLChecklist::AsyncState
 {
 
 public:
     static ProxyAuthLookup *Instance();
     virtual void checkForAsync(ACLChecklist *)const;
 
 private:
     static ProxyAuthLookup instance_;
     static void LookupDone(void *data, char *result);
 };
 
-class ProxyAuthNeeded : public ACLChecklist::AsyncState
-{
-
-public:
-    static ProxyAuthNeeded *Instance();
-    virtual void checkForAsync(ACLChecklist *)const;
-
-private:
-    static ProxyAuthNeeded instance_;
-};
-
 class ACLProxyAuth : public ACL
 {
 
 public:
     MEMPROXY_CLASS(ACLProxyAuth);
 
     ~ACLProxyAuth();
     ACLProxyAuth(ACLData<char const *> *, char const *);
     ACLProxyAuth (ACLProxyAuth const &);
     ACLProxyAuth &operator= (ACLProxyAuth const &);
 
     virtual char const *typeString() const;
     virtual void parse();
     virtual bool isProxyAuth() const {return true;}
 
     virtual int match(ACLChecklist *checklist);
     virtual wordlist *dump() const;
     virtual bool valid () const;
     virtual bool empty () const;
     virtual bool requiresRequest() const {return true;}

=== modified file 'src/external_acl.cc'
--- src/external_acl.cc	2012-05-14 10:37:40 +0000
+++ src/external_acl.cc	2012-05-31 17:11:39 +0000
@@ -788,114 +788,132 @@
             }
         } 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;
         }
     }
 
     external_acl_message = "MISSING REQUIRED INFORMATION";
 
     if (!entry) {
         debugs(82, 9, HERE << "No helper entry available");
 #if USE_AUTH
         if (acl->def->require_auth) {
-            int ti = AuthenticateAcl(ch);
             /* Make sure the user is authenticated */
             debugs(82, 3, HERE << acl->def->name << " check user authenticated.");
-            if (ti != 1) {
+            const allow_t ti = AuthenticateAcl(ch);
+            if (ti != ACCESS_ALLOWED) {
                 debugs(82, 2, HERE << acl->def->name << " user not authenticated (" << ti << ")");
-                return ACCESS_AUTH_REQUIRED;
+                return ti;
             }
             debugs(82, 3, HERE << acl->def->name << " user is authenticated.");
         }
 #endif
         key = makeExternalAclKey(ch, acl);
 
         if (!key) {
             /* Not sufficient data to process */
             return ACCESS_DUNNO;
         }
 
         entry = static_cast<external_acl_entry *>(hash_lookup(acl->def->cache, key));
 
-        if (!entry || external_acl_grace_expired(acl->def, entry)) {
+        external_acl_entry *staleEntry = entry;
+        if (entry && external_acl_entry_expired(acl->def, entry))
+            entry = NULL;
+
+        if (entry && external_acl_grace_expired(acl->def, entry)) {
+            // refresh in the background
+            ExternalACLLookup::Start(ch, acl, true);
+            debugs(82, 4, HERE << "no need to wait for the refresh of '" <<
+                   key << "' in '" << acl->def->name << "' (ch=" << ch << ").");
+        }
+
+        if (!entry) {
             debugs(82, 2, HERE << acl->def->name << "(\"" << key << "\") = lookup needed");
             debugs(82, 2, HERE << "\"" << key << "\": entry=@" <<
                    entry << ", age=" << (entry ? (long int) squid_curtime - entry->date : 0));
 
             if (acl->def->theHelper->stats.queue_size <= (int)acl->def->theHelper->childs.n_active) {
                 debugs(82, 2, HERE << "\"" << key << "\": queueing a call.");
                 ch->changeState(ExternalACLLookup::Instance());
                 debugs(82, 2, HERE << "\"" << key << "\": return -1.");
-                return ACCESS_DUNNO; // to get here we have to have an expired cache entry. MUST not use.
+                return ACCESS_DUNNO; // expired cached or simply absent entry
             } else {
-                if (!entry) {
+                if (!staleEntry) {
                     debugs(82, DBG_IMPORTANT, "WARNING: external ACL '" << acl->def->name <<
                            "' queue overload. Request rejected '" << key << "'.");
                     external_acl_message = "SYSTEM TOO BUSY, TRY AGAIN LATER";
                     return ACCESS_DUNNO;
                 } else {
                     debugs(82, DBG_IMPORTANT, "WARNING: external ACL '" << acl->def->name <<
                            "' queue overload. Using stale result. '" << key << "'.");
+                    entry = staleEntry;
                     /* Fall thru to processing below */
                 }
             }
         }
     }
 
+    debugs(82, 4, HERE << "entry = { date=" <<
+           (long unsigned int) entry->date <<
+           ", result=" << entry->result <<
+           " tag=" << entry->tag <<
+           " log=" << entry->log << " }");
+#if USE_AUTH
+    debugs(82, 4, HERE << "entry user=" << entry->user);
+#endif
+
     external_acl_cache_touch(acl->def, entry);
     external_acl_message = entry->message.termedBuf();
 
     debugs(82, 2, HERE << acl->def->name << " = " << entry->result);
     copyResultsFromEntry(ch->request, entry);
     return entry->result;
 }
 
 int
 ACLExternal::match(ACLChecklist *checklist)
 {
     allow_t answer = aclMatchExternal(data, Filled(checklist));
-    if (answer != ACCESS_DENIED && answer != ACCESS_ALLOWED) {
-        // If the answer is not allowed or denied (matches/not matches), requires 
-        // authentication (ACCESS_AUTH_*) or the authentication is in progress (ACCESS_DUNNO)
-        // so change the state in the related checklist.
-        checklist->currentAnswer(answer);
-    }
-
     // convert to tri-state ACL match 1,0,-1
     switch (answer) {
     case ACCESS_ALLOWED:
         return 1; // match
 
     case ACCESS_DENIED:
         return 0; // non-match
 
     case ACCESS_DUNNO:
     case ACCESS_AUTH_REQUIRED:
     default:
+        // If the answer is not allowed or denied (matches/not matches) and
+        // async authentication is not needed (asyncNeeded), then we are done.
+        if (!checklist->asyncNeeded())
+            checklist->markFinished(answer, "aclMatchExternal exception");
         return -1; // other
     }
 }
 
 wordlist *
 ACLExternal::dump() const
 {
     external_acl_data const *acl = data;
     wordlist *result = NULL;
     wordlist *arg;
     MemBuf mb;
     mb.init();
     mb.Printf("%s", acl->def->name);
 
     for (arg = acl->arguments; arg; arg = arg->next) {
         mb.Printf(" %s", arg->key);
     }
 
     wordlistAdd(&result, mb.buf);
     mb.clean();
@@ -1353,177 +1371,119 @@
                 external_acl_cache_delete(state->def, oldentry);
         }
     }
 
     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, EAH * callback, void *callback_data)
+ACLExternal::ExternalAclLookup(ACLChecklist *checklist, ACLExternal * me)
 {
-    MemBuf buf;
-    external_acl_data *acl = me->data;
-    external_acl *def = acl->def;
-    externalAclState *state;
-    dlink_node *node;
-    externalAclState *oldstate = NULL;
-    bool graceful = 0;
+    ExternalACLLookup::Start(checklist, me->data, false);
+}
 
+void
+ExternalACLLookup::Start(ACLChecklist *checklist, external_acl_data *acl, bool inBackground)
+{
+    external_acl *def = acl->def;
+ 
     ACLFilledChecklist *ch = Filled(checklist);
-#if USE_AUTH
-    if (acl->def->require_auth) {
-        int ti;
-        /* Make sure the user is authenticated */
-        debugs(82, 3, HERE << acl->def->name << " check user authenticated.");
-
-        if ((ti = AuthenticateAcl(ch)) != 1) {
-            debugs(82, DBG_IMPORTANT, "WARNING: " << acl->def->name <<
-                   " user authentication failure (" << ti << ", ch=" << ch << ")");
-            callback(callback_data, NULL);
-            return;
-        }
-        debugs(82, 3, HERE << acl->def->name << " user is authenticated.");
-    }
-#endif
-
     const char *key = makeExternalAclKey(ch, acl);
+    assert(key);
 
-    if (!key) {
-        debugs(82, 1, "externalAclLookup: lookup in '" << def->name <<
-               "', prerequisit failure (ch=" << ch << ")");
-        callback(callback_data, NULL);
-        return;
-    }
-
-    debugs(82, 2, "externalAclLookup: lookup in '" << def->name << "' for '" << key << "'");
-
-    external_acl_entry *entry = static_cast<external_acl_entry *>(hash_lookup(def->cache, key));
-
-    if (entry && external_acl_entry_expired(def, entry))
-        entry = NULL;
+    debugs(82, 2, HERE << (inBackground ? "bg" : "fg") << " lookup in '" <<
+           def->name << "' for '" << key << "'");
 
     /* Check for a pending lookup to hook into */
     // only possible if we are caching results.
+    externalAclState *oldstate = NULL;
     if (def->cache_size > 0) {
-        for (node = def->queue.head; node; node = node->next) {
+        for (dlink_node *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 (entry && external_acl_grace_expired(def, entry)) {
-        if (oldstate) {
-            debugs(82, 4, "externalAclLookup: in grace period, but already pending lookup ('" << key << "', ch=" << ch << ")");
-            callback(callback_data, entry);
-            return;
-        } else {
-            graceful = 1; // grace expired, (neg)ttl did not, and we must start a new lookup.
-        }
-    }
-
-    // The entry is in the cache, grace_ttl did not expired.
-    if (!graceful && entry && !external_acl_grace_expired(def, entry)) {
-        /* Should not really happen, but why not.. */
-        callback(callback_data, entry);
-        debugs(82, 4, "externalAclLookup: no lookup pending for '" << key << "', and grace not expired");
-        debugs(82, 4, "externalAclLookup: (what tha' hell?)");
+    // A background refresh has no need to piggiback on a pending request:
+    // When the pending request completes, the cache will be refreshed anyway.
+    if (oldstate && inBackground) {
+        debugs(82, 7, HERE << "'" << def->name << "' queue is already being refreshed (ch=" << ch << ")");
         return;
     }
 
-    /* No pending lookup found. Sumbit to helper */
-    state = cbdataAlloc(externalAclState);
-
+    externalAclState *state = cbdataAlloc(externalAclState);
     state->def = cbdataReference(def);
 
     state->key = xstrdup(key);
 
-    if (!graceful) {
-        state->callback = callback;
-        state->callback_data = cbdataReference(callback_data);
+    if (!inBackground) {
+        state->callback = &ExternalACLLookup::LookupDone;
+        state->callback_data = cbdataReference(checklist);
     }
 
     if (oldstate) {
         /* Hook into pending lookup */
         state->queue = oldstate->queue;
         oldstate->queue = state;
     } else {
+        /* No pending lookup found. Sumbit to helper */
+
         /* Check for queue overload */
 
         if (def->theHelper->stats.queue_size >= (int)def->theHelper->childs.n_running) {
-            debugs(82, 1, "externalAclLookup: '" << def->name << "' queue overload (ch=" << ch << ")");
+            debugs(82, 7, HERE << "'" << def->name << "' queue is too long");
+            assert(inBackground); // or the caller should have checked
             cbdataFree(state);
-            callback(callback_data, entry);
             return;
         }
 
         /* Send it off to the helper */
+        MemBuf buf;
         buf.init();
 
         buf.Printf("%s\n", key);
 
         debugs(82, 4, "externalAclLookup: looking up for '" << key << "' in '" << def->name << "'.");
 
         helperSubmit(def->theHelper, buf.buf, externalAclHandleReply, state);
 
         dlinkAdd(state, &state->list, &def->queue);
 
         buf.clean();
     }
 
-    if (graceful) {
-        /* No need to wait during grace period */
-        debugs(82, 4, "externalAclLookup: no need to wait for the result of '" <<
-               key << "' in '" << def->name << "' (ch=" << ch << ").");
-        debugs(82, 4, "externalAclLookup: using cached entry " << entry);
-
-        if (entry != NULL) {
-            debugs(82, 4, "externalAclLookup: entry = { date=" <<
-                   (long unsigned int) entry->date <<
-                   ", result=" << entry->result <<
-                   " tag=" << entry->tag <<
-                   " log=" << entry->log << " }");
-#if USE_AUTH
-            debugs(82, 4, "externalAclLookup: user=" << entry->user);
-#endif
-            copyResultsFromEntry(ch->request, entry);
-        }
-
-        callback(callback_data, entry);
-        return;
-    }
-
     debugs(82, 4, "externalAclLookup: will wait for the result of '" << key <<
            "' in '" << def->name << "' (ch=" << ch << ").");
 }
 
 static void
 externalAclStats(StoreEntry * sentry)
 {
     external_acl *p;
 
     for (p = Config.externalAclHelperList; p; p = p->next) {
         storeAppendPrintf(sentry, "External ACL Statistics: %s\n", p->name);
         storeAppendPrintf(sentry, "Cache size: %d\n", p->cache->count);
         helperStats(sentry, p->theHelper);
         storeAppendPrintf(sentry, "\n");
     }
 }
 
 static void
 externalAclRegisterWithCacheManager(void)
 {
@@ -1574,43 +1534,44 @@
     }
 }
 
 ExternalACLLookup ExternalACLLookup::instance_;
 ExternalACLLookup *
 ExternalACLLookup::Instance()
 {
     return &instance_;
 }
 
 void
 ExternalACLLookup::checkForAsync(ACLChecklist *checklist)const
 {
     /* TODO: optimise this - we probably have a pointer to this
      * around somewhere */
     ACL *acl = ACL::FindByName(AclMatchedName);
     assert(acl);
     ACLExternal *me = dynamic_cast<ACLExternal *> (acl);
     assert (me);
     checklist->asyncInProgress(true);
-    ACLExternal::ExternalAclLookup(checklist, me, LookupDone, checklist);
+    ACLExternal::ExternalAclLookup(checklist, me);
 }
 
+/// Called when an async lookup returns
 void
 ExternalACLLookup::LookupDone(void *data, void *result)
 {
     ACLFilledChecklist *checklist = Filled(static_cast<ACLChecklist*>(data));
     checklist->extacl_entry = cbdataReference((external_acl_entry *)result);
     checklist->asyncInProgress(false);
     checklist->changeState (ACLChecklist::NullState::Instance());
     checklist->matchNonBlocking();
 }
 
 /* This registers "external" in the registry. To do dynamic definitions
  * of external ACL's, rather than a static prototype, have a Prototype instance
  * prototype in the class that defines each external acl 'class'.
  * Then, then the external acl instance is created, it self registers under
  * it's name.
  * Be sure that clone is fully functional for that acl class though!
  */
 ACL::Prototype ACLExternal::RegistryProtoype(&ACLExternal::RegistryEntry_, "external");
 
 ACLExternal ACLExternal::RegistryEntry_("external");

=== modified file 'src/http.cc'
--- src/http.cc	2012-05-14 10:37:40 +0000
+++ src/http.cc	2012-05-31 16:46:50 +0000
@@ -728,41 +728,41 @@
 void
 HttpStateData::handle1xx(HttpReply *reply)
 {
     HttpMsgPointerT<HttpReply> msg(reply); // will destroy reply if unused
 
     // one 1xx at a time: we must not be called while waiting for previous 1xx
     Must(!flags.handling1xx);
     flags.handling1xx = true;
 
     if (!request->canHandle1xx()) {
         debugs(11, 2, HERE << "ignoring client-unsupported 1xx");
         proceedAfter1xx();
         return;
     }
 
 #if USE_HTTP_VIOLATIONS
     // check whether the 1xx response forwarding is allowed by squid.conf
     if (Config.accessList.reply) {
         ACLFilledChecklist ch(Config.accessList.reply, originalRequest(), NULL);
         ch.reply = HTTPMSGLOCK(reply);
-        if (!ch.fastCheck()) { // TODO: support slow lookups?
+        if (ch.fastCheck() != ACCESS_ALLOWED) { // TODO: support slow lookups?
             debugs(11, 3, HERE << "ignoring denied 1xx");
             proceedAfter1xx();
             return;
         }
     }
 #endif // USE_HTTP_VIOLATIONS
 
     debugs(11, 2, HERE << "forwarding 1xx to client");
 
     // the Sink will use this to call us back after writing 1xx to the client
     typedef NullaryMemFunT<HttpStateData> CbDialer;
     const AsyncCall::Pointer cb = JobCallback(11, 3, CbDialer, this,
                                   HttpStateData::proceedAfter1xx);
     CallJobHere1(11, 4, request->clientConnectionManager, ConnStateData,
                  ConnStateData::sendControlMsg, HttpControlMsg(msg, cb));
     // If the call is not fired, then the Sink is gone, and HttpStateData
     // will terminate due to an aborted store entry or another similar error.
     // If we get stuck, it is not handle1xx fault if we could get stuck
     // for similar reasons without a 1xx response.
 }
@@ -2174,41 +2174,41 @@
     statCounter.server.http.requests++;
 
     /*
      * We used to set the read timeout here, but not any more.
      * Now its set in httpSendComplete() after the full request,
      * including request body, has been written to the server.
      */
 }
 
 /// if broken posts are enabled for the request, try to fix and return true
 bool
 HttpStateData::finishingBrokenPost()
 {
 #if USE_HTTP_VIOLATIONS
     if (!Config.accessList.brokenPosts) {
         debugs(11, 5, HERE << "No brokenPosts list");
         return false;
     }
 
     ACLFilledChecklist ch(Config.accessList.brokenPosts, originalRequest(), NULL);
-    if (!ch.fastCheck()) {
+    if (ch.fastCheck() != ACCESS_ALLOWED) {
         debugs(11, 5, HERE << "didn't match brokenPosts");
         return false;
     }
 
     if (!Comm::IsConnOpen(serverConnection)) {
         debugs(11, 3, HERE << "ignoring broken POST for closed " << serverConnection);
         assert(closeHandler != NULL);
         return true; // prevent caller from proceeding as if nothing happened
     }
 
     debugs(11, 3, "finishingBrokenPost: fixing broken POST");
     typedef CommCbMemFunT<HttpStateData, CommIoCbParams> Dialer;
     requestSender = JobCallback(11,5,
                                 Dialer, this, HttpStateData::wroteLast);
     Comm::Write(serverConnection, "\r\n", 2, requestSender, NULL);
     return true;
 #else
     return false;
 #endif /* USE_HTTP_VIOLATIONS */
 }

=== modified file 'src/ident/AclIdent.cc'
--- src/ident/AclIdent.cc	2012-01-20 18:55:04 +0000
+++ src/ident/AclIdent.cc	2012-05-31 17:11:27 +0000
@@ -72,87 +72,88 @@
 
 void
 ACLIdent::parse()
 {
     if (!data) {
         debugs(28, 3, HERE << "current is null. Creating");
         data = new ACLUserData;
     }
 
     data->parse();
 }
 
 int
 ACLIdent::match(ACLChecklist *cl)
 {
     ACLFilledChecklist *checklist = Filled(cl);
     if (checklist->rfc931[0]) {
         return data->match(checklist->rfc931);
     } else if (checklist->conn() != NULL && checklist->conn()->clientConnection != NULL && checklist->conn()->clientConnection->rfc931[0]) {
         return data->match(checklist->conn()->clientConnection->rfc931);
-    } else {
+    } else if (checklist->conn() != NULL && Comm::IsConnOpen(checklist->conn()->clientConnection)) {
         debugs(28, 3, HERE << "switching to ident lookup state");
         checklist->changeState(IdentLookup::Instance());
         return 0;
+    } else {
+        debugs(28, DBG_IMPORTANT, HERE << "Can't start ident lookup. No client connection" );
+        checklist->markFinished(ACCESS_DUNNO, "cannot start ident lookup");
+        return -1;
     }
 }
 
 wordlist *
 ACLIdent::dump() const
 {
     return data->dump();
 }
 
 bool
 ACLIdent::empty () const
 {
     return data->empty();
 }
 
 ACL *
 ACLIdent::clone() const
 {
     return new ACLIdent(*this);
 }
 
 
 IdentLookup IdentLookup::instance_;
 
 IdentLookup *
 IdentLookup::Instance()
 {
     return &instance_;
 }
 
 void
 IdentLookup::checkForAsync(ACLChecklist *cl)const
 {
     ACLFilledChecklist *checklist = Filled(cl);
-    if (checklist->conn() != NULL && Comm::IsConnOpen(checklist->conn()->clientConnection)) {
+    const ConnStateData *conn = checklist->conn();
+    // check that ACLIdent::match() tested this lookup precondition
+    assert(conn && Comm::IsConnOpen(conn->clientConnection));
         debugs(28, 3, HERE << "Doing ident lookup" );
         checklist->asyncInProgress(true);
         Ident::Start(checklist->conn()->clientConnection, LookupDone, checklist);
-    } else {
-        debugs(28, DBG_IMPORTANT, "IdentLookup::checkForAsync: Can't start ident lookup. No client connection" );
-        checklist->currentAnswer(ACCESS_DENIED);
-        checklist->markFinished();
-    }
 }
 
 void
 IdentLookup::LookupDone(const char *ident, void *data)
 {
     ACLFilledChecklist *checklist = Filled(static_cast<ACLChecklist*>(data));
     assert(checklist->asyncState() == IdentLookup::Instance());
 
     if (ident) {
         xstrncpy(checklist->rfc931, ident, USER_IDENT_SZ);
     } else {
         xstrncpy(checklist->rfc931, dash_str, USER_IDENT_SZ);
     }
 
     /*
      * Cache the ident result in the connection, to avoid redoing ident lookup
      * over and over on persistent connections
      */
     if (checklist->conn() != NULL && checklist->conn()->clientConnection != NULL && !checklist->conn()->clientConnection->rfc931[0])
         xstrncpy(checklist->conn()->clientConnection->rfc931, checklist->rfc931, USER_IDENT_SZ);