Re: [PATCH] config: fix case sensitive subsection names on writing

[Brandon Williams <bmwill@google.com>]

On 07/27, Stefan Beller wrote:
> A use reported a submodule issue regarding strange case indentation
> issues, but it could be boiled down to the following test case:
> 
>   $ git init test  && cd test
>   $ git config foo."Bar".key test
>   $ git config foo."bar".key test
>   $ tail -n 3 .git/config
>   [foo "Bar"]
>         key = test
>         key = test
> 
> Sub sections are case sensitive and we have a test for correctly reading
> them. However we do not have a test for writing out config correctly with
> case sensitive subsection names, which is why this went unnoticed in
> 6ae996f2acf (git_config_set: make use of the config parser's event
> stream, 2018-04-09)
> 
> Make the subsection case sensitive and provide a test for both reading
> and writing.
> 
> Reported-by: JP Sugarbroad <jpsugar@google.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  config.c          |  2 +-
>  t/t1300-config.sh | 18 ++++++++++++++++++
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/config.c b/config.c
> index 3aacddfec4b..3ded92b678b 100644
> --- a/config.c
> +++ b/config.c
> @@ -2374,7 +2374,7 @@ static int store_aux_event(enum config_event_t type,
>  		store->is_keys_section =
>  			store->parsed[store->parsed_nr].is_keys_section =
>  			cf->var.len - 1 == store->baselen &&
> -			!strncasecmp(cf->var.buf, store->key, store->baselen);
> +			!strncmp(cf->var.buf, store->key, store->baselen);

I've done some work in the config part of our codebase but I don't
really know whats going on here due to two things: this is a callback
function and it relies on global state...

I can say though that we might want to be careful about completely
converting this to a case sensitive compare.  Our config is a key
value store with the following format: 'section.subsection.key'.  IIRC
both section and key are always compared case insensitively.  The
subsection can be case sensitive or insensitive based on how its
stored in the config files itself:

  # Case insensitive
  [section.subsection]
      key = value

  # Case sensitive 
  [section "subsection"]
      key = value

But I don't know how you distinguish between these cases when a config
is specified on a single line (section.subsection.key).  Do we always
assume the sensitive version because the insensitive version is
documented to be deprecated?

Either way you're probably going to need to be careful about how you do
string comparison against the different parts.

>  		if (store->is_keys_section) {
>  			store->section_seen = 1;
>  			ALLOC_GROW(store->seen, store->seen_nr + 1,
> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> index 03c223708eb..8325d4495f4 100755
> --- a/t/t1300-config.sh
> +++ b/t/t1300-config.sh
> @@ -1218,6 +1218,24 @@ test_expect_success 'last one wins: three level vars' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'setting different case subsections ' '
> +	test_when_finished "rm -f caseSens caseSens_actual caseSens_expect" &&
> +
> +	# v.a.r and v.A.r are not the same variable, as the middle
> +	# level of a three-level configuration variable name is
> +	# case sensitive.
> +	git config -f caseSens v."A".r VAL &&
> +	git config -f caseSens v."a".r val &&
> +
> +	echo VAL >caseSens_expect &&
> +	git config -f caseSens v."A".r >caseSens_actual &&
> +	test_cmp caseSens_expect caseSens_actual &&
> +
> +	echo val >caseSens_expect &&
> +	git config -f caseSens v."a".r >caseSens_actual &&
> +	test_cmp caseSens_expect caseSens_actual
> +'
> +
>  for VAR in a .a a. a.0b a."b c". a."b c".0d
>  do
>  	test_expect_success "git -c $VAR=VAL rejects invalid '$VAR'" '
> -- 
> 2.18.0.345.g5c9ce644c3-goog
> 

-- 
Brandon Williams