You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Arwin Arni <ar...@collab.net> on 2011/02/09 13:09:55 UTC
[PATCH] Fix for Issue #3781 (Case Sensitive Authz)
Hi All,
This patch fixes Issue #3781 ->
http://subversion.tigris.org/issues/show_bug.cgi?id=3781
and the corresponding XFail case in authz_tests.py (case_sensitive_authz).
This makes the section names of the authz files case sensitive.
Attached is the patch and log message.
Re: [PATCH] Fix for Issue #3781 (Case Sensitive Authz)
Posted by Kamesh Jayachandran <ka...@collab.net>.
On 02/11/2011 07:49 PM, Arwin Arni wrote:
> On Friday 11 February 2011 07:06 PM, Kamesh Jayachandran wrote:
>>
>> Thanks Arwin. Committed at r1069791.
>>
>> With regards
>> Kamesh Jayachandran
>>
>
> Here is the follow-up patch that fixes the deprecated calls of
> svn_config_read.
>
> Regards,
> Arwin Arni
Thanks Arwin committed in r1069821.
With regards
Kamesh Jayachandran
Re: [PATCH] Fix for Issue #3781 (Case Sensitive Authz)
Posted by Arwin Arni <ar...@collab.net>.
On Friday 11 February 2011 07:06 PM, Kamesh Jayachandran wrote:
>
> Thanks Arwin. Committed at r1069791.
>
> With regards
> Kamesh Jayachandran
>
Here is the follow-up patch that fixes the deprecated calls of
svn_config_read.
Regards,
Arwin Arni
Re: [PATCH] Fix for Issue #3781 (Case Sensitive Authz)
Posted by Kamesh Jayachandran <ka...@collab.net>.
On 02/10/2011 01:15 PM, Arwin Arni wrote:
> On Thursday 10 February 2011 12:48 PM, Arwin Arni wrote:
>> On Wednesday 09 February 2011 09:21 PM, Arwin Arni wrote:
>>> Hi All,
>>>
>>> Here's the patch for the implementation of the logic.
>>>
>>> Regards,
>>> Arwin Arni
>>>
>> Sorry, Please ignore the previous patch. There was a stray caller of
>> svn_config_create (whose signature I changed and didn't rev as it was
>> new in the 1.7 API.). Here's the correct patch.
>>
>> Regards,
>> Arwin Arni
> I missed adding the documentation for the deviation in behavior for
> SVN_CONFIG__DEFAULT_SECTION. So here is a patch with the documentation.
> This is the last correction. I promise.
>
Thanks Arwin. Committed at r1069791.
With regards
Kamesh Jayachandran
> Regards,
> Arwin Arni
>
Re: [PATCH] Fix for Issue #3781 (Case Sensitive Authz)
Posted by Arwin Arni <ar...@collab.net>.
On Thursday 10 February 2011 12:48 PM, Arwin Arni wrote:
> On Wednesday 09 February 2011 09:21 PM, Arwin Arni wrote:
>> Hi All,
>>
>> Here's the patch for the implementation of the logic.
>>
>> Regards,
>> Arwin Arni
>>
> Sorry, Please ignore the previous patch. There was a stray caller of
> svn_config_create (whose signature I changed and didn't rev as it was
> new in the 1.7 API.). Here's the correct patch.
>
> Regards,
> Arwin Arni
I missed adding the documentation for the deviation in behavior for
SVN_CONFIG__DEFAULT_SECTION. So here is a patch with the documentation.
This is the last correction. I promise.
Regards,
Arwin Arni
Re: [PATCH] Fix for Issue #3781 (Case Sensitive Authz)
Posted by Arwin Arni <ar...@collab.net>.
On Wednesday 09 February 2011 09:21 PM, Arwin Arni wrote:
> Hi All,
>
> Here's the patch for the implementation of the logic.
>
> Regards,
> Arwin Arni
>
Sorry, Please ignore the previous patch. There was a stray caller of
svn_config_create (whose signature I changed and didn't rev as it was
new in the 1.7 API.). Here's the correct patch.
Regards,
Arwin Arni
Re: [PATCH] Fix for Issue #3781 (Case Sensitive Authz)
Posted by Arwin Arni <ar...@collab.net>.
Hi All,
Here's the patch for the implementation of the logic.
Regards,
Arwin Arni
Re: [PATCH] Fix for Issue #3781 (Case Sensitive Authz)
Posted by Arwin Arni <ar...@collab.net>.
On Wednesday 09 February 2011 07:02 PM, Daniel Shahaf wrote:
> Arwin Arni wrote on Wed, Feb 09, 2011 at 17:39:55 +0530:
>> * subversion/libsvn_repos/authz.c,
>> subversion/tests/libsvn_subr/cache-test.c,
>> subversion/tests/libsvn_subr/config-test.c,
>> subversion/tests/cmdline/atomic-ra-revprop-change.c,
>> subversion/svnserve/serve.c,
>> subversion/libsvn_fs_fs/fs_fs.c
>>
>> (svn_repos_authz_read, test_memcache_basic, test_memcache_longkey,
>> test_text_retrieval,test_boolean_retrieval, test_has_section,
>> construct_config, load_configs, read_config) : Fixed callers.
>>
>
> Thanks for doing this, but please make it a separate patch. It
> distracts the review and it'll have to be split out prior to commit
> anyway.
>
Sure. I'll send these separately.
>> Index: subversion/libsvn_subr/config.c
>> ===================================================================
>> --- subversion/libsvn_subr/config.c (revision 1068828)
>> +++ subversion/libsvn_subr/config.c (working copy)
>> @@ -78,7 +78,9 @@
>>
>
> There is a apr_strnatcasecmp() call which your patch doesn't remove.
> Don't you have to change it?
>
This is the comparison being done for SVN_CONFIG__DEFAULT_SECTION.
I've left this out specifically, so it doesn't matter if the user says
[default] or [DEFAULT]. (kind of a special case)
I'll document this clearly in the code.
This doesn't concern the issue that I'm fixing anyway, because there
isn't a default section in an authz file.
> Also: do we parse option names (the 'foo' in foo=bar) case-sensitively
> or not? (The current code seems to be insensitive.) Should we allow
> for case-sensitive option names (not just section names) while we're
> here? (IMO, yes.)
>
I had initially proposed additional parameters that decide case
sensitivity of optins/values:
http://svn.haxx.se/dev/archive-2011-01/0528.shtml
Mike felt that we'd be over-engineering it. So currently only section
names are parsed case sensitively.
> The rest looks good. I'm not +1 yet, because I haven't checked whether
> there are any other hunks (other than the apr_strnatcasecmp()) that need
> to be added.
>
Thanks and Regards,
Arwin Arni
Re: [PATCH] Fix for Issue #3781 (Case Sensitive Authz)
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Arwin Arni wrote on Wed, Feb 09, 2011 at 17:39:55 +0530:
> * subversion/libsvn_repos/authz.c,
> subversion/tests/libsvn_subr/cache-test.c,
> subversion/tests/libsvn_subr/config-test.c,
> subversion/tests/cmdline/atomic-ra-revprop-change.c,
> subversion/svnserve/serve.c,
> subversion/libsvn_fs_fs/fs_fs.c
>
> (svn_repos_authz_read, test_memcache_basic, test_memcache_longkey,
> test_text_retrieval,test_boolean_retrieval, test_has_section,
> construct_config, load_configs, read_config) : Fixed callers.
>
Thanks for doing this, but please make it a separate patch. It
distracts the review and it'll have to be split out prior to commit
anyway.
> Index: subversion/libsvn_subr/config.c
> ===================================================================
> --- subversion/libsvn_subr/config.c (revision 1068828)
> +++ subversion/libsvn_subr/config.c (working copy)
> @@ -78,7 +78,9 @@
>
There is a apr_strnatcasecmp() call which your patch doesn't remove.
Don't you have to change it?
Also: do we parse option names (the 'foo' in foo=bar) case-sensitively
or not? (The current code seems to be insensitive.) Should we allow
for case-sensitive option names (not just section names) while we're
here? (IMO, yes.)
The rest looks good. I'm not +1 yet, because I haven't checked whether
there are any other hunks (other than the apr_strnatcasecmp()) that need
to be added.