On 03.12.2018 09:15, Julian Foad wrote:
> Branko Čibej wrote:
>> https://issues.apache.org/jira/browse/SVN-4204
>> https://issues.apache.org/jira/browse/SVN-4795
>>
>> Even though apr_fnmatch(), which we use for matching glob patterns in
>> authz rules, supports character classes ([A-Z] etc.), we can't use them
>> in the glob rules because of the way the config parser works. For
>> example, the following rule:
>>
>> [:glob:/**/*.[Dd]oc]
>>
>> will be silently parsed to ":glob:/**/*.[Dd" [because] our config parser
>> strictly follows the semantics of Python's ConfigParser and treats the
>> first ']' it finds as the section terminator, ignoring any remaining
>> characters to the end of the line.
>>
>> The proposed patch changes this behaviour as follows:
>>
>> * the /last/ ']' in the line is the section terminator;
>> * only whitespace is allowed after the terminator to the end of the line.
> Glad to see a proposal.
>
> It makes me uncomfortable to depart from standard parsing. What if users are relying on Python ConfigParser or other compatible parsing as part of their Subversion authz infrastructure?
Then they're not using character classes in glob patterns.
The problem is that ConfigParser does not define an escaping method for
the ], so we can't even adopt that in our parser — and if we did, then
existing rules could silently change their meaning in really nasty ways.
> First I wondered if anything bad could happen if there's a silent change in meaning where a user has written, let's say,
>
>> [:glob:/**/secret1] # was [:glob:/**/secret2]
Yes, this is the sort of case where the meaning would change.
> It's hard to find any plausible example that would successfully parse and actually match something, but may be possible.
If one stretches "plausible" far enough ...
>> The proposed change in the parser is only enabled for parsing authz and
>> global group files, other Subversion configuration files will use the
>> current semantics.
> These sorts of quirks tend to make a system hard to maintain in the long run. What if we find another case where we'd like to depart from that parsing? How far would we go?
>
> What alternative solution could we consider?
I can't think of a solution that would not depart from ConfigParser
semantics in one way or another. As for "the system" ... well, the fact
that config and authz parsing shares common code is really just an
accident, and certainly an implementation detail.
-- Brane
Received on 2018-12-03 10:03:41 CET