git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Brandon Williams <bmwill@google.com>
To: Stefan Beller <sbeller@google.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] config: fix case sensitive subsection names on writing
Date: Fri, 27 Jul 2018 14:21:40 -0700
Message-ID: <20180727212140.GA54208@google.com> (raw)
In-Reply-To: <20180727205117.243770-1-sbeller@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

  reply index

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-27 20:51 Stefan Beller
2018-07-27 21:21 ` Brandon Williams [this message]
2018-07-27 21:39   ` Junio C Hamano
2018-07-27 23:35     ` Stefan Beller
2018-07-27 23:36       ` Stefan Beller
2018-07-27 23:37         ` Stefan Beller
2018-07-28  1:01         ` Junio C Hamano
2018-07-28  3:52           ` Stefan Beller
2018-07-28 10:53             ` Jeff King
2018-07-28  1:37         ` Junio C Hamano
2018-07-30 12:49           ` Johannes Schindelin
2018-07-30 23:04             ` [PATCH 0/3] " Stefan Beller
2018-07-30 23:04               ` [PATCH 1/3] t1300: document current behavior of setting options Stefan Beller
2018-07-30 23:04               ` [PATCH 2/3] config: fix case sensitive subsection names on writing Stefan Beller
2018-07-31 20:16                 ` Junio C Hamano
2018-07-30 23:04               ` [PATCH 3/3] config: treat section case insensitive in store_aux_event Stefan Beller
2018-07-31 15:16               ` [PATCH 0/3] config: fix case sensitive subsection names on writing Junio C Hamano
2018-08-01 19:34                 ` [PATCH 0/3] sb/config-write-fix done without robbing Peter Stefan Beller
2018-08-01 19:34                   ` [PATCH 1/3] t1300: document current behavior of setting options Stefan Beller
2018-08-01 19:34                   ` [PATCH 2/3] config: fix case sensitive subsection names on writing Stefan Beller
2018-08-01 21:01                     ` Ramsay Jones
2018-08-01 22:26                       ` Junio C Hamano
2018-08-01 22:51                     ` Junio C Hamano
2018-08-03  0:30                       ` Stefan Beller
2018-08-03 15:51                         ` Junio C Hamano
2018-08-01 19:34                   ` [PATCH 3/3] git-config: document accidental multi-line setting in deprecated syntax Stefan Beller
2018-08-01 19:58                   ` [PATCH 0/3] sb/config-write-fix done without robbing Peter Eric Sunshine
2018-08-02 19:49                   ` Junio C Hamano
2018-08-03  0:34                   ` [PATCH 0/3] Reroll of sb/config-write-fix Stefan Beller
2018-08-03  0:34                     ` [PATCH 1/3] t1300: document current behavior of setting options Stefan Beller
2018-08-03  0:34                     ` [PATCH 2/3] config: fix case sensitive subsection names on writing Stefan Beller
2018-08-03  0:34                     ` [PATCH 3/3] git-config: document accidental multi-line setting in deprecated syntax Stefan Beller
2018-07-27 21:37 ` [PATCH] config: fix case sensitive subsection names on writing Junio C Hamano

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180727212140.GA54208@google.com \
    --to=bmwill@google.com \
    --cc=git@vger.kernel.org \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox