From 64244131d0534ac19f47227a751790fe67e562ba Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 25 Apr 2017 12:59:48 +0200 Subject: [PATCH 1/3] config_file: add missing include for `git_config_backend` The config_file.h header provides some inline declarations accessing the `git_config_backend`, but misses its declaration. Add the missing include for "git2/sys/config.h" to add it. --- src/config_file.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/config_file.h b/src/config_file.h index 1c52892c3..654e6cacf 100644 --- a/src/config_file.h +++ b/src/config_file.h @@ -7,6 +7,7 @@ #ifndef INCLUDE_config_file_h__ #define INCLUDE_config_file_h__ +#include "git2/sys/config.h" #include "git2/config.h" GIT_INLINE(int) git_config_file_open(git_config_backend *cfg, unsigned int level) From 95f29fb35b9fbc7bd98f20bd91886522ddc03f4b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 25 Apr 2017 12:40:13 +0200 Subject: [PATCH 2/3] config: skip r/o backends when writing Configuration backends have a readonly-flag which is currently used to distinguish configuration snapshots. But somewhat unexpectedly, we do not use the flag to prevent writing to a readonly backend but happily proceed to do so. This commit modifies logic to also honor the readonly flag for configuration setters. We will now traverse through all backends and pick the first one which is not marked as read-only whenever we want to write new configuration. --- src/config.c | 63 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 22 deletions(-) diff --git a/src/config.c b/src/config.c index 0d73ad2cc..cbcea2e78 100644 --- a/src/config.c +++ b/src/config.c @@ -576,22 +576,50 @@ int git_config_foreach_match( * Setters **************/ -static int config_error_nofiles(const char *name) +typedef enum { + BACKEND_USE_SET, + BACKEND_USE_DELETE +} backend_use; + +static const char *uses[] = { + "set", + "delete" +}; + +static int get_backend_for_use(git_config_backend **out, + git_config *cfg, const char *name, backend_use use) { + size_t i; + file_internal *f; + + *out = NULL; + + if (git_vector_length(&cfg->files) == 0) { + giterr_set(GITERR_CONFIG, + "cannot %s value for '%s' when no config files exist", + uses[use], name); + return GIT_ENOTFOUND; + } + + git_vector_foreach(&cfg->files, i, f) { + if (!f->file->readonly) { + *out = f->file; + return 0; + } + } + giterr_set(GITERR_CONFIG, - "cannot set value for '%s' when no config files exist", name); + "cannot %s value for '%s' when all config files are readonly", + uses[use], name); return GIT_ENOTFOUND; } int git_config_delete_entry(git_config *cfg, const char *name) { git_config_backend *file; - file_internal *internal; - internal = git_vector_get(&cfg->files, 0); - if (!internal || !internal->file) - return config_error_nofiles(name); - file = internal->file; + if (get_backend_for_use(&file, cfg, name, BACKEND_USE_DELETE) < 0) + return GIT_ENOTFOUND; return file->del(file, name); } @@ -617,17 +645,14 @@ int git_config_set_string(git_config *cfg, const char *name, const char *value) { int error; git_config_backend *file; - file_internal *internal; if (!value) { giterr_set(GITERR_CONFIG, "the value to set cannot be NULL"); return -1; } - internal = git_vector_get(&cfg->files, 0); - if (!internal || !internal->file) - return config_error_nofiles(name); - file = internal->file; + if (get_backend_for_use(&file, cfg, name, BACKEND_USE_SET) < 0) + return GIT_ENOTFOUND; error = file->set(file, name, value); @@ -1032,12 +1057,9 @@ on_error: int git_config_set_multivar(git_config *cfg, const char *name, const char *regexp, const char *value) { git_config_backend *file; - file_internal *internal; - internal = git_vector_get(&cfg->files, 0); - if (!internal || !internal->file) - return config_error_nofiles(name); - file = internal->file; + if (get_backend_for_use(&file, cfg, name, BACKEND_USE_DELETE) < 0) + return GIT_ENOTFOUND; return file->set_multivar(file, name, regexp, value); } @@ -1045,12 +1067,9 @@ int git_config_set_multivar(git_config *cfg, const char *name, const char *regex int git_config_delete_multivar(git_config *cfg, const char *name, const char *regexp) { git_config_backend *file; - file_internal *internal; - internal = git_vector_get(&cfg->files, 0); - if (!internal || !internal->file) - return config_error_nofiles(name); - file = internal->file; + if (get_backend_for_use(&file, cfg, name, BACKEND_USE_DELETE) < 0) + return GIT_ENOTFOUND; return file->del_multivar(file, name, regexp); } From 2a7086fa890a7c4876699a3b3ac1561dfd71ccf6 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 25 Apr 2017 13:23:04 +0200 Subject: [PATCH 3/3] tests: config: verify functionality with read-only backends --- tests/config/readonly.c | 64 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 tests/config/readonly.c diff --git a/tests/config/readonly.c b/tests/config/readonly.c new file mode 100644 index 000000000..f45abdd29 --- /dev/null +++ b/tests/config/readonly.c @@ -0,0 +1,64 @@ +#include "clar_libgit2.h" +#include "config_file.h" +#include "config.h" + +static git_config *cfg; + +void test_config_readonly__initialize(void) +{ + cl_git_pass(git_config_new(&cfg)); +} + +void test_config_readonly__cleanup(void) +{ + git_config_free(cfg); + cfg = NULL; +} + +void test_config_readonly__writing_to_readonly_fails(void) +{ + git_config_backend *backend; + + cl_git_pass(git_config_file__ondisk(&backend, "global")); + backend->readonly = 1; + cl_git_pass(git_config_add_backend(cfg, backend, GIT_CONFIG_LEVEL_GLOBAL, 0)); + + cl_git_fail_with(GIT_ENOTFOUND, git_config_set_string(cfg, "foo.bar", "baz")); + cl_assert(!git_path_exists("global")); +} + +void test_config_readonly__writing_to_cfg_with_rw_precedence_succeeds(void) +{ + git_config_backend *backend; + + cl_git_pass(git_config_file__ondisk(&backend, "global")); + backend->readonly = 1; + cl_git_pass(git_config_add_backend(cfg, backend, GIT_CONFIG_LEVEL_GLOBAL, 0)); + + cl_git_pass(git_config_file__ondisk(&backend, "local")); + cl_git_pass(git_config_add_backend(cfg, backend, GIT_CONFIG_LEVEL_LOCAL, 0)); + + cl_git_pass(git_config_set_string(cfg, "foo.bar", "baz")); + + cl_assert(git_path_exists("local")); + cl_assert(!git_path_exists("global")); + cl_git_pass(p_unlink("local")); +} + +void test_config_readonly__writing_to_cfg_with_ro_precedence_succeeds(void) +{ + git_config_backend *backend; + + cl_git_pass(git_config_file__ondisk(&backend, "local")); + backend->readonly = 1; + cl_git_pass(git_config_add_backend(cfg, backend, GIT_CONFIG_LEVEL_LOCAL, 0)); + + cl_git_pass(git_config_file__ondisk(&backend, "global")); + cl_git_pass(git_config_add_backend(cfg, backend, GIT_CONFIG_LEVEL_GLOBAL, 0)); + + cl_git_pass(git_config_set_string(cfg, "foo.bar", "baz")); + + cl_assert(!git_path_exists("local")); + cl_assert(git_path_exists("global")); + cl_git_pass(p_unlink("global")); +}