Re: [PATCΗ] Quoted values in squid.conf

>> On 05/26/2013 10:26 PM, Amos Jeffries wrote:
>>> Whether macros are supported or not is irrelevant to the string being
>>> de-coded into a single token or split on whitespace.

> On 05/27/2013 07:23 PM, Alex Rousskov wrote:
>> I agree that we can and perhaps should keep runtime %macros handling
>> outside squid.conf tokenization process. If an option supports macros,
>> it can validate their syntax _after_ receiving the token from the
>> parser. This is no different than validating tokens containing integer
>> values or ACL names.
>>
>> One could argue that integrated/centralized value validation is better,
>> but since the current code does not use integrated validation for other
>> value types, perhaps this patch should not introduce it.
>>
>> External ACL and logformat can do something like this:
>>
>> token = ConfigParser::NextToken();
>> handleRuntimeMacros(token);
>>
>> This will keep runtime %macros outside ConfigParser scope. The
>> MacroUser::handleRuntimeMacros() method can organized all the necessary
>> checks in one place, using an already proposed virtual method for
>> validating individual macros. ConfigParser would not know about this though.

On 06/27/2013 12:03 PM, Tsantilas Christos wrote:
> I think that this one it is equivalent to completely remove the %macros
> checking from tokens parsing and remove the MacroUser class from patch.
> Is it OK?

It is not exactly equivalent because MacroUser::handleRuntimeMacros()
provides a centralized macro validation place for ACLs and logformat and
it uses the new MacroUser API for that. All the macro-validation code
remains in one place (just like in your original patch), but that place
is now outside the tokenization code (addressing Amos' concern).

HTH,

Alex.
Received on Thu Jun 27 2013 - 20:29:28 MDT