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

On 05/26/2013 10:26 PM, Amos Jeffries wrote:
> On 27/05/2013 2:07 a.m., Tsantilas Christos wrote:
>> On 05/26/2013 06:30 AM, Amos Jeffries wrote:
>>> The MacroUser system you are adding here is combining some, but not
>>> enough, of the next step in the libformat project. Thank you for doing
>>> that, but I do not think it appropriate to merge it into this whitespace
>>> and quote handling patch. It was planned to be done as a followup once
>>> the parser was updated in a way such as what this patch is doing.

>> The MacroUser just only check if %macros supported or not, and which
>> macros supported. Also the user of this class may select to not do any
>> check at all, just use it to enable macros in the next parsed token.
>> I do not think that it is related to libformat ...

> Whether macros are supported or not is irrelevant to the string being
> de-coded into a single token or split on whitespace.

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.

On 05/27/2013 04:40 AM, Tsantilas Christos wrote:
> What are you suggesting for MacroUser class? Just remove it and don't do
> any check for '%' fmt codes? Replace it with a simple on/off which
> enables/disable '%' chars in tokens? eg:
>
> ConfigParser::EnableMacro();
> token = ConfigParser::NextToken();
> ConfigParser::DisableMacro();

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.

Would that make everybody happy?

HTH,

Alex.
Received on Mon May 27 2013 - 16:23:46 MDT