From ee98df3fa41d83f037b96cb11dc72181816920c7 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 23 Feb 2017 14:44:07 -0800 Subject: [PATCH 1/2] config: move a few helper functions up git_config_parse_key() implements the validation and downcasing of
and in "
[.]." configuration variable name. Move it (and helpers it uses) a bit up so that it can be used by git_config_parse_parameter(), which is used to check configuration settings that are given on the command line (i.e. "git -c VAR=VAL cmd"), in a later patch. Signed-off-by: Junio C Hamano --- config.c | 184 +++++++++++++++++++++++++++---------------------------- 1 file changed, 92 insertions(+), 92 deletions(-) diff --git a/config.c b/config.c index 0dfed682b8..b8cce1dffa 100644 --- a/config.c +++ b/config.c @@ -199,6 +199,98 @@ void git_config_push_parameter(const char *text) strbuf_release(&env); } +static inline int iskeychar(int c) +{ + return isalnum(c) || c == '-'; +} + +/* + * Auxiliary function to sanity-check and split the key into the section + * identifier and variable name. + * + * Returns 0 on success, -1 when there is an invalid character in the key and + * -2 if there is no section name in the key. + * + * store_key - pointer to char* which will hold a copy of the key with + * lowercase section and variable name + * baselen - pointer to int which will hold the length of the + * section + subsection part, can be NULL + */ +static int git_config_parse_key_1(const char *key, char **store_key, int *baselen_, int quiet) +{ + int i, dot, baselen; + const char *last_dot = strrchr(key, '.'); + + /* + * Since "key" actually contains the section name and the real + * key name separated by a dot, we have to know where the dot is. + */ + + if (last_dot == NULL || last_dot == key) { + if (!quiet) + error("key does not contain a section: %s", key); + return -CONFIG_NO_SECTION_OR_NAME; + } + + if (!last_dot[1]) { + if (!quiet) + error("key does not contain variable name: %s", key); + return -CONFIG_NO_SECTION_OR_NAME; + } + + baselen = last_dot - key; + if (baselen_) + *baselen_ = baselen; + + /* + * Validate the key and while at it, lower case it for matching. + */ + if (store_key) + *store_key = xmallocz(strlen(key)); + + dot = 0; + for (i = 0; key[i]; i++) { + unsigned char c = key[i]; + if (c == '.') + dot = 1; + /* Leave the extended basename untouched.. */ + if (!dot || i > baselen) { + if (!iskeychar(c) || + (i == baselen + 1 && !isalpha(c))) { + if (!quiet) + error("invalid key: %s", key); + goto out_free_ret_1; + } + c = tolower(c); + } else if (c == '\n') { + if (!quiet) + error("invalid key (newline): %s", key); + goto out_free_ret_1; + } + if (store_key) + (*store_key)[i] = c; + } + + return 0; + +out_free_ret_1: + if (store_key) { + free(*store_key); + *store_key = NULL; + } + return -CONFIG_INVALID_KEY; +} + +int git_config_parse_key(const char *key, char **store_key, int *baselen) +{ + return git_config_parse_key_1(key, store_key, baselen, 0); +} + +int git_config_key_is_valid(const char *key) +{ + return !git_config_parse_key_1(key, NULL, NULL, 1); +} + int git_config_parse_parameter(const char *text, config_fn_t fn, void *data) { @@ -354,11 +446,6 @@ static char *parse_value(void) } } -static inline int iskeychar(int c) -{ - return isalnum(c) || c == '-'; -} - static int get_value(config_fn_t fn, void *data, struct strbuf *name) { int c; @@ -1962,93 +2049,6 @@ void git_config_set(const char *key, const char *value) git_config_set_multivar(key, value, NULL, 0); } -/* - * Auxiliary function to sanity-check and split the key into the section - * identifier and variable name. - * - * Returns 0 on success, -1 when there is an invalid character in the key and - * -2 if there is no section name in the key. - * - * store_key - pointer to char* which will hold a copy of the key with - * lowercase section and variable name - * baselen - pointer to int which will hold the length of the - * section + subsection part, can be NULL - */ -static int git_config_parse_key_1(const char *key, char **store_key, int *baselen_, int quiet) -{ - int i, dot, baselen; - const char *last_dot = strrchr(key, '.'); - - /* - * Since "key" actually contains the section name and the real - * key name separated by a dot, we have to know where the dot is. - */ - - if (last_dot == NULL || last_dot == key) { - if (!quiet) - error("key does not contain a section: %s", key); - return -CONFIG_NO_SECTION_OR_NAME; - } - - if (!last_dot[1]) { - if (!quiet) - error("key does not contain variable name: %s", key); - return -CONFIG_NO_SECTION_OR_NAME; - } - - baselen = last_dot - key; - if (baselen_) - *baselen_ = baselen; - - /* - * Validate the key and while at it, lower case it for matching. - */ - if (store_key) - *store_key = xmallocz(strlen(key)); - - dot = 0; - for (i = 0; key[i]; i++) { - unsigned char c = key[i]; - if (c == '.') - dot = 1; - /* Leave the extended basename untouched.. */ - if (!dot || i > baselen) { - if (!iskeychar(c) || - (i == baselen + 1 && !isalpha(c))) { - if (!quiet) - error("invalid key: %s", key); - goto out_free_ret_1; - } - c = tolower(c); - } else if (c == '\n') { - if (!quiet) - error("invalid key (newline): %s", key); - goto out_free_ret_1; - } - if (store_key) - (*store_key)[i] = c; - } - - return 0; - -out_free_ret_1: - if (store_key) { - free(*store_key); - *store_key = NULL; - } - return -CONFIG_INVALID_KEY; -} - -int git_config_parse_key(const char *key, char **store_key, int *baselen) -{ - return git_config_parse_key_1(key, store_key, baselen, 0); -} - -int git_config_key_is_valid(const char *key) -{ - return !git_config_parse_key_1(key, NULL, NULL, 1); -} - /* * If value==NULL, unset in (remove from) config, * if value_regex!=NULL, disregard key/value pairs where value does not match. From 1274a155af96fcbcf2e60ae9fc9ee8dbdbbe987a Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 23 Feb 2017 15:04:40 -0800 Subject: [PATCH 2/2] 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
.. name in which the 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 --- config.c | 14 ++++++---- t/t1300-repo-config.sh | 62 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 71 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) diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index 923bfc5a26..ea371020fa 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -1097,6 +1097,68 @@ test_expect_success 'multiple git -c appends config' ' test_cmp expect actual ' +test_expect_success 'last one wins: two level vars' ' + + # sec.var and sec.VAR are the same variable, as the first + # and the last level of a configuration variable name is + # case insensitive. + + echo VAL >expect && + + git -c sec.var=val -c sec.VAR=VAL config --get sec.var >actual && + test_cmp expect actual && + git -c SEC.var=val -c sec.var=VAL config --get sec.var >actual && + test_cmp expect actual && + + git -c sec.var=val -c sec.VAR=VAL config --get SEC.var >actual && + test_cmp expect actual && + git -c SEC.var=val -c sec.var=VAL config --get sec.VAR >actual && + test_cmp expect actual +' + +test_expect_success 'last one wins: three level vars' ' + + # 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. + + echo val >expect && + git -c v.a.r=val -c v.A.r=VAL config --get v.a.r >actual && + test_cmp expect actual && + git -c v.a.r=val -c v.A.r=VAL config --get V.a.R >actual && + test_cmp expect actual && + + # v.a.r and V.a.R are the same variable, as the first + # and the last level of a configuration variable name is + # case insensitive. + + echo VAL >expect && + git -c v.a.r=val -c v.a.R=VAL config --get v.a.r >actual && + test_cmp expect actual && + git -c v.a.r=val -c V.a.r=VAL config --get v.a.r >actual && + test_cmp expect actual && + git -c v.a.r=val -c v.a.R=VAL config --get V.a.R >actual && + test_cmp expect actual && + git -c v.a.r=val -c V.a.r=VAL config --get V.a.R >actual && + test_cmp expect 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'" ' + test_must_fail git -c "$VAR=VAL" config -l + ' +done + +for VAR in a.b a."b c".d +do + test_expect_success "git -c $VAR=VAL works with valid '$VAR'" ' + echo VAL >expect && + git -c "$VAR=VAL" config --get "$VAR" >actual && + test_cmp expect actual + ' +done + test_expect_success 'git -c is not confused by empty environment' ' GIT_CONFIG_PARAMETERS="" git -c x.one=1 config --list '