[squid-dev] [RFC] Where do ACL flags belong?

[Alex Rousskov]

On 12/04/2015 06:26 AM, Amos Jeffries wrote:
> On 4/12/2015 7:51 p.m., Alex Rousskov wrote:
>> To illustrate this even better, you need to add more lines and values:
>>
>>   acl foo dstdomain v1
>>   acl foo dstdomain -n v2a v2b
>>   acl foo dstdomain v3
>>
>> is interpreted after #1 changes as
>>
>>   acl foo1  dstdomain v1
>>   acl foo2a dstdomain -n v2a
>>   acl foo2b dstdomain -n v2b
>>   acl foo3  dstdomain v3
>>   acl foo anyof foo1 foo2a foo2b foo3
>>
>> Does this match what you think we should do?


> Yes.


>> If yes, what to do about
>> backward compatibility? Break it because -n is not that widely used?
> 
> Yes. 

Noted.


> With a +n flag (akin to the +i, re-enabling what -n disabled) we
> can follow -i/+i pattern with an auto-reset at the end of each line but
> add a warning if the +n is omitted on the next one.

I am not against adding +n support, but I do not recommend forcing users
to add +n, just like we do not force them to add +i.

Now for flags in-between acl values on the same line:

  acl old dstname foo -n bar -x baz

I think we should prohibit general flags between same-line values (-i
support preserved for backward compatibility, but I would probably warn
about it anyway). Such support does not really add much value AFAICT,
but the resulting ACL lines become more and more difficult for a human
to interpret correctly because each flag "area" overlaps subsequent
flags on the same line. KISS.

General flags in front of lines loaded from an acl parameter file should
be supported.


To protect admins from typos and such, we need to prohibit non-quoted
acl values that

a) start with a dash followed by a single character (-x and -x=foo)

and possibly (for future flags)

b) start with two dashes followed by one or more characters (--long and
--long=foo)

Same for those starting with a + sign.

We can start with deprecation warnings (except for +n which will be
mis-interpreted in old configurations read by newer Squid, I guess).


Would you recommend any changes to the above approach?


> The internal implementation of that could be a node listing values to
> check pre-rDNS and a node listing values only checked post-rDNS.

Most likely, this fix will not be done using multiple ACL nodes because
each ACL class object is meant to combine everything about a single ACL
named foo. We could add more "fake" or "implicit" ACL objects to handle
this, but I think it would be better to change ACLData instead. That's
how I would start anyway.


> Could this be something completed quickly enough for Squid-4?

IMO, this change can always go into Squid-4 because it is essentially a
bug fix. If Factory does the work, and accounting for current overload
and upcoming holidays, we can probably deliver #1 patch for review by
the end of January 2016, but this is an estimate and not a promise. Do
you want Factory to work on this?


Thank you,

Alex.