From 750be86aedb867a43680f872e1c9824379644739 Mon Sep 17 00:00:00 2001 From: Adam Roben Date: Sat, 9 Jun 2012 12:45:21 -0400 Subject: [PATCH 1/3] Add a test that shows we don't preserve quotes in config values --- tests-clar/config/write.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests-clar/config/write.c b/tests-clar/config/write.c index f8774473e..04811d7f0 100644 --- a/tests-clar/config/write.c +++ b/tests-clar/config/write.c @@ -90,3 +90,20 @@ void test_config_write__delete_inexistent(void) cl_assert(git_config_delete(cfg, "core.imaginary") == GIT_ENOTFOUND); git_config_free(cfg); } + +void test_config_write__value_containing_quotes(void) +{ + git_config *cfg; + const char* str; + + cl_git_pass(git_config_open_ondisk(&cfg, "config9")); + cl_git_pass(git_config_set_string(cfg, "core.somevar", "this \"has\" quotes")); + cl_git_pass(git_config_get_string(&str, cfg, "core.somevar")); + cl_assert_equal_s(str, "this \"has\" quotes"); + git_config_free(cfg); + + cl_git_pass(git_config_open_ondisk(&cfg, "config9")); + cl_git_pass(git_config_get_string(&str, cfg, "core.somevar")); + cl_assert_equal_s(str, "this \"has\" quotes"); + git_config_free(cfg); +} From 49938cad9133dc6bb2120fc3e339cb0132ea71cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 11 Jun 2012 16:28:51 +0200 Subject: [PATCH 2/3] config: correctly escape quotes in the value When a configuration option is set, we didn't check to see whether there was any escaping needed. Escape the available characters so we can unescape them correctly when we read them. --- src/config_file.c | 53 +++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 6 deletions(-) diff --git a/src/config_file.c b/src/config_file.c index 1c748fad1..fd1aa8d08 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -87,6 +87,7 @@ typedef struct { static int config_parse(diskfile_backend *cfg_file); static int parse_variable(diskfile_backend *cfg, char **var_name, char **var_value); static int config_write(diskfile_backend *cfg, const char *key, const regex_t *preg, const char *value); +static char *escape_value(const char *ptr); static void set_parse_error(diskfile_backend *backend, int col, const char *error_str) { @@ -212,9 +213,9 @@ static int config_set(git_config_file *cfg, const char *name, const char *value) { cvar_t *var = NULL, *old_var; diskfile_backend *b = (diskfile_backend *)cfg; - char *key; + char *key, *esc_value = NULL; khiter_t pos; - int rval; + int rval, ret; if (normalize_name(name, &key) < 0) return -1; @@ -237,12 +238,17 @@ static int config_set(git_config_file *cfg, const char *name, const char *value) if (value) { tmp = git__strdup(value); GITERR_CHECK_ALLOC(tmp); + esc_value = escape_value(value); + GITERR_CHECK_ALLOC(esc_value); } git__free(existing->value); existing->value = tmp; - return config_write(b, existing->key, NULL, value); + ret = config_write(b, existing->key, NULL, esc_value); + + git__free(esc_value); + return ret; } var = git__malloc(sizeof(cvar_t)); @@ -256,13 +262,17 @@ static int config_set(git_config_file *cfg, const char *name, const char *value) if (value) { var->value = git__strdup(value); GITERR_CHECK_ALLOC(var->value); + esc_value = escape_value(value); + GITERR_CHECK_ALLOC(esc_value); } - if (config_write(b, key, NULL, value) < 0) { + if (config_write(b, key, NULL, esc_value) < 0) { + git__free(esc_value); cvar_free(var); return -1; } + git__free(esc_value); git_strmap_insert2(b->values, key, var, old_var, rval); if (rval < 0) return -1; @@ -1155,13 +1165,44 @@ rewrite_fail: return -1; } +static const char *escapes = "ntb\"\\"; +static const char *escaped = "\n\t\b\"\\"; + +/* Escape the values to write them to the file */ +static char *escape_value(const char *ptr) +{ + git_buf buf = GIT_BUF_INIT; + size_t len; + const char *esc; + + assert(ptr); + + len = strlen(ptr); + git_buf_grow(&buf, len); + + while (*ptr != '\0') { + if ((esc = strchr(escaped, *ptr)) != NULL) { + git_buf_putc(&buf, '\\'); + git_buf_putc(&buf, escapes[esc - escaped]); + } else { + git_buf_putc(&buf, *ptr); + } + ptr++; + } + + if (git_buf_oom(&buf)) { + git_buf_free(&buf); + return NULL; + } + + return git_buf_detach(&buf); +} + /* '\"' -> '"' etc */ static char *fixup_line(const char *ptr, int quote_count) { char *str = git__malloc(strlen(ptr) + 1); char *out = str, *esc; - const char *escapes = "ntb\"\\"; - const char *escaped = "\n\t\b\"\\"; if (str == NULL) return NULL; From 67d334c1cd569d13b1709c3ef1864d630e608c95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 11 Jun 2012 16:57:02 +0200 Subject: [PATCH 3/3] config: add more tests for writing escaped chars --- tests-clar/config/write.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/tests-clar/config/write.c b/tests-clar/config/write.c index 04811d7f0..13b669cb2 100644 --- a/tests-clar/config/write.c +++ b/tests-clar/config/write.c @@ -106,4 +106,33 @@ void test_config_write__value_containing_quotes(void) cl_git_pass(git_config_get_string(&str, cfg, "core.somevar")); cl_assert_equal_s(str, "this \"has\" quotes"); git_config_free(cfg); + + /* The code path for values that already exist is different, check that one as well */ + cl_git_pass(git_config_open_ondisk(&cfg, "config9")); + cl_git_pass(git_config_set_string(cfg, "core.somevar", "this also \"has\" quotes")); + cl_git_pass(git_config_get_string(&str, cfg, "core.somevar")); + cl_assert_equal_s(str, "this also \"has\" quotes"); + git_config_free(cfg); + + cl_git_pass(git_config_open_ondisk(&cfg, "config9")); + cl_git_pass(git_config_get_string(&str, cfg, "core.somevar")); + cl_assert_equal_s(str, "this also \"has\" quotes"); + git_config_free(cfg); +} + +void test_config_write__escape_value(void) +{ + git_config *cfg; + const char* str; + + cl_git_pass(git_config_open_ondisk(&cfg, "config9")); + cl_git_pass(git_config_set_string(cfg, "core.somevar", "this \"has\" quotes and \t")); + cl_git_pass(git_config_get_string(&str, cfg, "core.somevar")); + cl_assert_equal_s(str, "this \"has\" quotes and \t"); + git_config_free(cfg); + + cl_git_pass(git_config_open_ondisk(&cfg, "config9")); + cl_git_pass(git_config_get_string(&str, cfg, "core.somevar")); + cl_assert_equal_s(str, "this \"has\" quotes and \t"); + git_config_free(cfg); }