On 29/06/11 00:23, Amos Jeffries wrote:
In summary:
* use nonBlockingCheck() or fastCheck() to test ACLs.
* be prepared to handle any allow_t in the result.
ACL testing functions publicly available from ACLChecklist are:
- nonBlockingCheck (public), fastCheck public), check (public but not to
be used)
- matchAclListFast (public), matchAclListSlow (private), matchAclList
(private).
Given that there are only two types of test performed, this array of API
methods has been causing confusion and mistakes for some developers.
This patch seeks to clarify that API by correcting a flaw in the naming
of check() and matchAclListFast().
Due to "Fast" ACLs coming in two types there are two overloaded
fastCheck() functions. Now with identical output behaviour. Both return
the allow_t result of the lookup. This is expected to _usually_ be
ACCESS_ALLOWED/ACCESS_DENIED but that is not always the case. Callers
need to be written with consideration that the set of enum results may
change.
- fastCheck(), no parameters, when a full set of "Fast" *_access lines
are to be scanned. The checklist constructor accepts the list to be
scanned. This is the old fastCheck(), with the new ALLOWED/DENIED/DUNNO
result.
- fastCheck(list), one parameter, when a single-line set of ACLs is to
be scanned. This is the old matchAclListFast(), with the new
ALLOWED/DENIED/DUNNO result. Will return ALLOWED whenever the whole set
of ACLs matches. Other results may vary.
- nonBlockingCheck() - for "Slow" non-blocking lookups with asynchronous
callback handler. NP: not touched by this patch.
The output change from boolean to allow_t is due to the fastCheck
callers mixed set of needs allow/deny/other which boolean cannot meet.
Mapping that tri-state need to a boolean result has led to inconsistent
cases of fastCheck() producing unusual values for "true". Sometimes
wrongly for the caller.
Added result lookup type ACCESS_DUNNO, to indicate a test was unable to
be completed BUT there was no allow/deny/auth-required resulting.
Alters all previous calling code to use the new fastCheck() API output.
Some have been polished up to boolean where appropriate instead of
relying on integer values.
Removes matchAclListFast/matchAclListSlow,
Renames check() to matchNonBlocking;
all match*() functions are internal operations during ACL testing.
FUTURE WORK:
* update check() to send allow_t to its callbacks as well.
* update remaining messy callers from using int(0 or 1) types to bool or
allow_t as appropriate.
Amos
Jenkins found two build errors in this patch and the check() callbacks
turned out to all be using int to receive allow_t values anyway. So
complete the future work with zero logic changes.
Applied the sum result since this seems to have passed the 10-day
criteria no complaints.
If anyone is actually interested in a relatively small but useful job
there are at least two polishing cleanups now possible:
1) splitting the ACCESS_REQ_PROXY_AUTH state result into several,
indicating the presence/absence and state of credentials. ie auth okay,
auth failed, credentials required, expired was okay, expired was failed.
2) merging the prefer_direct, always_direct and never_direct access
control lists into one direct_access lookup that returns states PREFER,
ALWAYS, NEVER, ALLOWED, DENIED.
Amos
--
Please be using
Current Stable Squid 2.7.STABLE9 or 3.1.14
Beta testers wanted for 3.2.0.9
