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

On 05/27/2013 12:44 AM, Amos Jeffries wrote:
> On 27/05/2013 4:36 a.m., Kinkie wrote:
>> On Sun, May 26, 2013 at 7:29 AM, Alex Rousskov wrote:
>>> On 05/25/2013 09:30 PM, Amos Jeffries wrote:
>>>> IMO, we should drop the '-quoted string handling from this. It is
>>>> the first step on a slipery-slope toward arbitrary quoting styles and "why
>>>> not also add {-quoted strings or [-quoted strings?". Those are becoming
>>>> equally popular as people become familiar with YAML/SAS/JSON syntax.

>>> Single quoted strings were added not because they were popular but
>>> because they offer a unique functionality -- string-global suppression
>>> of macro expansion. If {-quoted string or [-quoted strings offer
>>> something equality useful they should be _considered_.

>> Single- and double- quotes have a well-specified meaning in shell
>> scripting and some other languages (e.g. PERL). From your description,
>> it seems that this behavior is implemented, so I believe there is
>> benefit and no confusion from having both.

> Yes they have a meaning. And yes that meaning is implemented - and then
> extended a bit beyond the well-known meaning.

What do you see as an extension? We did model this after shell and Perl
strings: Double-quoted strings are further interpreted for various
substitutions while single-quoted strings are treated as opaque tokens.
Do you see the proposed implementation going beyond that somehow?

> What I am trying to get as is do we actually need to implement them
> both? I don't think so. The one which is known to allow escaping should
> be sufficient.
> We can extend to include the other if necessary later, but I see no need
> to go quite that far yet.

One obvious use case is ease of converting old configuration files to
handle new syntax rules. With single-quoted strings, that conversion is
easier -- whenever in doubt, surround the token (or string) with single
quotes. Without single quoted strings, one has to carefully go through
each token component and escape everything that may be expanded today or
tomorrow.

Please note that even if we do not support single-quoted strings, we
still need to check for them to refuse tokens that start with a single
quote (to avoid upgrade problems in the future). So some single-quoted
strings code will have to go in regardless of whether we support them or
not.

> We already have macro expansion without *any* string quoting so _at
> this point_ (and I stress that it is only right now where this
> argument applies) we don't *need* it in the quoting patch.

Not exactly: header_add does not expand macros when they are not
double-quoted, by design. Here is an excerpt from squid.conf.documented:

> Field-value is either a token or a quoted string. If quoted
> string format is used, then the surrounding quotes are removed
> while escape sequences and %macros are processed.
>
> In theory, all of the logformat codes can be used as %macros.

The above was designed in hope that we will eventually add proper
support for quoted strings throughout squid.conf, which is what
Christos' patch is adding now.

Furthermore, %macros will not be processed outside double-quoted strings
in any future directives. We just have to make an "grandfathered"
exception for logformat and external ACLs to simplify the upgrade path
for admins.

This is actually one of the problems with the squid.conf syntax -- we
were adding constructs without thinking about the whole syntax
integrity, resulting in a large collection of inconsistent syntax rules
and tricks. While we cannot solve all of those problems with one patch,
 we should be thinking about macro expansion and escape sequences while
adding proper support for quoted strings. The "one step at a time"
approach is not so good when designing or fixing syntax. A comprehensive
approach works much better in that context IMO.

HTH,

Alex.
Received on Mon May 27 2013 - 16:48:58 MDT