Re: [PATCH] Unknown cfg function

On 07/29/2013 06:31 PM, Amos Jeffries wrote:
> On 30/07/2013 5:25 a.m., Amos Jeffries wrote:
>> Attached is a patch which adds a secondary form of undo for handling
>> incorrectly identified ConfigParser::FunctionNameToken elements.
>> Instead of aborting Squid on any non-function token containing a '('
>> we take the wrongly identified assumed function name element and
>> reform it with the '(' delimiter and trailing element. The resulting
>> aggregate token which should have been identified initially is then
>> sent to the upper parser layer.

> Updated patch. This one does all the above

If new syntax is enabled, I think Squid should quit if it does not
recognize a configuration function name. If legacy syntax is enabled,
Squid should not try to recognize function names at all.

My understanding is that a patched Squid will not die when a function
name is not recognized in a new syntax context. I think that is wrong.

> plus:
> * handles the case where a token starts with '(' as in the default
> refresh_pattern for cgi-bin handling.

The new syntax should probably [continue to] reject these so that we can
later add support for expressions with parenthesis. The legacy syntax
should [continue to] accept them, of course.

> * removes $macro handling

Yes, I cannot think of a reason to keep this if we standardize on
%macros. Christos?

If the code should be removed, let's just remove it completely instead
of commenting it out.

> * adds debugs statements for parameters("foo") file loading

> + message << "Bungled (#1) " << f->filePath << " line " << f->lineNo <<
> + fatalf("Bungled (#2) %s line %d: %s",

Can these two be rephrased to give admins something more than mysterious
#1 and #2 numbers? One of those lines alone would look especially
strange -- "did I miss #1?" or "Will there be a #2 later?".

Thank you,

Alex.
Received on Tue Jul 30 2013 - 01:25:17 MDT