Re: [PATCH v2] config: reject invalid VAR in 'git -c VAR=VAL command'
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 24, 2017, 06:08 UTC
- Message-ID
- <xmqqd1e85ddy.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20170224042227.2rjgf4zbiadxbrtz@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 23 quoted lines
>> Backtracking will not fundamentally "fix" parsing of >> >> a.b=c=.d >> >> between twhse two >> >> [a "b="] c = ".d" >> [a] b = "c=.d" >> >> unfortunately, I think. I do not think it is worth doing the "best >> effort" with erroring out when ambiguous, because there is no way >> for the end user to disambiguate, unless we introduce a different >> syntax, at which point we cannot use config_parse_key() anymore. > > Ah, yeah, you're right. I thought the problem was just that the "split" > was too naive, but it really is that the whole syntax is badly > specified. > > I guess "git config --list" suffers from the same problem. You can get > around it there with "-z", but that probably would not be very pleasant > here. :) > > Probably not worth worrying too much about if nobody is complaining.
Yup.
Anyway, here is an updated one (the part of the patch to t/ is not shown as it is unchanged).
-- >8 -- Subject: [PATCH] config: use git_config_parse_key() in git_config_parse_parameter()
The parsing of one-shot assignments of configuration variables that come from the command line historically was quite loose and allowed anything to pass. It also downcased everything in the variable name, even a three-level <section>.<subsection>.<variable> name in which the <subsection> part must be treated in a case sensitive manner.
Existing git_config_parse_key() helper is used to parse the variable name that comes from the command line, i.e. "git config VAR VAL", and handles these details correctly. Replace the strbuf_tolower() call in git_config_parse_parameter() with a call to it to correct both issues. git_config_parse_key() does a bit more things that are not necessary for the purpose of this codepath (e.g. it allocates a separate buffer to return the canonicalized variable name because it takes a "const char *" input), but we are not in a performance-critical codepath here.
Signed-off-by: Junio C Hamano <gitster@pobox.com> --- config.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/config.c b/config.c index b8cce1dffa..1c1a1520ff 100644 --- a/config.c +++ b/config.c @@ -295,7 +295,9 @@ int git_config_parse_parameter(const char *text, config_fn_t fn, void *data) { const char *value; + char *canonical_name; struct strbuf **pair; + int ret; pair = strbuf_split_str(text, '=', 2); if (!pair[0]) @@ -313,13 +315,15 @@ int git_config_parse_parameter(const char *text, strbuf_list_free(pair); return error("bogus config parameter: %s", text); } - strbuf_tolower(pair[0]); - if (fn(pair[0]->buf, value, data) < 0) { - strbuf_list_free(pair); - return -1; + + if (git_config_parse_key(pair[0]->buf, &canonical_name, NULL)) { + ret = -1; + } else { + ret = (fn(canonical_name, value, data) < 0) ? -1 : 0; + free(canonical_name); } strbuf_list_free(pair); - return 0; + return ret; } int git_config_from_parameters(config_fn_t fn, void *data)
-- 2.12.0-rc2-308-gbf7e63c428