On 31/07/2013 7:52 a.m., Amos Jeffries wrote:
> On 31/07/2013 6:23 a.m., Tsantilas Christos wrote:
>> On 07/30/2013 08:12 PM, Amos Jeffries wrote:
>>> I just ran your patch past my set of test cases built from the user
>>> complaints so far:
>>>
>>>
>>> 2013/07/31 04:56:39| Processing:
>>> configuration_includes_quoted_values off
>>> ...
>>> 2013/07/31 04:56:39| Processing: acl notAfunction url_regex
>>> ^somethingOLD(.*)
>>> 2013/07/31 04:56:39| Processing: acl notAmacro url_regex
>>> "somethingOLD$"
>>> 2013/07/31 04:56:39| Unsupported cfg macro: $"
>>> FATAL: Bungled (#2) strings.conf line 23: acl notAmacro url_regex
>>> "somethingOLD$"
>>>
>> Use single quotes. The single quotes does not try to interpret the '$'
>> character
>>
>> The "configuration_includes_quoted_values" does not disables the macros
>> in quotes.
>> It justs control the quotes behavior for some configuration parameters.
>> The quotes for some alcs for example means read from file. It
>> disables/enables this behavior.
>>
>> However we can extend its behavior.
>
> This is with quoting *disabled*.
>
> It should simply be returning the token "somethingOLD$" quotes and
> all.
>
>>
>>> Of somewhat less importance there is also this weird case showing up
>>> when I create a bungled http_access line using parameters() in the OFF
>>> mode.
>>> This one is not specific to your patch, it happens with mine too,
>>> but it
>>> does worry me...
>>>
>>> 2013/07/31 04:56:39| Processing:
>>> configuration_includes_quoted_values off
>>> ...
>>> 2013/07/31 05:00:41| Processing: http_access parameters("foo")
>>> 2013/07/31 05:00:41| aclParseAccessLine: strings.conf line 18:
>>> http_access parameters("foo")
>>> 2013/07/31 05:00:41| aclParseAccessLine: Access line contains no ACL's,
>>> skipping
>> Which is the problem here? Does the foo file has any values?
>
> This test covering the use of parameters() by someone who forgot to
> enable the new parser mode. In general we want it to fail, but on this
> directive the old parser used to strangely just drop the line without
> even an ERROR highlighting the problem.
>
> The issue I wanted you to notice is that something (probably a
> forgotten strtok or Undo) breaks the config parsing for the next
> config file line.
> (There used to be no side effects of that ACL parsing bug in the old
> parser.)
>
>>
>>> 2013/07/31 05:00:41| Processing: acl notAfunction url_regex
>>> ^somethingOLD(.*)
>>> 2013/07/31 05:00:41| FATAL: Invalid ACL type 'notAfunction' for ACL
>>> named all
>>> FATAL: Bungled (#2) strings.conf line 21: acl notAfunction url_regex
>>> ^somethingOLD(.*)
>> If I use the following:
>> configuration_includes_quoted_values off
>> acl notAfunction url_regex ^somethingOLD(.*)
>> configuration_includes_quoted_values on
>>
>> I am not getting any parse error...
>> Are you sure you are not have any
>> "configuration_includes_quoted_values on"
>> line?
>
> I do and it works perfectly when I comment out the http_access line.
>
> Notice the FATAL message is talking about ACL *type* and displaying
> the *name*.
>
> Turning on the level9 debug I get this trace:
>
> 2013/07/31 07:30:07.814| Processing: http_access parameters("foo")
> 2013/07/31 07:30:07.816| src/ConfigParser.cc(391) startParse: Parsing
> from foo
> 2013/07/31 07:30:07.816| aclParseAccessLine: strings.conf line 17:
> http_access parameters("foo")
> 2013/07/31 07:30:07.816| aclParseAccessLine: Access line contains no
> ACL's, skipping
> 2013/07/31 07:30:07.817| ../src/acl/Acl.cc(399) ~ACL: ACL::~ACL:
> 'http_access parameters("foo")'
> 2013/07/31 07:30:07.817| Processing: acl notAmacro url_regex
> "somethingOLD$
> 2013/07/31 07:30:07.818| src/ConfigParser.cc(287) NextToken:
> CfgFiles.pop foo
>
> ... so "foo" is apparently the ACL name token.
>
> I did some digging and the generated parser is still using a strtok()
> to start the line processing on each new line. That seems to be why
> the "acl" directive name is working.
>
> The major *new* bugs here are that:
> a) parameters("foo") is supposed to be a single invalid SimpleToken
> when the parser is *disabled*.
> b) the Undo stack has been left containing the "foo" token across an
> end-of-line directive change.
>
I think I have tracked this one down to src/acl/InnerNode.cc lineParse
method using strtokFile() instead of NextToken().
Not fully tested yet, but the first run through seems to move on and
parse the following lines properly after changing that.
> 2013/07/31 07:30:07.818| ../src/acl/Acl.cc(417) Registered:
> ACL::Prototype::Registered: invoked for type notAmacro
> 2013/07/31 07:30:07.818| ../src/acl/Acl.cc(425) Registered:
> ACL::Prototype::Registered: no
> 2013/07/31 07:30:07.818| FATAL: Invalid ACL type 'notAmacro'
>
>
> Amos
Received on Tue Jul 30 2013 - 23:58:33 MDT
This archive was generated by hypermail 2.2.0 : Wed Jul 31 2013 - 12:00:07 MDT