Merge pull request #758 from libgit2/config-values-containing-quotes Quotes inside config values don't survive serialization/deserialization
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163
diff --git a/src/config_file.c b/src/config_file.c
index 1c748fa..fd1aa8d 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;
diff --git a/tests-clar/config/write.c b/tests-clar/config/write.c
index f877447..13b669c 100644
--- a/tests-clar/config/write.c
+++ b/tests-clar/config/write.c
@@ -90,3 +90,49 @@ 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);
+
+ /* 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);
+}