This patch fixes the ACLChecklist::fastCheck method so that it can
always be reusable.
On 06/10/2011 09:05 PM, Amos Jeffries wrote:
Some fluffy bits:
"The new methd does not change the internal state of the ACLChecklist
object"
* this is false. The funtion clearly changes currentAnswer().
This is true
* the PROF_* bits used by fastCheck() need to be kept for benchmark
profiling.
OK done.
* can the local cbdata references be made CbcPointer please?
That should clear up all the pointer shuffling.
It was not easy to use CbcPointers because of various
"error: invalid conversion from ‘const ...’ to ..." compile error
messages, which probably requires changes to CbcPointer code.
The problem here is that the ACLChecklist::accessList is const but the
CbcPointer does not support const cbdata classes.
Amos
Fixed bypass of SSL certificate validation errors.
The bypass code was calling ACLChecklist::fastCheck() multiple times
if multiple certificate errors were found. That method should not be
called multiple times because it changes the internal ACLChecklist
state, producing wrong answers for repeated calls.
This patch fixes the ACLChecklist::fastCheck() method so it can be called
multiple times. Each fastCheck() call results in an independent access
list check.
This is a Measurement Factory project
=== modified file 'src/acl/Checklist.cc'
--- src/acl/Checklist.cc 2009-03-08 19:41:27 +0000
+++ src/acl/Checklist.cc 2011-06-13 10:41:59 +0000
@@ -323,60 +323,54 @@
*
* NP: this should probably be made Async now.
*/
void
ACLChecklist::nonBlockingCheck(PF * callback_, void *callback_data_)
{
callback = callback_;
callback_data = cbdataReference(callback_data_);
check();
}
/* Warning: do not cbdata lock this here - it
* may be static or on the stack
*/
int
ACLChecklist::fastCheck()
{
PROF_start(aclCheckFast);
currentAnswer(ACCESS_DENIED);
debugs(28, 5, "aclCheckFast: list: " << accessList);
-
- while (accessList) {
- preCheck();
- matchAclListFast(accessList->aclList);
-
- if (finished()) {
+ const acl_access *acl = cbdataReference(accessList);
+ while (acl != NULL && cbdataReferenceValid(acl)) {
+ currentAnswer(acl->allow);
+ if (matchAclListFast(acl->aclList)) {
PROF_stop(aclCheckFast);
- cbdataReferenceDone(accessList);
+ cbdataReferenceDone(acl);
return currentAnswer() == ACCESS_ALLOWED;
}
/*
* Reference the next access entry
*/
- const acl_access *A = accessList;
-
- assert (A);
-
- accessList = cbdataReference(A->next);
-
+ const acl_access *A = acl;
+ acl = cbdataReference(acl->next);
cbdataReferenceDone(A);
}
debugs(28, 5, "aclCheckFast: no matches, returning: " << (currentAnswer()
== ACCESS_DENIED));
PROF_stop(aclCheckFast);
return currentAnswer() == ACCESS_DENIED;
}
bool
ACLChecklist::checking() const
{
return checking_;
}
void
ACLChecklist::checking (bool const newValue)
{
checking_ = newValue;
=== modified file 'src/acl/Checklist.h'
--- src/acl/Checklist.h 2009-04-07 13:51:57 +0000
+++ src/acl/Checklist.h 2011-06-13 08:07:23 +0000
@@ -85,41 +85,41 @@
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.
*/
void nonBlockingCheck(PF * callback, void *callback_data);
/**
* Trigger a blocking access check for a set of *_access options.
*
* 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 DENIED.
+ * 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 1/true Access Allowed
* \retval 0/false Access Denied
*/
int fastCheck();
/**
* Trigger a blocking access check for a single ACL line (a AND b AND c).
*
* 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.
*
* \retval 1/true Access Allowed
* \retval 0/false Access Denied
*/
bool matchAclListFast(const ACLList * list);
