subject:"\[PATCH\] Fix for Issue #3781 \(Case Sensitive Authz\)"

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

2011-02-11 Thread Kamesh Jayachandran


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)

2011-02-11 Thread Arwin Arni


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
Index: subversion/libsvn_subr/config.c
===
--- subversion/libsvn_subr/config.c (revision 1069799)
+++ subversion/libsvn_subr/config.c (working copy)
@@ -159,7 +159,7 @@
 #ifdef WIN32
   if (sys_registry_path)
 {
-  SVN_ERR(svn_config_read(cfgp, sys_registry_path, FALSE, pool));
+  SVN_ERR(svn_config_read2(cfgp, sys_registry_path, FALSE, FALSE, pool));
   red_config = TRUE;
 }
 #endif /* WIN32 */
@@ -170,7 +170,7 @@
 SVN_ERR(svn_config_merge(*cfgp, sys_file_path, FALSE));
   else
 {
-  SVN_ERR(svn_config_read(cfgp, sys_file_path, FALSE, pool));
+  SVN_ERR(svn_config_read2(cfgp, sys_file_path, FALSE, FALSE, pool));
   red_config = TRUE;
 }
 }
@@ -184,7 +184,8 @@
 SVN_ERR(svn_config_merge(*cfgp, usr_registry_path, FALSE));
   else
 {
-  SVN_ERR(svn_config_read(cfgp, usr_registry_path, FALSE, pool));
+  SVN_ERR(svn_config_read2(cfgp, usr_registry_path,
+   FALSE, FALSE, pool));
   red_config = TRUE;
 }
 }
@@ -196,7 +197,7 @@
 SVN_ERR(svn_config_merge(*cfgp, usr_file_path, FALSE));
   else
 {
-  SVN_ERR(svn_config_read(cfgp, usr_file_path, FALSE, pool));
+  SVN_ERR(svn_config_read2(cfgp, usr_file_path, FALSE, FALSE, pool));
   red_config = TRUE;
 }
 }
@@ -332,7 +333,7 @@
  ### We could use a tmp subpool for this, since merge_cfg is going
  to be tossed afterwards.  Premature optimization, though? */
   svn_config_t *merge_cfg;
-  SVN_ERR(svn_config_read(&merge_cfg, file, must_exist, cfg->pool));
+  SVN_ERR(svn_config_read2(&merge_cfg, file, must_exist, FALSE, cfg->pool));
 
   /* Now copy the new options into the original table. */
   for_each_option(merge_cfg, cfg, merge_cfg->pool, merge_callback);
Index: subversion/tests/libsvn_subr/cache-test.c
===
--- subversion/tests/libsvn_subr/cache-test.c   (revision 1069799)
+++ subversion/tests/libsvn_subr/cache-test.c   (working copy)
@@ -169,7 +169,8 @@
 
   if (opts->config_file)
 {
-  SVN_ERR(svn_config_read(&config, opts->config_file, TRUE, pool));
+  SVN_ERR(svn_config_read2(&config, opts->config_file,
+   TRUE, FALSE, pool));
   SVN_ERR(svn_cache__make_memcache_from_config(&memcache, config, pool));
 }
 
@@ -215,7 +216,8 @@
 
   if (opts->config_file)
 {
-  SVN_ERR(svn_config_read(&config, opts->config_file, TRUE, pool));
+  SVN_ERR(svn_config_read2(&config, opts->config_file,
+   TRUE, FALSE, pool));
   SVN_ERR(svn_cache__make_memcache_from_config(&memcache, config, pool));
 }
 
Index: subversion/tests/libsvn_subr/config-test.c
===
--- subversion/tests/libsvn_subr/config-test.c  (revision 1069799)
+++ subversion/tests/libsvn_subr/config-test.c  (working copy)
@@ -109,7 +109,7 @@
 SVN_ERR(init_params(pool));
 
   cfg_file = apr_pstrcat(pool, srcdir, "/", "config-test.cfg", (char *)NULL);
-  SVN_ERR(svn_config_read(&cfg, cfg_file, TRUE, pool));
+  SVN_ERR(svn_config_read2(&cfg, cfg_file, TRUE, FALSE, pool));
 
   /* Test values retrieved from our ConfigParser instance against
  values retrieved using svn_config. */
@@ -160,7 +160,7 @@
 SVN_ERR(init_params(pool));
 
   cfg_file = apr_pstrcat(pool, srcdir, "/", "config-test.cfg", (char *)NULL);
-  SVN_ERR(svn_config_read(&cfg, cfg_file, TRUE, pool));
+  SVN_ERR(svn_config_read2(&cfg, cfg_file, TRUE, FALSE, pool));
 
   for (i = 0; true_keys[i] != NULL; i++)
 {
@@ -220,7 +220,7 @@
 SVN_ERR(init_params(pool));
 
   cfg_file = apr_pstrcat(pool, srcdir, "/", "config-test.cfg", (char *)NULL);
-  SVN_ERR(svn_config_read(&cfg, cfg_file, TRUE, pool));
+  SVN_ERR(svn_config_read2(&cfg, cfg_file, TRUE, FALSE, pool));
 
   if (! svn_config_has_section(cfg, "section1"))
 return fail(pool, "Failed to find section1");
Index: subversion/svnserve/serve.c
===
--- subversion/svnserve/serve.c (revision 1069799)
+++ subversion/svnserve/serve.c (working copy)
@@ -235,7 +235,7 @@
   const char *pwdb_path, *authzdb_path;
   svn_error_t *err;
 
-  SVN_ERR(svn_config_read(cfg, filename, must_exist, pool));
+  SVN_ERR(svn_config_read2(cfg, filename, must_exist, FALSE, pool));
 
   svn_config_get(*cfg, &pwdb_path, SVN_CONFIG_SECTION_GENERAL,
  SVN_CONFIG_OPTION_PASSWORD_DB, NULL);
@@ -245,7 +245,7 @@
 {
   pwdb_path = svn_dirent_join(base, pwdb_path, pool);
 
-  err = svn_c

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

2011-02-11 Thread Kamesh Jayachandran


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)

2011-02-09 Thread Arwin Arni


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

Index: subversion/tests/cmdline/authz_tests.py
===
--- subversion/tests/cmdline/authz_tests.py (revision 1069239)
+++ subversion/tests/cmdline/authz_tests.py (working copy)
@@ -1084,7 +1084,6 @@
  [], 'ls', '-R',
  sbox.repo_url)
 
-@XFail()
 @Issue(3781)
 @Skip(svntest.main.is_ra_type_file)
 def case_sensitive_authz(sbox):
Index: subversion/include/svn_config.h
===
--- subversion/include/svn_config.h (revision 1069239)
+++ subversion/include/svn_config.h (working copy)
@@ -189,10 +189,14 @@
 /** Set @a *cfgp to an empty @c svn_config_t structure,
  * allocated in @a result_pool.
  *
+ * Pass TRUE to @a section_names_case_sensitive if 
+ * section names are to be populated case sensitively.
+ *
  * @since New in 1.7.
  */
 svn_error_t *
 svn_config_create(svn_config_t **cfgp,
+  svn_boolean_t section_names_case_sensitive,
   apr_pool_t *result_pool);
 
 /** Read configuration data from @a file (a file or registry path) into
@@ -200,8 +204,27 @@
  *
  * If @a file does not exist, then if @a must_exist, return an error,
  * otherwise return an empty @c svn_config_t.
+ * 
+ * If @a section_names_case_sensitive is TRUE, populate section name hashes 
+ * case sensitively. Except for the default SVN_CONFIG__DEFAULT_SECTION.
+ * 
+ * @since New in 1.7.
  */
+
 svn_error_t *
+svn_config_read2(svn_config_t **cfgp,
+ const char *file,
+ svn_boolean_t must_exist,
+ svn_boolean_t section_names_case_sensitive,
+ apr_pool_t *pool);
+
+/** Similar to svn_config_read2, but always passes FALSE to 
+ * section_names_case_sensitive.
+ *
+ * @deprecated Provided for backward compatibility with 1.6 API.
+ */
+SVN_DEPRECATED
+svn_error_t *
 svn_config_read(svn_config_t **cfgp,
 const char *file,
 svn_boolean_t must_exist,
Index: subversion/libsvn_subr/config_impl.h
===
--- subversion/libsvn_subr/config_impl.h(revision 1069239)
+++ subversion/libsvn_subr/config_impl.h(working copy)
@@ -63,6 +63,9 @@
   /* Temporary value used for expanded default values in svn_config_get.
  (Using a stringbuf so that frequent resetting is efficient.) */
   svn_stringbuf_t *tmp_value;
+
+  /* Specifies whether section names are populated case sensitively. */
+  svn_boolean_t section_names_case_sensitive;
 };
 
 
Index: subversion/libsvn_subr/deprecated.c
===
--- subversion/libsvn_subr/deprecated.c (revision 1069239)
+++ subversion/libsvn_subr/deprecated.c (working copy)
@@ -1064,3 +1064,16 @@
  start, end, TRUE,
  pool, pool));
 }
+
+/*** From config.c ***/
+
+svn_error_t *
+svn_config_read(svn_config_t **cfgp, const char *file,
+svn_boolean_t must_exist,
+apr_pool_t *pool)
+{
+  return svn_error_return(svn_config_read2(cfgp, file,
+   must_exist,
+   FALSE,
+   pool));
+}
Index: subversion/libsvn_subr/config.c
===
--- subversion/libsvn_subr/config.c (revision 1069239)
+++ subversion/libsvn_subr/config.c (working copy)
@@ -78,7 +78,9 @@
 
 
 svn_error_t *
-svn_config_create(svn_config_t **cfgp, apr_pool_t *result_pool)
+svn_config_create(svn_config_t **cfgp,
+  svn_boolean_t section_names_case_sensitive,
+  apr_pool_t *result_pool)
 {
   svn_config_t *cfg = apr_palloc(result_pool, sizeof(*cfg));
 
@@ -88,19 +90,22 @@
   cfg->x_values = FALSE;
   cfg->tmp_key = svn_stringbuf_create("", result_pool);
   cfg->tmp_value = svn_stringbuf_create("", result_pool);
-
+  cfg->section_names_case_sensitive = section_names_case_sensitive; 
+  
   *cfgp = cfg;
   return SVN_NO_ERROR;
 }
 
 svn_error_t *
-svn_config_read(svn_config_t **cfgp, const char *file,
-svn_boolean_t must_exist, apr_pool_t *pool)

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

2011-02-09 Thread Arwin Arni


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
Fix Issue #3781 (Case sensitive authz).

* subversion/tests/cmdline/authz_tests.py
  (case_sensitive_authz): Removed XFail decorator.

* subversion/include/svn_config.h
  (svn_config_create) : Documented new parameter.
  (svn_config_read)   : Deprecated.
  (svn_config_read2)  : New function prototype, updated documentation.

* subversion/libsvn_subr/config_impl.h
  (svn_config_t): Added a new boolean called section_names_case_sensitive.

* subversion/libsvn_subr/deprecated.c
  (svn_config_read): New function that calls svn_config_read2
 with section_names_case_sensitive as FALSE.

* subversion/libsvn_subr/config.c
  (svn_config_create): Added a parameter section_names_case_sensitive
   which is used to initialize the config.
  (find_option, 
   svn_config_set)   : Added logic to make section names case sensitive.

  (svn_config_read)  : Deprecated.

  (svn_config_read2) : New function that deprecates svn_config_read.
   Accepts a boolean section_names_case_sensitive,
   and populates the section names case sensitively 
   if TRUE.

* subversion/libsvn_repos/authz.c
  (svn_repos_authz_read) : Fixed the caller with 
   section_names_case_sensitive as TRUE.
* subversion/tests/cmdline/atomic-ra-revprop-change.c
  (construct_config) : Fixed call of svn_config_create.
Patch by: Arwin Arni 

Index: subversion/tests/cmdline/authz_tests.py
===
--- subversion/tests/cmdline/authz_tests.py (revision 1069239)
+++ subversion/tests/cmdline/authz_tests.py (working copy)
@@ -1084,7 +1084,6 @@
  [], 'ls', '-R',
  sbox.repo_url)
 
-@XFail()
 @Issue(3781)
 @Skip(svntest.main.is_ra_type_file)
 def case_sensitive_authz(sbox):
Index: subversion/include/svn_config.h
===
--- subversion/include/svn_config.h (revision 1069239)
+++ subversion/include/svn_config.h (working copy)
@@ -189,10 +189,14 @@
 /** Set @a *cfgp to an empty @c svn_config_t structure,
  * allocated in @a result_pool.
  *
+ * Pass TRUE to @a section_names_case_sensitive if 
+ * section names are to be populated case sensitively.
+ *
  * @since New in 1.7.
  */
 svn_error_t *
 svn_config_create(svn_config_t **cfgp,
+  svn_boolean_t section_names_case_sensitive,
   apr_pool_t *result_pool);
 
 /** Read configuration data from @a file (a file or registry path) into
@@ -200,8 +204,27 @@
  *
  * If @a file does not exist, then if @a must_exist, return an error,
  * otherwise return an empty @c svn_config_t.
+ * 
+ * If @a section_names_case_sensitive is TRUE, populate section name hashes 
+ * case sensitively.
+ * 
+ * @since New in 1.7.
  */
+
 svn_error_t *
+svn_config_read2(svn_config_t **cfgp,
+ const char *file,
+ svn_boolean_t must_exist,
+ svn_boolean_t section_names_case_sensitive,
+ apr_pool_t *pool);
+
+/** Similar to svn_config_read2, but always passes FALSE to 
+ * section_names_case_sensitive.
+ *
+ * @deprecated Provided for backward compatibility with 1.6 API.
+ */
+SVN_DEPRECATED
+svn_error_t *
 svn_config_read(svn_config_t **cfgp,
 const char *file,
 svn_boolean_t must_exist,
Index: subversion/libsvn_subr/config_impl.h
===
--- subversion/libsvn_subr/config_impl.h(revision 1069239)
+++ subversion/libsvn_subr/config_impl.h(working copy)
@@ -63,6 +63,9 @@
   /* Temporary value used for expanded default values in svn_config_get.
  (Using a stringbuf so that frequent resetting is efficient.) */
   svn_stringbuf_t *tmp_value;
+
+  /* Specifies whether section names are populated case sensitively. */
+  svn_boolean_t section_names_case_sensitive;
 };
 
 
Index: subversion/libsvn_subr/deprecated.c
===
--- subversion/libsvn_subr/deprecated.c (revision 1069239)
+++ subversion/libsvn_subr/deprecated.c (working copy)
@@ -1064,3 +1064,16 @@
  start, end, TRUE,
  pool, pool));
 }
+
+/*** From config.c ***/
+
+svn_error_t *
+svn_config_read(svn_config_t **cfgp, const char *file,
+svn_boolean_t must_exist,
+apr_pool_t *pool)
+{
+  return svn_error_return(svn_config_read2(cf

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

2011-02-09 Thread Arwin Arni


Hi All,

Here's the patch for the implementation of the logic.

Regards,
Arwin Arni

Index: subversion/libsvn_subr/config.c
===
--- subversion/libsvn_subr/config.c (revision 1068869)
+++ subversion/libsvn_subr/config.c (working copy)
@@ -78,7 +78,9 @@
 
 
 svn_error_t *
-svn_config_create(svn_config_t **cfgp, apr_pool_t *result_pool)
+svn_config_create(svn_config_t **cfgp,
+  svn_boolean_t section_names_case_sensitive,
+  apr_pool_t *result_pool)
 {
   svn_config_t *cfg = apr_palloc(result_pool, sizeof(*cfg));
 
@@ -88,19 +90,22 @@
   cfg->x_values = FALSE;
   cfg->tmp_key = svn_stringbuf_create("", result_pool);
   cfg->tmp_value = svn_stringbuf_create("", result_pool);
-
+  cfg->section_names_case_sensitive = section_names_case_sensitive; 
+  
   *cfgp = cfg;
   return SVN_NO_ERROR;
 }
 
 svn_error_t *
-svn_config_read(svn_config_t **cfgp, const char *file,
-svn_boolean_t must_exist, apr_pool_t *pool)
+svn_config_read2(svn_config_t **cfgp, const char *file,
+ svn_boolean_t must_exist, 
+ svn_boolean_t section_names_case_sensitive,
+ apr_pool_t *pool)
 {
   svn_config_t *cfg;
   svn_error_t *err;
 
-  SVN_ERR(svn_config_create(&cfg, pool));
+  SVN_ERR(svn_config_create(&cfg, section_names_case_sensitive, pool));
 
   /* Yes, this is platform-specific code in Subversion, but there's no
  practical way to migrate it into APR, as it's simultaneously
@@ -387,7 +392,8 @@
 
   /* Canonicalize the hash key */
   svn_stringbuf_set(cfg->tmp_key, section);
-  make_hash_key(cfg->tmp_key->data);
+  if (! cfg->section_names_case_sensitive)
+make_hash_key(cfg->tmp_key->data);
 
   sec_ptr = apr_hash_get(cfg->sections, cfg->tmp_key->data,
  cfg->tmp_key->len);
@@ -618,7 +624,10 @@
   /* Even the section doesn't exist. Create it. */
   sec = apr_palloc(cfg->pool, sizeof(*sec));
   sec->name = apr_pstrdup(cfg->pool, section);
-  sec->hash_key = make_hash_key(apr_pstrdup(cfg->pool, section));
+  if(cfg->section_names_case_sensitive)
+sec->hash_key = sec->name;
+  else
+sec->hash_key = make_hash_key(apr_pstrdup(cfg->pool, section));
   sec->options = apr_hash_make(cfg->pool);
   apr_hash_set(cfg->sections, sec->hash_key, APR_HASH_KEY_STRING, sec);
 }
Index: subversion/libsvn_subr/config_impl.h
===
--- subversion/libsvn_subr/config_impl.h(revision 1068869)
+++ subversion/libsvn_subr/config_impl.h(working copy)
@@ -63,6 +63,9 @@
   /* Temporary value used for expanded default values in svn_config_get.
  (Using a stringbuf so that frequent resetting is efficient.) */
   svn_stringbuf_t *tmp_value;
+
+  /* Specifies whether section names are populated case sensitively. */
+  svn_boolean_t section_names_case_sensitive;
 };
 
 
Index: subversion/libsvn_subr/deprecated.c
===
--- subversion/libsvn_subr/deprecated.c (revision 1068869)
+++ subversion/libsvn_subr/deprecated.c (working copy)
@@ -1064,3 +1064,16 @@
  start, end, TRUE,
  pool, pool));
 }
+
+/*** From config.c ***/
+
+svn_error_t *
+svn_config_read(svn_config_t **cfgp, const char *file,
+svn_boolean_t must_exist,
+apr_pool_t *pool)
+{
+  return svn_error_return(svn_config_read2(cfgp, file,
+   must_exist,
+   FALSE,
+   pool));
+}
Index: subversion/tests/cmdline/authz_tests.py
===
--- subversion/tests/cmdline/authz_tests.py (revision 1068869)
+++ subversion/tests/cmdline/authz_tests.py (working copy)
@@ -1084,7 +1084,6 @@
  [], 'ls', '-R',
  sbox.repo_url)
 
-@XFail()
 @Issue(3781)
 @Skip(svntest.main.is_ra_type_file)
 def case_sensitive_authz(sbox):
Index: subversion/include/svn_config.h
===
--- subversion/include/svn_config.h (revision 1068869)
+++ subversion/include/svn_config.h (working copy)
@@ -189,10 +189,14 @@
 /** Set @a *cfgp to an empty @c svn_config_t structure,
  * allocated in @a result_pool.
  *
+ * Pass TRUE to @a section_names_case_sensitive if 
+ * section names are to be populated case sensitively.
+ *
  * @since New in 1.7.
  */
 svn_error_t *
 svn_config_create(svn_config_t **cfgp,
+  svn_boolean_t section_names_case_sensitive,
   apr_pool_t *result_pool);
 
 /** Read configuration data from @a file (a file or registry path) into
@@ -200,8 +204,27 @@
  *
  * If @

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

2011-02-09 Thread Arwin Arni


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)

2011-02-09 Thread Daniel Shahaf

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.

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

2011-02-09 Thread Arwin Arni


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.


Index: subversion/libsvn_subr/config.c
===
--- subversion/libsvn_subr/config.c (revision 1068828)
+++ subversion/libsvn_subr/config.c (working copy)
@@ -78,7 +78,9 @@
 
 
 svn_error_t *
-svn_config_create(svn_config_t **cfgp, apr_pool_t *result_pool)
+svn_config_create(svn_config_t **cfgp,
+  svn_boolean_t section_names_case_sensitive,
+  apr_pool_t *result_pool)
 {
   svn_config_t *cfg = apr_palloc(result_pool, sizeof(*cfg));
 
@@ -88,19 +90,22 @@
   cfg->x_values = FALSE;
   cfg->tmp_key = svn_stringbuf_create("", result_pool);
   cfg->tmp_value = svn_stringbuf_create("", result_pool);
-
+  cfg->section_names_case_sensitive = section_names_case_sensitive; 
+  
   *cfgp = cfg;
   return SVN_NO_ERROR;
 }
 
 svn_error_t *
-svn_config_read(svn_config_t **cfgp, const char *file,
-svn_boolean_t must_exist, apr_pool_t *pool)
+svn_config_read2(svn_config_t **cfgp, const char *file,
+ svn_boolean_t must_exist, 
+ svn_boolean_t section_names_case_sensitive,
+ apr_pool_t *pool)
 {
   svn_config_t *cfg;
   svn_error_t *err;
 
-  SVN_ERR(svn_config_create(&cfg, pool));
+  SVN_ERR(svn_config_create(&cfg, section_names_case_sensitive, pool));
 
   /* Yes, this is platform-specific code in Subversion, but there's no
  practical way to migrate it into APR, as it's simultaneously
@@ -154,7 +159,7 @@
 #ifdef WIN32
   if (sys_registry_path)
 {
-  SVN_ERR(svn_config_read(cfgp, sys_registry_path, FALSE, pool));
+  SVN_ERR(svn_config_read2(cfgp, sys_registry_path, FALSE, FALSE, pool));
   red_config = TRUE;
 }
 #endif /* WIN32 */
@@ -165,7 +170,7 @@
 SVN_ERR(svn_config_merge(*cfgp, sys_file_path, FALSE));
   else
 {
-  SVN_ERR(svn_config_read(cfgp, sys_file_path, FALSE, pool));
+  SVN_ERR(svn_config_read2(cfgp, sys_file_path, FALSE, FALSE, pool));
   red_config = TRUE;
 }
 }
@@ -179,7 +184,7 @@
 SVN_ERR(svn_config_merge(*cfgp, usr_registry_path, FALSE));
   else
 {
-  SVN_ERR(svn_config_read(cfgp, usr_registry_path, FALSE, pool));
+  SVN_ERR(svn_config_read2(cfgp, usr_registry_path, FALSE, FALSE, 
pool));
   red_config = TRUE;
 }
 }
@@ -191,7 +196,7 @@
 SVN_ERR(svn_config_merge(*cfgp, usr_file_path, FALSE));
   else
 {
-  SVN_ERR(svn_config_read(cfgp, usr_file_path, FALSE, pool));
+  SVN_ERR(svn_config_read2(cfgp, usr_file_path, FALSE, FALSE, pool));
   red_config = TRUE;
 }
 }
@@ -327,7 +332,7 @@
  ### We could use a tmp subpool for this, since merge_cfg is going
  to be tossed afterwards.  Premature optimization, though? */
   svn_config_t *merge_cfg;
-  SVN_ERR(svn_config_read(&merge_cfg, file, must_exist, cfg->pool));
+  SVN_ERR(svn_config_read2(&merge_cfg, file, must_exist, FALSE, cfg->pool));
 
   /* Now copy the new options into the original table. */
   for_each_option(merge_cfg, cfg, merge_cfg->pool, merge_callback);
@@ -387,7 +392,8 @@
 
   /* Canonicalize the hash key */
   svn_stringbuf_set(cfg->tmp_key, section);
-  make_hash_key(cfg->tmp_key->data);
+  if (! cfg->section_names_case_sensitive)
+make_hash_key(cfg->tmp_key->data);
 
   sec_ptr = apr_hash_get(cfg->sections, cfg->tmp_key->data,
  cfg->tmp_key->len);
@@ -618,7 +624,10 @@
   /* Even the section doesn't exist. Create it. */
   sec = apr_palloc(cfg->pool, sizeof(*sec));
   sec->name = apr_pstrdup(cfg->pool, section);
-  sec->hash_key = make_hash_key(apr_pstrdup(cfg->pool, section));
+  if(cfg->section_names_case_sensitive)
+sec->hash_key = sec->name;
+  else
+sec->hash_key = make_hash_key(apr_pstrdup(cfg->pool, section));
   sec->options = apr_hash_make(cfg->pool);
   apr_hash_set(cfg->sections, sec->hash_key, APR_HASH_KEY_STRING, sec);
 }
Index: subversion/libsvn_subr/config_impl.h
===
--- subversion/libsvn_subr/config_impl.h(revision 1068828)
+++ subversion/libsvn_subr/config_impl.h(working copy)
@@ -63,6 +63,9 @@
   /* Temporary value used for expanded default values in svn_config_get.
  (Using a stringbuf so that frequent resetting is efficient.) */
   svn_stringbuf_t *tmp_value;
+
+  /* Specifies whether section names are populated case sensitively. */
+  svn_boolean_t section_names_case_sensitive;
 };
 
 
Index: subversion/libsvn_subr/deprecated.c
===
--- subversion/libsvn_subr/depreca

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

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

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

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

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

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

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

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

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

9 matches

Site Navigation

Mail list logo

Footer information