diff --git a/src/config.c b/src/config.c index 84f3ba0f9..ce105089e 100644 --- a/src/config.c +++ b/src/config.c @@ -11,6 +11,7 @@ #include "git2/config.h" #include "vector.h" #include "buf_text.h" +#include "config_file.h" #if GIT_WIN32 # include #endif @@ -758,42 +759,36 @@ fail_parse: return -1; } -struct rename_data -{ +struct rename_data { git_config *config; - const char *old_name; - const char *new_name; + git_buf *name; + size_t old_len; + int actual_error; }; static int rename_config_entries_cb( const git_config_entry *entry, void *payload) { + int error = 0; struct rename_data *data = (struct rename_data *)payload; + size_t base_len = git_buf_len(data->name); - if (data->new_name != NULL) { - git_buf name = GIT_BUF_INIT; - int error; - - if (git_buf_printf( - &name, - "%s.%s", - data->new_name, - entry->name + strlen(data->old_name) + 1) < 0) - return -1; - + if (base_len > 0 && + !(error = git_buf_puts(data->name, entry->name + data->old_len))) + { error = git_config_set_string( - data->config, - git_buf_cstr(&name), - entry->value); + data->config, git_buf_cstr(data->name), entry->value); - git_buf_free(&name); - - if (error) - return error; + git_buf_truncate(data->name, base_len); } - return git_config_delete_entry(data->config, entry->name); + if (!error) + error = git_config_delete_entry(data->config, entry->name); + + data->actual_error = error; /* preserve actual error code */ + + return error; } int git_config_rename_section( @@ -802,36 +797,44 @@ int git_config_rename_section( const char *new_section_name) { git_config *config; - git_buf pattern = GIT_BUF_INIT; - int error = -1; + git_buf pattern = GIT_BUF_INIT, replace = GIT_BUF_INIT; + int error = 0; struct rename_data data; - git_buf_text_puts_escape_regex(&pattern, old_section_name); - git_buf_puts(&pattern, "\\..+"); - if (git_buf_oom(&pattern)) + git_buf_text_puts_escape_regex(&pattern, old_section_name); + + if ((error = git_buf_puts(&pattern, "\\..+")) < 0) goto cleanup; - if (git_repository_config__weakptr(&config, repo) < 0) + if ((error = git_repository_config__weakptr(&config, repo)) < 0) goto cleanup; - data.config = config; - data.old_name = old_section_name; - data.new_name = new_section_name; + data.config = config; + data.name = &replace; + data.old_len = strlen(old_section_name) + 1; + data.actual_error = 0; - if ((error = git_config_foreach_match( - config, - git_buf_cstr(&pattern), - rename_config_entries_cb, &data)) < 0) { - giterr_set(GITERR_CONFIG, - "Cannot rename config section '%s' to '%s'", - old_section_name, - new_section_name); - goto cleanup; + if ((error = git_buf_join(&replace, '.', new_section_name, "")) < 0) + goto cleanup; + + if (new_section_name != NULL && + (error = git_config_file_normalize_section( + replace.ptr, strchr(replace.ptr, '.'))) < 0) + { + giterr_set( + GITERR_CONFIG, "Invalid config section '%s'", new_section_name); + goto cleanup; } - error = 0; + error = git_config_foreach_match( + config, git_buf_cstr(&pattern), rename_config_entries_cb, &data); + + if (error == GIT_EUSER) + error = data.actual_error; cleanup: git_buf_free(&pattern); + git_buf_free(&replace); + return error; } diff --git a/src/config_file.c b/src/config_file.c index 2b1be05bf..8b51ab21b 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -105,6 +105,29 @@ static void cvar_free(cvar_t *var) git__free(var); } +int git_config_file_normalize_section(char *start, char *end) +{ + char *scan; + + if (start == end) + return GIT_EINVALIDSPEC; + + /* Validate and downcase range */ + for (scan = start; *scan; ++scan) { + if (end && scan >= end) + break; + if (isalnum(*scan)) + *scan = tolower(*scan); + else if (*scan != '-' || scan == start) + return GIT_EINVALIDSPEC; + } + + if (scan == start) + return GIT_EINVALIDSPEC; + + return 0; +} + /* Take something the user gave us and make it nice for our hash function */ static int normalize_name(const char *in, char **out) { @@ -118,19 +141,26 @@ static int normalize_name(const char *in, char **out) fdot = strchr(name, '.'); ldot = strrchr(name, '.'); - if (fdot == NULL || ldot == NULL) { - git__free(name); - giterr_set(GITERR_CONFIG, - "Invalid variable name: '%s'", in); - return -1; - } + if (fdot == NULL || fdot == name || ldot == NULL || !ldot[1]) + goto invalid; - /* Downcase up to the first dot and after the last one */ - git__strntolower(name, fdot - name); - git__strtolower(ldot); + /* Validate and downcase up to first dot and after last dot */ + if (git_config_file_normalize_section(name, fdot) < 0 || + git_config_file_normalize_section(ldot + 1, NULL) < 0) + goto invalid; + + /* If there is a middle range, make sure it doesn't have newlines */ + while (fdot < ldot) + if (*fdot++ == '\n') + goto invalid; *out = name; return 0; + +invalid: + git__free(name); + giterr_set(GITERR_CONFIG, "Invalid config item name '%s'", in); + return GIT_EINVALIDSPEC; } static void free_vars(git_strmap *values) @@ -271,8 +301,8 @@ static int config_set(git_config_backend *cfg, const char *name, const char *val khiter_t pos; int rval, ret; - if (normalize_name(name, &key) < 0) - return -1; + if ((rval = normalize_name(name, &key)) < 0) + return rval; /* * Try to find it in the existing values and update it if it @@ -352,9 +382,10 @@ static int config_get(const git_config_backend *cfg, const char *name, const git diskfile_backend *b = (diskfile_backend *)cfg; char *key; khiter_t pos; + int error; - if (normalize_name(name, &key) < 0) - return -1; + if ((error = normalize_name(name, &key)) < 0) + return error; pos = git_strmap_lookup_index(b->values, key); git__free(key); @@ -379,9 +410,10 @@ static int config_get_multivar( diskfile_backend *b = (diskfile_backend *)cfg; char *key; khiter_t pos; + int error; - if (normalize_name(name, &key) < 0) - return -1; + if ((error = normalize_name(name, &key)) < 0) + return error; pos = git_strmap_lookup_index(b->values, key); git__free(key); @@ -444,8 +476,8 @@ static int config_set_multivar( assert(regexp); - if (normalize_name(name, &key) < 0) - return -1; + if ((result = normalize_name(name, &key)) < 0) + return result; pos = git_strmap_lookup_index(b->values, key); if (!git_strmap_valid_index(b->values, pos)) { @@ -515,8 +547,8 @@ static int config_delete(git_config_backend *cfg, const char *name) int result; khiter_t pos; - if (normalize_name(name, &key) < 0) - return -1; + if ((result = normalize_name(name, &key)) < 0) + return result; pos = git_strmap_lookup_index(b->values, key); git__free(key); diff --git a/src/config_file.h b/src/config_file.h index cf1a59036..7445859c4 100644 --- a/src/config_file.h +++ b/src/config_file.h @@ -54,5 +54,7 @@ GIT_INLINE(int) git_config_file_foreach_match( return cfg->foreach(cfg, regexp, fn, data); } +extern int git_config_file_normalize_section(char *start, char *end); + #endif diff --git a/tests-clar/config/validkeyname.c b/tests-clar/config/validkeyname.c new file mode 100644 index 000000000..03c13d723 --- /dev/null +++ b/tests-clar/config/validkeyname.c @@ -0,0 +1,68 @@ +#include "clar_libgit2.h" + +#include "config.h" + +static git_config *cfg; +static const char *value; + +void test_config_validkeyname__initialize(void) +{ + cl_fixture_sandbox("config/config10"); + + cl_git_pass(git_config_open_ondisk(&cfg, "config10")); +} + +void test_config_validkeyname__cleanup(void) +{ + git_config_free(cfg); + cfg = NULL; + + cl_fixture_cleanup("config10"); +} + +static void assert_invalid_config_key_name(const char *name) +{ + cl_git_fail_with(git_config_get_string(&value, cfg, name), + GIT_EINVALIDSPEC); + cl_git_fail_with(git_config_set_string(cfg, name, "42"), + GIT_EINVALIDSPEC); + cl_git_fail_with(git_config_delete_entry(cfg, name), + GIT_EINVALIDSPEC); + cl_git_fail_with(git_config_get_multivar(cfg, name, "*", NULL, NULL), + GIT_EINVALIDSPEC); + cl_git_fail_with(git_config_set_multivar(cfg, name, "*", "42"), + GIT_EINVALIDSPEC); +} + +void test_config_validkeyname__accessing_requires_a_valid_name(void) +{ + assert_invalid_config_key_name(""); + assert_invalid_config_key_name("."); + assert_invalid_config_key_name(".."); + assert_invalid_config_key_name("core."); + assert_invalid_config_key_name("d#ff.dirstat.lines"); + assert_invalid_config_key_name("diff.dirstat.lines#"); + assert_invalid_config_key_name("dif\nf.dirstat.lines"); + assert_invalid_config_key_name("dif.dir\nstat.lines"); + assert_invalid_config_key_name("dif.dirstat.li\nes"); +} + +static void assert_invalid_config_section_name(git_repository *repo, const char *name) +{ + cl_git_fail_with(git_config_rename_section(repo, "branch.remoteless", name), GIT_EINVALIDSPEC); +} + +void test_config_validkeyname__renaming_a_section_requires_a_valid_name(void) +{ + git_repository *repo; + + cl_git_pass(git_repository_open(&repo, cl_fixture("testrepo.git"))); + + assert_invalid_config_section_name(repo, ""); + assert_invalid_config_section_name(repo, "bra\nch"); + assert_invalid_config_section_name(repo, "branc#"); + assert_invalid_config_section_name(repo, "bra\nch.duh"); + assert_invalid_config_section_name(repo, "branc#.duh"); + + git_repository_free(repo); +} diff --git a/tests-clar/core/buffer.c b/tests-clar/core/buffer.c index 5d9b7850c..49ab41f71 100644 --- a/tests-clar/core/buffer.c +++ b/tests-clar/core/buffer.c @@ -457,6 +457,9 @@ void test_core_buffer__8(void) git_buf_free(&a); + check_joinbuf_2(NULL, "", ""); + check_joinbuf_2(NULL, "a", "a"); + check_joinbuf_2(NULL, "/a", "/a"); check_joinbuf_2("", "", ""); check_joinbuf_2("", "a", "a"); check_joinbuf_2("", "/a", "/a");