Sanity-check config variable names

Sanity-check config variable names when adding and retrieving them.  As a side
effect code duplication between git_config_set_multivar and get_value (in
builtin/config.c) was removed and the common functionality was placed in
git_config_parse_key.

This breaks a test in t1300 which used invalid section-less keys in the tests
for "git -c". However, allowing such names there was useless, since there was
no way to set them via config file, and no part of git actually tried to use
section-less keys. This patch updates the test to use more realistic examples
as well as adding its own test.

Signed-off-by: Libor Pechacek <lpechacek@suse.cz>
Acked-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Libor Pechacek
2011-01-30 20:40:41 +01:00
committed by Junio C Hamano
parent 597a630542
commit b09c53a3e3
4 changed files with 102 additions and 50 deletions

106
config.c
View File

@ -1098,6 +1098,70 @@ int git_config_set(const char *key, const char *value)
return 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
*/
int git_config_parse_key(const char *key, char **store_key, int *baselen_)
{
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) {
error("key does not contain a section: %s", key);
return -2;
}
baselen = last_dot - key;
if (baselen_)
*baselen_ = baselen;
/*
* Validate the key and while at it, lower case it for matching.
*/
*store_key = xmalloc(strlen(key) + 1);
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))) {
error("invalid key: %s", key);
goto out_free_ret_1;
}
c = tolower(c);
} else if (c == '\n') {
error("invalid key (newline): %s", key);
goto out_free_ret_1;
}
(*store_key)[i] = c;
}
(*store_key)[i] = 0;
return 0;
out_free_ret_1:
free(*store_key);
return -1;
}
/*
* If value==NULL, unset in (remove from) config,
* if value_regex!=NULL, disregard key/value pairs where value does not match.
@ -1124,59 +1188,23 @@ int git_config_set(const char *key, const char *value)
int git_config_set_multivar(const char *key, const char *value,
const char *value_regex, int multi_replace)
{
int i, dot;
int fd = -1, in_fd;
int ret;
char *config_filename;
struct lock_file *lock = NULL;
const char *last_dot = strrchr(key, '.');
if (config_exclusive_filename)
config_filename = xstrdup(config_exclusive_filename);
else
config_filename = git_pathdup("config");
/*
* 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) {
error("key does not contain a section: %s", key);
ret = 2;
/* parse-key returns negative; flip the sign to feed exit(3) */
ret = 0 - git_config_parse_key(key, &store.key, &store.baselen);
if (ret)
goto out_free;
}
store.baselen = last_dot - key;
store.multi_replace = multi_replace;
/*
* Validate the key and while at it, lower case it for matching.
*/
store.key = xmalloc(strlen(key) + 1);
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 > store.baselen) {
if (!iskeychar(c) || (i == store.baselen+1 && !isalpha(c))) {
error("invalid key: %s", key);
free(store.key);
ret = 1;
goto out_free;
}
c = tolower(c);
} else if (c == '\n') {
error("invalid key (newline): %s", key);
free(store.key);
ret = 1;
goto out_free;
}
store.key[i] = c;
}
store.key[i] = 0;
/*
* The lock serves a purpose in addition to locking: the new