On 30/07/2013 6:40 a.m., Alex Rousskov wrote:
On 07/29/2013 11:25 AM, Amos Jeffries wrote:
Looks like nobody actually ran the trivial "-k parse" test using 3.HEAD
or the initial quoted strings code.
Or perhaps nobody ran such a test with non-trivial regular expressions
in squid.conf? I know I tested [very] early versions of the quoted
strings code (which is now running on live servers), but I suspect my
configs did not use regular expressions that trigger these problems or
perhaps the early code was different enough not to trigger them: IIRC,
there have been many revisions of the original patch.


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.

After this patch, if I type parametres(foo) instead of parameters(foo),
will Squid think that I am defining a regular expression instead of
importing foo where my true regular expressions are stored?

Yes. And the only way to avoid that is to either prohibit blah(foo) in places where regex is expected, or to ignore such typos at this level and rely on the upper level data validation.

The core problem here is that '(' is treated as a primary delimiter just like whitespace outside of quoted strings. So *any* use of brackets inside a token shifts to the filename loading logics. You suggested on IRC that we use perl syntax s/(foo)/ patterns. There is still a '(' present inside there which will be detected as end of opaque element "s/" -> invalid function name -> self_destruct().

Regex are inherently dangerous to a very high degree simply from their nature and operation. Overall I don't see typos in regex fields as being made any more worse or problematic for the existence of "parameters(" tokens. Other non-regex config details should be able to and actively applying more targeted token vlidation on top and detect these typos with a better context-aware message - which may or may not result in self_destruct() dependign on that extra context.

Amos

Reply via email to