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

On 1/06/2013 5:14 a.m., Alex Rousskov wrote:
> On 05/31/2013 10:05 AM, Tsantilas Christos wrote:
>> On 05/27/2013 07:12 PM, Alex Rousskov wrote:
>>> On 05/27/2013 04:54 AM, Tsantilas Christos wrote:
>>>> On 05/26/2013 10:05 PM, Kinkie wrote:
>>>>>>> The only thing I would like to see differently implemented is the
>>>>>>> syntax used to include files:
>>>>>>> file(path) would be IMO easier to understand and less prone to
>>>>>>> confusion than the proposed syntax.
>>>>>> OK.
>>>>>> But imagine in the future also the following syntax:
>>>>>> file:/path/file
>>>>>> system:/usr/local/squid/bin/my-squid-conf (to read from an executable
>>>>>> stdout configuration options)
>>>>>> http://hostname/cfgfile (to get from web page configuration)
>>>>>>
>>>>>> All the above can be implemented in the future...
>>>>> Sure, I agree.
>>>>>
>>>>> file(/path/file)
>>>>> system(/some/executable)
>>>>> http_get(http://hostname/file)
>>>> Well, this is not a bad scheme :-)
>>>>
>>>> Just the file:/path/to/file a little easier to implement. But not
>>>> something important...
>>>
>>> I agree that file() is a good alternative. Amos (and others), do you
>>> have a preference between
>>>
>>> file:"/path/file"
>>>
>>> and
>>>
>>> file("/path/file")
>>>
>>>
>>> syntax?
>>>
>> About these two schemes I have a different suggestion.
>> Some opensource packages using the "file:/" or "db:/" for dynamic data.
>>
>> I do not know if it make sense for squid but I am suggesting the
>> following scheme
>> 1) Use the file("path") syntax for configuration file parsing
>> 2) preserve the file:/ or db:/ etc for dynamic data. For example acls
>> values stored in bdb or sql server.
>
> Stepping back a little, I think there are two big problems with the "URL
> scheme-like" approaches (file:, db:, etc.):
>
> 1) They combine the method of access (local file, HTTP, database query,
> etc.) with the meaning of the received information (configuration values
> to use now or configuration values to interpret at the time of directive
> use). With function-like style, this is not a problem:
>
> # static configurations to use now:
> parameters("/usr/local/etc/config.txt")
> parameters("http://example.com/config.txt")
> parameters("db:config_table")
>
> # dynamically updated configuration:
> dynamic_parameters("/usr/local/etc/config.txt")
The above is rather ambiguous, is that a text string or a filename? this
is the problem we seek to resolve with "file:" prefix.

> dynamic_parameters("http://example.com/config.txt")
> dynamic_parameters("db:config_table")

I think you have stepped back too far on this point. The interpretation
is imposed by the parser context:

   include file:/somewhere
   acl foo dstdomain file:/somewhere2
   directive_with_options parameter=file:/somewhere3
   fancy_directive parameters="http://example.com/config.txt"
more_parameters="http://example.com/config.txt method=QUERY"

The syntax we are deciding is specifically the syntax for the delimiter
between access method and location. We seem to be getting side-tracked
by the implementation choice of placing these checks inside NextToken()
due to the first two lines above being representative of how files are
currently used in squid.conf. They are in no way representative of the
future possibilities and should not be limiting the potential usage
anywhere elase in the config (as demonstrated by the latter two of my
lines above, and most of your examples).

Also the access methods we will have here are likely to be quite
restricted, either to direct filesystem methods or network locations -
all of which are URI-representable methods.

> 2) They make it difficult to add other options/parameters for that
> access (e.g., HTTP request method or database reload frequency). With
> function-like style, this is not a problem:
>
> parameters("http://example.com/config.txt", method=QUERY)
> dynamic_parameters("db:config_table", reload_frequency=1s)

I notice you are using the db:config_table syntax here for the access
method and location. Which supports the point that these ":" separator
is the best one to use for simplicity, parser re-use and familiarity for
the users.

I say we go with "file:" for now and leave the design of context
extensions to a later point when such extensions are needed.

Back to the code ...

As submitted the presence of specific checks for file:"..." in
NextToken() seems to be a bit restrictive on what can be done with them
and appears to loose the EOL marker for detecting the end of current
directive options. I think nextToken() should just return the
file:whatever token to the requestor and let that directives context
handle whether it becomes a file parsed by the ConfigParser or not.
Exposing a method which allows handlers like include directive to push a
file to be loaded onto the stack of files currently being parsed should
be sufficient for that.

If we allow the config to side-track itself into a sub-file at any point
we could end up with weird/undersirable things like this happening:

squid.conf:
   directive_A param file:/foo other_parameters

foo:
  param2
  new_directive something
  directive_A2

and directive_A2 gets the other_parameters instead of directive_A1.
Which is rather far from what we want in most situations.

We have two main use-cases for sub-files right now; the include
directive - where the file is a set of directive lines, and everywhere
else - where the file is a series of options for the current directive
at one per line.

Amos
Received on Sun Jun 02 2013 - 12:44:22 MDT