diff --git a/src/config_file.c b/src/config_file.c index 010c494eb..afd80d3d5 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -114,8 +114,7 @@ typedef struct { diskfile_backend *snapshot_from; } diskfile_readonly_backend; -static int config_parse(git_strmap *values, diskfile_backend *cfg_file, struct reader *reader, git_config_level_t level, int depth); -static int parse_variable(struct reader *reader, char **var_name, char **var_value); +static int config_read(git_strmap *values, diskfile_backend *cfg_file, struct reader *reader, git_config_level_t level, int depth); static int config_write(diskfile_backend *cfg, const char *key, const regex_t *preg, const char *value); static char *escape_value(const char *ptr); @@ -288,7 +287,7 @@ static int config_open(git_config_backend *cfg, git_config_level_t level) if (res == GIT_ENOTFOUND) return 0; - if (res < 0 || (res = config_parse(b->header.values->values, b, reader, level, 0)) < 0) { + if (res < 0 || (res = config_read(b->header.values->values, b, reader, level, 0)) < 0) { refcounted_strmap_free(b->header.values); b->header.values = NULL; } @@ -313,7 +312,7 @@ static int config__refresh(git_config_backend *cfg) reader = git_array_get(b->readers, git_array_size(b->readers) - 1); GITERR_CHECK_ALLOC(reader); - if ((error = config_parse(values->values, b, reader, b->level, 0)) < 0) + if ((error = config_read(values->values, b, reader, b->level, 0)) < 0) goto out; git_mutex_lock(&b->header.values_mutex); @@ -831,7 +830,7 @@ static int reader_getchar_raw(struct reader *reader) if (c == 0) { reader->eof = 1; - c = '\n'; + c = '\0'; } return c; @@ -850,13 +849,12 @@ static int reader_getchar(struct reader *reader, int flags) do { c = reader_getchar_raw(reader); - } while (skip_whitespace && git__isspace(c) && - !reader->eof); + } while (c != '\n' && c != '\0' && skip_whitespace && git__isspace(c)); if (skip_comments && (c == '#' || c == ';')) { do { c = reader_getchar_raw(reader); - } while (c != '\n'); + } while (c != '\n' && c != '\0'); } return c; @@ -1200,398 +1198,6 @@ static int included_path(git_buf *out, const char *dir, const char *path) return git_path_join_unrooted(out, path, dir, NULL); } -static int config_parse(git_strmap *values, diskfile_backend *cfg_file, struct reader *reader, git_config_level_t level, int depth) -{ - int c; - char *current_section = NULL; - char *var_name; - char *var_value; - cvar_t *var; - git_buf buf = GIT_BUF_INIT; - int result = 0; - uint32_t reader_idx; - - if (depth >= MAX_INCLUDE_DEPTH) { - giterr_set(GITERR_CONFIG, "Maximum config include depth reached"); - return -1; - } - - reader_idx = git_array_size(cfg_file->readers) - 1; - /* Initialize the reading position */ - reader->read_ptr = reader->buffer.ptr; - reader->eof = 0; - - /* If the file is empty, there's nothing for us to do */ - if (*reader->read_ptr == '\0') - return 0; - - skip_bom(reader); - - while (result == 0 && !reader->eof) { - - c = reader_peek(reader, SKIP_WHITESPACE); - - switch (c) { - case '\n': /* EOF when peeking, set EOF in the reader to exit the loop */ - reader->eof = 1; - break; - - case '[': /* section header, new section begins */ - git__free(current_section); - current_section = NULL; - result = parse_section_header(reader, ¤t_section); - break; - - case ';': - case '#': - reader_consume_line(reader); - break; - - default: /* assume variable declaration */ - result = parse_variable(reader, &var_name, &var_value); - if (result < 0) - break; - - git__strtolower(var_name); - git_buf_printf(&buf, "%s.%s", current_section, var_name); - git__free(var_name); - - if (git_buf_oom(&buf)) { - git__free(var_value); - return -1; - } - - var = git__calloc(1, sizeof(cvar_t)); - GITERR_CHECK_ALLOC(var); - var->entry = git__calloc(1, sizeof(git_config_entry)); - GITERR_CHECK_ALLOC(var->entry); - - var->entry->name = git_buf_detach(&buf); - var->entry->value = var_value; - var->entry->level = level; - var->included = !!depth; - - - if ((result = append_entry(values, var)) < 0) - break; - else - result = 0; - - /* Add or append the new config option */ - if (!git__strcmp(var->entry->name, "include.path")) { - struct reader *r; - git_buf path = GIT_BUF_INIT; - char *dir; - uint32_t index; - - r = git_array_alloc(cfg_file->readers); - /* The reader may have been reallocated */ - reader = git_array_get(cfg_file->readers, reader_idx); - memset(r, 0, sizeof(struct reader)); - if ((result = git_path_dirname_r(&path, reader->file_path)) < 0) - break; - - /* We need to know our index in the array, as the next config_parse call may realloc */ - index = git_array_size(cfg_file->readers) - 1; - dir = git_buf_detach(&path); - result = included_path(&path, dir, var->entry->value); - git__free(dir); - - if (result < 0) - break; - - r->file_path = git_buf_detach(&path); - git_buf_init(&r->buffer, 0); - result = git_futils_readbuffer_updated(&r->buffer, r->file_path, &r->file_mtime, - &r->file_size, NULL); - - if (result == 0) { - result = config_parse(values, cfg_file, r, level, depth+1); - r = git_array_get(cfg_file->readers, index); - reader = git_array_get(cfg_file->readers, reader_idx); - } - else if (result == GIT_ENOTFOUND) { - giterr_clear(); - result = 0; - } - - git_buf_free(&r->buffer); - - if (result < 0) - break; - } - - break; - } - } - - git__free(current_section); - return result; -} - -static int write_section(git_filebuf *file, const char *key) -{ - int result; - const char *dot; - git_buf buf = GIT_BUF_INIT; - - /* All of this just for [section "subsection"] */ - dot = strchr(key, '.'); - git_buf_putc(&buf, '['); - if (dot == NULL) { - git_buf_puts(&buf, key); - } else { - char *escaped; - git_buf_put(&buf, key, dot - key); - escaped = escape_value(dot + 1); - GITERR_CHECK_ALLOC(escaped); - git_buf_printf(&buf, " \"%s\"", escaped); - git__free(escaped); - } - git_buf_puts(&buf, "]\n"); - - if (git_buf_oom(&buf)) - return -1; - - result = git_filebuf_write(file, git_buf_cstr(&buf), buf.size); - git_buf_free(&buf); - - return result; -} - -static const char *quotes_for_value(const char *value) -{ - const char *ptr; - - if (value[0] == ' ' || value[0] == '\0') - return "\""; - - for (ptr = value; *ptr; ++ptr) { - if (*ptr == ';' || *ptr == '#') - return "\""; - } - - if (ptr[-1] == ' ') - return "\""; - - return ""; -} - -/* - * This is pretty much the parsing, except we write out anything we don't have - */ -static int config_write(diskfile_backend *cfg, const char *key, const regex_t *preg, const char* value) -{ - int result, c; - int section_matches = 0, last_section_matched = 0, preg_replaced = 0, write_trailer = 0; - const char *pre_end = NULL, *post_start = NULL, *data_start, *write_start; - char *current_section = NULL, *section, *name, *ldot; - git_filebuf file = GIT_FILEBUF_INIT; - struct reader *reader = git_array_get(cfg->readers, 0); - - /* We need to read in our own config file */ - result = git_futils_readbuffer(&reader->buffer, cfg->file_path); - - /* Initialise the reading position */ - if (result == GIT_ENOTFOUND) { - reader->read_ptr = NULL; - reader->eof = 1; - data_start = NULL; - git_buf_clear(&reader->buffer); - } else if (result == 0) { - reader->read_ptr = reader->buffer.ptr; - reader->eof = 0; - data_start = reader->read_ptr; - } else { - return -1; /* OS error when reading the file */ - } - - write_start = data_start; - - /* Lock the file */ - if ((result = git_filebuf_open( - &file, cfg->file_path, 0, GIT_CONFIG_FILE_MODE)) < 0) { - git_buf_free(&reader->buffer); - return result; - } - - skip_bom(reader); - ldot = strrchr(key, '.'); - name = ldot + 1; - section = git__strndup(key, ldot - key); - - while (!reader->eof) { - c = reader_peek(reader, SKIP_WHITESPACE); - - if (c == '\n') { /* We've arrived at the end of the file */ - break; - - } else if (c == '[') { /* section header, new section begins */ - /* - * We set both positions to the current one in case we - * need to add a variable to the end of a section. In that - * case, we want both variables to point just before the - * new section. If we actually want to replace it, the - * default case will take care of updating them. - */ - pre_end = post_start = reader->read_ptr; - - git__free(current_section); - current_section = NULL; - if (parse_section_header(reader, ¤t_section) < 0) - goto rewrite_fail; - - /* Keep track of when it stops matching */ - last_section_matched = section_matches; - section_matches = !strcmp(current_section, section); - } - - else if (c == ';' || c == '#') { - reader_consume_line(reader); - } - - else { - /* - * If the section doesn't match, but the last section did, - * it means we need to add a variable (so skip the line - * otherwise). If both the section and name match, we need - * to overwrite the variable (so skip the line - * otherwise). pre_end needs to be updated each time so we - * don't loose that information, but we only need to - * update post_start if we're going to use it in this - * iteration. - * If the section doesn't match and we are trying to delete an entry - * (value == NULL), we must continue searching; there may be another - * matching section later. - */ - if (!section_matches) { - if (!last_section_matched || value == NULL) { - reader_consume_line(reader); - continue; - } - } else { - int has_matched = 0; - char *var_name, *var_value; - - pre_end = reader->read_ptr; - if (parse_variable(reader, &var_name, &var_value) < 0) - goto rewrite_fail; - - /* First try to match the name of the variable */ - if (strcasecmp(name, var_name) == 0) - has_matched = 1; - - /* If the name matches, and we have a regex to match the - * value, try to match it */ - if (has_matched && preg != NULL) - has_matched = (regexec(preg, var_value, 0, NULL, 0) == 0); - - git__free(var_name); - git__free(var_value); - - /* if there is no match, keep going */ - if (!has_matched) - continue; - - post_start = reader->read_ptr; - } - - /* We've found the variable we wanted to change, so - * write anything up to it */ - git_filebuf_write(&file, write_start, pre_end - write_start); - preg_replaced = 1; - - /* Then replace the variable. If the value is NULL, it - * means we want to delete it, so don't write anything. */ - if (value != NULL) { - const char *q = quotes_for_value(value); - git_filebuf_printf(&file, "\t%s = %s%s%s\n", name, q, value, q); - } - - /* - * If we have a multivar, we should keep looking for entries, - * but only if we're in the right section. Otherwise we'll end up - * looping on the edge of a matching and a non-matching section. - */ - if (section_matches && preg != NULL) { - write_start = post_start; - continue; - } - - write_trailer = 1; - break; /* break from the loop */ - } - } - - /* - * Being here can mean that - * - * 1) our section is the last one in the file and we're - * adding a variable - * - * 2) we didn't find a section for us so we need to create it - * ourselves. - * - * 3) we're setting a multivar with a regex, which means we - * continue to search for matching values - * - * In the last case, if we've already replaced a value, we - * want to write the rest of the file. Otherwise we need to write - * out the whole file and then the new variable. - */ - if (write_trailer) { - /* Write out rest of the file */ - git_filebuf_write(&file, post_start, reader->buffer.size - (post_start - data_start)); - } else { - if (preg_replaced) { - git_filebuf_printf(&file, "\n%s", write_start); - } else { - const char *q; - - git_filebuf_write(&file, reader->buffer.ptr, reader->buffer.size); - - if (reader->buffer.size > 0 && *(reader->buffer.ptr + reader->buffer.size - 1) != '\n') - git_filebuf_write(&file, "\n", 1); - - /* And now if we just need to add a variable */ - if (!section_matches && write_section(&file, section) < 0) - goto rewrite_fail; - - /* Sanity check: if we are here, and value is NULL, that means that somebody - * touched the config file after our initial read. We should probably assert() - * this, but instead we'll handle it gracefully with an error. */ - if (value == NULL) { - giterr_set(GITERR_CONFIG, - "race condition when writing a config file (a cvar has been removed)"); - goto rewrite_fail; - } - - /* If we are here, there is at least a section line */ - q = quotes_for_value(value); - git_filebuf_printf(&file, "\t%s = %s%s%s\n", name, q, value, q); - } - } - - git__free(section); - git__free(current_section); - - /* refresh stats - if this errors, then commit will error too */ - (void)git_filebuf_stats(&reader->file_mtime, &reader->file_size, &file); - - result = git_filebuf_commit(&file); - git_buf_free(&reader->buffer); - - return result; - -rewrite_fail: - git__free(section); - git__free(current_section); - - git_filebuf_cleanup(&file); - git_buf_free(&reader->buffer); - return -1; -} - static const char *escapes = "ntb\"\\"; static const char *escaped = "\n\t\b\"\\"; @@ -1813,3 +1419,435 @@ on_error: git__free(line); return -1; } + +static int config_parse( + struct reader *reader, + int (*on_section)(struct reader **reader, const char *current_section, const char *line, size_t line_len, void *data), + int (*on_variable)(struct reader **reader, const char *current_section, char *var_name, char *var_value, const char *line, size_t line_len, void *data), + int (*on_comment)(struct reader **reader, const char *line, size_t line_len, void *data), + int (*on_eof)(struct reader **reader, void *data), + void *data) +{ + char *current_section = NULL, *var_name, *var_value, *line_start; + char c; + size_t line_len; + int result = 0; + + skip_bom(reader); + + while (result == 0 && !reader->eof) { + line_start = reader->read_ptr; + + c = reader_peek(reader, SKIP_WHITESPACE); + + switch (c) { + case '\0': /* EOF when peeking, set EOF in the reader to exit the loop */ + reader->eof = 1; + break; + + case '[': /* section header, new section begins */ + git__free(current_section); + current_section = NULL; + + if ((result = parse_section_header(reader, ¤t_section)) == 0 && on_section) { + line_len = reader->read_ptr - line_start; + result = on_section(&reader, current_section, line_start, line_len, data); + } + break; + + case '\n': /* comment or whitespace-only */ + case ';': + case '#': + reader_consume_line(reader); + + if (on_comment) { + line_len = reader->read_ptr - line_start; + result = on_comment(&reader, line_start, line_len, data); + } + break; + + default: /* assume variable declaration */ + if ((result = parse_variable(reader, &var_name, &var_value)) == 0 && on_variable) { + line_len = reader->read_ptr - line_start; + result = on_variable(&reader, current_section, var_name, var_value, line_start, line_len, data); + } + break; + } + } + + if (on_eof) + result = on_eof(&reader, data); + + git__free(current_section); + return result; +} + +struct parse_data { + git_strmap *values; + diskfile_backend *cfg_file; + uint32_t reader_idx; + git_config_level_t level; + int depth; +}; + +static int read_on_variable( + struct reader **reader, + const char *current_section, + char *var_name, + char *var_value, + const char *line, + size_t line_len, + void *data) +{ + struct parse_data *parse_data = (struct parse_data *)data; + git_buf buf = GIT_BUF_INIT; + cvar_t *var; + int result = 0; + + GIT_UNUSED(line); + GIT_UNUSED(line_len); + + git__strtolower(var_name); + git_buf_printf(&buf, "%s.%s", current_section, var_name); + git__free(var_name); + + if (git_buf_oom(&buf)) { + git__free(var_value); + return -1; + } + + var = git__calloc(1, sizeof(cvar_t)); + GITERR_CHECK_ALLOC(var); + var->entry = git__calloc(1, sizeof(git_config_entry)); + GITERR_CHECK_ALLOC(var->entry); + + var->entry->name = git_buf_detach(&buf); + var->entry->value = var_value; + var->entry->level = parse_data->level; + var->included = !!parse_data->depth; + + if ((result = append_entry(parse_data->values, var)) < 0) + return result; + + result = 0; + + /* Add or append the new config option */ + if (!git__strcmp(var->entry->name, "include.path")) { + struct reader *r; + git_buf path = GIT_BUF_INIT; + char *dir; + uint32_t index; + + r = git_array_alloc(parse_data->cfg_file->readers); + /* The reader may have been reallocated */ + *reader = git_array_get(parse_data->cfg_file->readers, parse_data->reader_idx); + memset(r, 0, sizeof(struct reader)); + + if ((result = git_path_dirname_r(&path, (*reader)->file_path)) < 0) + return result; + + /* We need to know our index in the array, as the next config_parse call may realloc */ + index = git_array_size(parse_data->cfg_file->readers) - 1; + dir = git_buf_detach(&path); + result = included_path(&path, dir, var->entry->value); + git__free(dir); + + if (result < 0) + return result; + + r->file_path = git_buf_detach(&path); + git_buf_init(&r->buffer, 0); + + result = git_futils_readbuffer_updated( + &r->buffer, r->file_path, &r->file_mtime, &r->file_size, NULL); + + if (result == 0) { + result = config_read(parse_data->values, parse_data->cfg_file, r, parse_data->level, parse_data->depth+1); + r = git_array_get(parse_data->cfg_file->readers, index); + *reader = git_array_get(parse_data->cfg_file->readers, parse_data->reader_idx); + } else if (result == GIT_ENOTFOUND) { + giterr_clear(); + result = 0; + } + + git_buf_free(&r->buffer); + } + + return result; +} + +static int config_read(git_strmap *values, diskfile_backend *cfg_file, struct reader *reader, git_config_level_t level, int depth) +{ + struct parse_data parse_data; + + if (depth >= MAX_INCLUDE_DEPTH) { + giterr_set(GITERR_CONFIG, "Maximum config include depth reached"); + return -1; + } + + /* Initialize the reading position */ + reader->read_ptr = reader->buffer.ptr; + reader->eof = 0; + + /* If the file is empty, there's nothing for us to do */ + if (*reader->read_ptr == '\0') + return 0; + + parse_data.values = values; + parse_data.cfg_file = cfg_file; + parse_data.reader_idx = git_array_size(cfg_file->readers) - 1; + parse_data.level = level; + parse_data.depth = depth; + + return config_parse(reader, NULL, read_on_variable, NULL, NULL, &parse_data); +} + +static int write_section(git_filebuf *file, const char *key) +{ + int result; + const char *dot; + git_buf buf = GIT_BUF_INIT; + + /* All of this just for [section "subsection"] */ + dot = strchr(key, '.'); + git_buf_putc(&buf, '['); + if (dot == NULL) { + git_buf_puts(&buf, key); + } else { + char *escaped; + git_buf_put(&buf, key, dot - key); + escaped = escape_value(dot + 1); + GITERR_CHECK_ALLOC(escaped); + git_buf_printf(&buf, " \"%s\"", escaped); + git__free(escaped); + } + git_buf_puts(&buf, "]\n"); + + if (git_buf_oom(&buf)) + return -1; + + result = git_filebuf_write(file, git_buf_cstr(&buf), buf.size); + git_buf_free(&buf); + + return result; +} + +static const char *quotes_for_value(const char *value) +{ + const char *ptr; + + if (value[0] == ' ' || value[0] == '\0') + return "\""; + + for (ptr = value; *ptr; ++ptr) { + if (*ptr == ';' || *ptr == '#') + return "\""; + } + + if (ptr[-1] == ' ') + return "\""; + + return ""; +} + +struct write_data { + git_filebuf *file; + unsigned int in_section : 1, + preg_replaced : 1; + const char *section; + const char *name; + const regex_t *preg; + const char *value; +}; + +static int write_line(struct write_data *write_data, const char *line, size_t line_len) +{ + int result = git_filebuf_write(write_data->file, line, line_len); + + if (!result && line_len && line[line_len-1] != '\n') + result = git_filebuf_printf(write_data->file, "\n"); + + return result; +} + +static int write_value(struct write_data *write_data) +{ + const char *q; + int result; + + q = quotes_for_value(write_data->value); + result = git_filebuf_printf(write_data->file, + "\t%s = %s%s%s\n", write_data->name, q, write_data->value, q); + + /* If we are updating a single name/value, we're done. Setting `value` + * to `NULL` will prevent us from trying to write it again later (in + * `write_on_section`) if we see the same section repeated. + */ + if (!write_data->preg) + write_data->value = NULL; + + return result; +} + +static int write_on_section( + struct reader **reader, + const char *current_section, + const char *line, + size_t line_len, + void *data) +{ + struct write_data *write_data = (struct write_data *)data; + int result = 0; + + GIT_UNUSED(reader); + + /* If we were previously in the correct section (but aren't anymore) + * and haven't written our value (for a simple name/value set, not + * a multivar), then append it to the end of the section before writing + * the new one. + */ + if (write_data->in_section && !write_data->preg && write_data->value) + result = write_value(write_data); + + write_data->in_section = strcmp(current_section, write_data->section) == 0; + + if (!result) + result = write_line(write_data, line, line_len); + + return result; +} + +static int write_on_variable( + struct reader **reader, + const char *current_section, + char *var_name, + char *var_value, + const char *line, + size_t line_len, + void *data) +{ + struct write_data *write_data = (struct write_data *)data; + bool has_matched = false; + + GIT_UNUSED(reader); + GIT_UNUSED(current_section); + + /* See if we are to update this name/value pair; first examine name */ + if (write_data->in_section && + strcasecmp(write_data->name, var_name) == 0) + has_matched = true; + + /* If we have a regex to match the value, see if it matches */ + if (has_matched && write_data->preg != NULL) + has_matched = (regexec(write_data->preg, var_value, 0, NULL, 0) == 0); + + git__free(var_name); + git__free(var_value); + + /* If this isn't the name/value we're looking for, simply dump the + * existing data back out and continue on. + */ + if (!has_matched) + return write_line(write_data, line, line_len); + + write_data->preg_replaced = 1; + + /* If value is NULL, we are deleting this value; write nothing. */ + if (!write_data->value) + return 0; + + return write_value(write_data); +} + +static int write_on_comment(struct reader **reader, const char *line, size_t line_len, void *data) +{ + struct write_data *write_data; + + GIT_UNUSED(reader); + + write_data = (struct write_data *)data; + return write_line(write_data, line, line_len); +} + +static int write_on_eof(struct reader **reader, void *data) +{ + struct write_data *write_data = (struct write_data *)data; + int result = 0; + + GIT_UNUSED(reader); + + /* If we are at the EOF and have not written our value (again, for a + * simple name/value set, not a multivar) then we have never seen the + * section in question and should create a new section and write the + * value. + */ + if ((!write_data->preg || !write_data->preg_replaced) && write_data->value) { + if ((result = write_section(write_data->file, write_data->section)) == 0) + result = write_value(write_data); + } + + return result; +} + +/* + * This is pretty much the parsing, except we write out anything we don't have + */ +static int config_write(diskfile_backend *cfg, const char *key, const regex_t *preg, const char* value) +{ + int result; + char *section, *name, *ldot; + git_filebuf file = GIT_FILEBUF_INIT; + struct reader *reader = git_array_get(cfg->readers, 0); + struct write_data write_data; + + /* Lock the file */ + if ((result = git_filebuf_open( + &file, cfg->file_path, 0, GIT_CONFIG_FILE_MODE)) < 0) { + git_buf_free(&reader->buffer); + return result; + } + + /* We need to read in our own config file */ + result = git_futils_readbuffer(&reader->buffer, cfg->file_path); + + /* Initialise the reading position */ + if (result == GIT_ENOTFOUND) { + reader->read_ptr = NULL; + reader->eof = 1; + git_buf_clear(&reader->buffer); + } else if (result == 0) { + reader->read_ptr = reader->buffer.ptr; + reader->eof = 0; + } else { + git_filebuf_cleanup(&file); + return -1; /* OS error when reading the file */ + } + + ldot = strrchr(key, '.'); + name = ldot + 1; + section = git__strndup(key, ldot - key); + + write_data.file = &file; + write_data.section = section; + write_data.in_section = 0; + write_data.preg_replaced = 0; + write_data.name = name; + write_data.preg = preg; + write_data.value = value; + + if ((result = config_parse(reader, write_on_section, write_on_variable, write_on_comment, write_on_eof, &write_data)) < 0) { + git_filebuf_cleanup(&file); + goto done; + } + + /* refresh stats - if this errors, then commit will error too */ + (void)git_filebuf_stats(&reader->file_mtime, &reader->file_size, &file); + + result = git_filebuf_commit(&file); + git_buf_free(&reader->buffer); + +done: + git_buf_free(&reader->buffer); + return result; +} + diff --git a/tests/config/write.c b/tests/config/write.c index 1832466ae..2e7b8182a 100644 --- a/tests/config/write.c +++ b/tests/config/write.c @@ -145,6 +145,138 @@ void test_config_write__delete_value_with_duplicate_header(void) git_config_free(cfg); } +/* + * This test exposes a bug where duplicate section headers could cause + * config_write to add a new entry when one already exists. + */ +void test_config_write__add_value_with_duplicate_header(void) +{ + const char *file_name = "config-duplicate-insert"; + const char *entry_name = "foo.c"; + const char *old_val = "old"; + const char *new_val = "new"; + const char *str; + git_config *cfg, *snapshot; + + /* c = old should be replaced by c = new. + * The bug causes c = new to be inserted under the first 'foo' header. + */ + const char *file_content = + "[foo]\n" \ + " a = b\n" \ + "[other]\n" \ + " a = b\n" \ + "[foo]\n" \ + " c = old\n"; + + /* Write the test config */ + cl_git_mkfile(file_name, file_content); + cl_git_pass(git_config_open_ondisk(&cfg, file_name)); + + /* make sure the expected entry (foo.c) exists */ + cl_git_pass(git_config_snapshot(&snapshot, cfg)); + cl_git_pass(git_config_get_string(&str, snapshot, entry_name)); + cl_assert_equal_s(old_val, str); + git_config_free(snapshot); + + /* Try setting foo.c to something else */ + cl_git_pass(git_config_set_string(cfg, entry_name, new_val)); + git_config_free(cfg); + + /* Reopen the file and make sure the new value was set */ + cl_git_pass(git_config_open_ondisk(&cfg, file_name)); + cl_git_pass(git_config_snapshot(&snapshot, cfg)); + cl_git_pass(git_config_get_string(&str, snapshot, entry_name)); + cl_assert_equal_s(new_val, str); + + /* Cleanup */ + git_config_free(snapshot); + git_config_free(cfg); +} + +void test_config_write__overwrite_value_with_duplicate_header(void) +{ + const char *file_name = "config-duplicate-header"; + const char *entry_name = "remote.origin.url"; + git_config *cfg; + git_config_entry *entry; + + /* This config can occur after removing and re-adding the origin remote */ + const char *file_content = + "[remote \"origin\"]\n" \ + "[branch \"master\"]\n" \ + " remote = \"origin\"\n" \ + "[remote \"origin\"]\n" \ + " url = \"foo\"\n"; + + /* Write the test config and make sure the expected entry exists */ + cl_git_mkfile(file_name, file_content); + cl_git_pass(git_config_open_ondisk(&cfg, file_name)); + cl_git_pass(git_config_get_entry(&entry, cfg, entry_name)); + + /* Update that entry */ + cl_git_pass(git_config_set_string(cfg, entry_name, "newurl")); + + /* Reopen the file and make sure the entry was updated */ + git_config_entry_free(entry); + git_config_free(cfg); + cl_git_pass(git_config_open_ondisk(&cfg, file_name)); + cl_git_pass(git_config_get_entry(&entry, cfg, entry_name)); + + cl_assert_equal_s("newurl", entry->value); + + /* Cleanup */ + git_config_entry_free(entry); + git_config_free(cfg); +} + +static int multivar_cb(const git_config_entry *entry, void *data) +{ + int *n = (int *)data; + + cl_assert_equal_s(entry->value, "newurl"); + + (*n)++; + + return 0; +} + +void test_config_write__overwrite_multivar_within_duplicate_header(void) +{ + const char *file_name = "config-duplicate-header"; + const char *entry_name = "remote.origin.url"; + git_config *cfg; + git_config_entry *entry; + int n = 0; + + /* This config can occur after removing and re-adding the origin remote */ + const char *file_content = + "[remote \"origin\"]\n" \ + " url = \"bar\"\n" \ + "[branch \"master\"]\n" \ + " remote = \"origin\"\n" \ + "[remote \"origin\"]\n" \ + " url = \"foo\"\n"; + + /* Write the test config and make sure the expected entry exists */ + cl_git_mkfile(file_name, file_content); + cl_git_pass(git_config_open_ondisk(&cfg, file_name)); + cl_git_pass(git_config_get_entry(&entry, cfg, entry_name)); + + /* Update that entry */ + cl_git_pass(git_config_set_multivar(cfg, entry_name, ".*", "newurl")); + git_config_entry_free(entry); + git_config_free(cfg); + + /* Reopen the file and make sure the entry was updated */ + cl_git_pass(git_config_open_ondisk(&cfg, file_name)); + cl_git_pass(git_config_get_multivar_foreach(cfg, entry_name, NULL, multivar_cb, &n)); + cl_assert_equal_i(2, n); + + /* Cleanup */ + git_config_free(cfg); +} + void test_config_write__write_subsection(void) { git_config *cfg; @@ -395,6 +527,75 @@ void test_config_write__outside_change(void) git_config_free(cfg); } +#define SECTION_FOO \ + "\n" \ + " \n" \ + " [section \"foo\"] \n" \ + " # here's a comment\n" \ + "\tname = \"value\"\n" \ + " name2 = \"value2\"\n" \ + "; another comment!\n" + +#define SECTION_BAR \ + "[section \"bar\"]\t\n" \ + "\t \n" \ + " barname=\"value\"\n" + + +void test_config_write__preserves_whitespace_and_comments(void) +{ + const char *file_name = "config-duplicate-header"; + const char *n; + git_config *cfg; + git_buf newfile = GIT_BUF_INIT; + + /* This config can occur after removing and re-adding the origin remote */ + const char *file_content = SECTION_FOO SECTION_BAR; + + /* Write the test config and make sure the expected entry exists */ + cl_git_mkfile(file_name, file_content); + cl_git_pass(git_config_open_ondisk(&cfg, file_name)); + cl_git_pass(git_config_set_string(cfg, "section.foo.other", "otherval")); + cl_git_pass(git_config_set_string(cfg, "newsection.newname", "new_value")); + + /* Ensure that we didn't needlessly mangle the config file */ + cl_git_pass(git_futils_readbuffer(&newfile, file_name)); + n = newfile.ptr; + + cl_assert_equal_strn(SECTION_FOO, n, strlen(SECTION_FOO)); + n += strlen(SECTION_FOO); + + cl_assert_equal_strn("\tother = otherval\n", n, strlen("\tother = otherval\n")); + n += strlen("\tother = otherval\n"); + + cl_assert_equal_strn(SECTION_BAR, n, strlen(SECTION_BAR)); + n += strlen(SECTION_BAR); + + cl_assert_equal_s("[newsection]\n\tnewname = new_value\n", n); + + git_buf_free(&newfile); + git_config_free(cfg); +} + +void test_config_write__preserves_entry_with_name_only(void) +{ + const char *file_name = "config-empty-value"; + git_config *cfg; + git_buf newfile = GIT_BUF_INIT; + + /* Write the test config and make sure the expected entry exists */ + cl_git_mkfile(file_name, "[section \"foo\"]\n\tname\n"); + cl_git_pass(git_config_open_ondisk(&cfg, file_name)); + cl_git_pass(git_config_set_string(cfg, "newsection.newname", "new_value")); + cl_git_pass(git_config_set_string(cfg, "section.foo.other", "otherval")); + + cl_git_pass(git_futils_readbuffer(&newfile, file_name)); + cl_assert_equal_s("[section \"foo\"]\n\tname\n\tother = otherval\n[newsection]\n\tnewname = new_value\n", newfile.ptr); + + git_buf_free(&newfile); + git_config_free(cfg); +} + void test_config_write__to_empty_file(void) { git_config *cfg; @@ -428,3 +629,4 @@ void test_config_write__to_file_with_only_comment(void) git_buf_free(&result); } +