[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Fix for Issue #3781 (Case Sensitive Authz)

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Wed, 9 Feb 2011 15:32:57 +0200

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.
Received on 2011-02-09 14:38:11 CET

This is an archived mail posted to the Subversion Dev mailing list.