Re: [PATCH] AND and OR ACLs

On 12/21/2012 02:21 PM, Tsantilas Christos wrote:
> On 12/21/2012 08:58 PM, Alex Rousskov wrote:
>> Christos,
>>
>> I wonder if we can avoid code duplication by moving existing AND and
>> OR logic from Checklist into a new ACL node type and then _always_ using
>> that node type to wrap _all_ ACL rules? It would be kind of the opposite
>> of what you have done: You are wrapping existing nodes into Checklist
>> rule when an AND/OR ACL is found. This solution would wrap ALL rules
>> into an AND or OR ACL node while Checklist will always check just _one_
>> ACL node (usually AND or OR).
>>
>> In other words, this solution would automatically transfer
>>
>> http_access allow a1 a2 a3
>> http_access allow b1 b2 b3
>>
>> into
>>
>> acl autoA all-of a1 a2 a3
>> acl autoB all-of b1 b2 b3
>> acl autoAB any-of autoA autoB
>> http_access allow autoAB
>>
>> We would need to store the allow/deny keyword with the and/or node to
>> make this work, but I did not show that detail in the above example in
>> hope to avoid further confusion.
>>
>> Do you see what I am getting at?
>
> I think yes...
> I don't now how easy is to implement it but it may worth it if we decide
> that the any-of and all-of needed (Personally I am seeing many advantages).
>
> The only concerns I have is that I prefer a one-to-one interpretation of
> squid.conf acls and acls related C++ structure. Interpreting the
> http_access rules to an other scheme may confuse development, more bugs,
> more difficult to solve problems etc...

Yes, if we want to implement AND/OR grouping logic in one class, then we
would have to use that class for all AND/OR groups, both on the ACL node
level (no keywords) and on the ACL rules level (with keywords). This may
be a little confusing/complex indeed.

Another option is to duplicate AND/OR processing for each of those two
levels. I am not against that, at least as a starting point, if no
better solution is found. Once implemented, we can see exactly what code
got duplicated and decide whether we can somehow reduce that duplication.

Yet another option is to place level-agnostic, "pure" AND/OR processing
into some abstract class that can handle any level (ACL node or ACL
rule), but I would not do that because it would probably end up being
even more confusing/complex.

Cheers,

Alex.
Received on Fri Dec 21 2012 - 22:11:00 MST