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

[Eliezer Croitoru]

Sorry to bump into the discussion without too much warnings.

On 04/12/2015 08:51, Alex Rousskov wrote:
>
> The -n approach is overall better than _str suffix approach because the
> former makes it easy to mix-and-match multiple flags and to support
> flags with values such as -m=delimiters.
>

The only real differences that I can understand(after almost 24 hours 
straight no sleep) is that admins are used to use "-n" in many unix tools.
Squid used the "-n" for a specific reason which might be wrong.

>>>> >>>AFAICS it should not be much more difficult to
>>>> >>>re-implement -n using #1 than to re-implement regex parsing and -i to
>>>> >>>implement #2.
>>> >>
>>> >>True, but I do not think we should touch the -i interface. It works as
>>> >>expected.
>>> >>
>> >
>> >Then I must have completely misunderstood what your #2 proposal was about.
> #2 issues a warning when -n is used on some but not all lines of a given
> ACL foo. No other changes in #2.

This should be done as a first step before anything else since it's 
really something that the admin should be notified about.
Later we can decide what to do and if required to removed the notice 
after any fix.

>> >  -i flag operates per-line, and resets at the EOL.
> Yes (which, BTW, also breaks the "multiple lines are the same as one
> combined line" design principle).
>
I will stick to the "-n" only for now

>> >If we remove per-line/value flags then a lot of configs are broken.
> Neither #1 nor #2 change how -i works.
>
> #1 makes -n work per line, as intended.

Which sounds great!

> #2 does not change how -n works, but issues a warning if -n is used
> inconsistently across multiple lines.
Which should be pretty simple to implement(leaving aside my C,C++ love 
issues)

>> >I am proposing option #3 be that we have the flag (or ACL type _suffix)
>> >cause parser to generate a similar sub-tree of nodes to what anyof would
>> >generate. BUT using only the specific named type of ACL that the line
>> >was given.
>> >
>> >
>> >So this:
>> >  acl foo dstdomain 192.0.2.1
>> >  acl foo dstdomain -n 192.0.2.3
>> >
>> >or this:
>> >  acl foo dstdomain 192.0.2.1
>> >  acl foo dstdomain_str 192.0.2.3
>> >
>> >
>> >both generates internally a tree identical to:
>> >
>> >   acl foo:1 dstdomain 192.0.2.1
>> >   acl foo:2 dstdomain -n 192.0.2.3
>> >   acl foo anyof foo:1 foo:2
>
> AFAICT this is the same as optimized #1 I have mentioned earlier (except
> for the _suffix part which I think we should avoid as discussed above).
> 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? If yes, what to do about
> backward compatibility? Break it because -n is not that widely used?

The idea above seems somehow reasonable to me but in the other hand I am 
almost sure that there is a mixture of "do we want to stick with -n 
being one thing in almost all unix systems? or choose another path?"
(I have tried to interpret the admins side of the picture from my eyes 
and maybe couple others on the config file.)

As much as I like the "-n" and as much as I do not want to harm running 
config files I am pretty sure that dstdomain_str would be the easiest 
and maybe(since I am not really 100% sure) the less riskier solution 
from the code side of the picture since it will not bring something that 
is actually new. It will only polish couple things.(*...assumption only*)

So +1 for both ideas but another +0.5 to dstdomain_str just because I 
sometimes just don't like -n and all sort of cryptic meanings that 
shortened words or flags have.

Eliezer

> Thank you,
>
> Alex.