From d14d42440d8370f8fe5016a6e212d101745f70cc Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 19 Feb 2014 00:58:52 +0200 Subject: [PATCH 1/4] config: disallow relative include paths from blobs When we see a relative config include like: [include] path = foo we make it relative to the containing directory of the file that contains the snippet. This makes no sense for config read from a blob, as it is not on the filesystem. Something like "HEAD:some/path" could have a relative path within the tree, but: 1. It would not be part of include.path, which explicitly refers to the filesystem. 2. It would need different parsing rules anyway to determine that it is a tree path. The current code just uses the "name" field, which is wrong. Let's split that into "name" and "path" fields, use the latter for relative includes, and fill in only the former for blobs. Signed-off-by: Jeff King Signed-off-by: Kirill A. Shutemov Signed-off-by: Junio C Hamano --- config.c | 10 ++++++---- t/t1305-config-include.sh | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/config.c b/config.c index d969a5aefc..b295310d3c 100644 --- a/config.c +++ b/config.c @@ -21,6 +21,7 @@ struct config_source { } buf; } u; const char *name; + const char *path; int die_on_error; int linenr; int eof; @@ -97,12 +98,12 @@ static int handle_path_include(const char *path, struct config_include_data *inc if (!is_absolute_path(path)) { char *slash; - if (!cf || !cf->name) + if (!cf || !cf->path) return error("relative config includes must come from files"); - slash = find_last_dir_sep(cf->name); + slash = find_last_dir_sep(cf->path); if (slash) - strbuf_add(&buf, cf->name, slash - cf->name + 1); + strbuf_add(&buf, cf->path, slash - cf->path + 1); strbuf_addstr(&buf, path); path = buf.buf; } @@ -1040,7 +1041,7 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data) struct config_source top; top.u.file = f; - top.name = filename; + top.name = top.path = filename; top.die_on_error = 1; top.do_fgetc = config_file_fgetc; top.do_ungetc = config_file_ungetc; @@ -1062,6 +1063,7 @@ int git_config_from_buf(config_fn_t fn, const char *name, const char *buf, top.u.buf.len = len; top.u.buf.pos = 0; top.name = name; + top.path = NULL; top.die_on_error = 0; top.do_fgetc = config_buf_fgetc; top.do_ungetc = config_buf_ungetc; diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh index a70707620f..6edd38b39a 100755 --- a/t/t1305-config-include.sh +++ b/t/t1305-config-include.sh @@ -122,6 +122,22 @@ test_expect_success 'relative includes from command line fail' ' test_must_fail git -c include.path=one config test.one ' +test_expect_success 'absolute includes from blobs work' ' + echo "[test]one = 1" >one && + echo "[include]path=$(pwd)/one" >blob && + blob=$(git hash-object -w blob) && + echo 1 >expect && + git config --blob=$blob test.one >actual && + test_cmp expect actual +' + +test_expect_success 'relative includes from blobs fail' ' + echo "[test]one = 1" >one && + echo "[include]path=one" >blob && + blob=$(git hash-object -w blob) && + test_must_fail git config --blob=$blob test.one +' + test_expect_success 'include cycles are detected' ' cat >.gitconfig <<-\EOF && [test]value = gitconfig From 6aea9f0fdd2f013e9b63137727c73da02d42a1be Mon Sep 17 00:00:00 2001 From: "Kirill A. Shutemov" Date: Wed, 19 Feb 2014 00:58:53 +0200 Subject: [PATCH 2/4] builtin/config.c: rename check_blob_write() -> check_write() The function will be reused to check for other conditions which prevent write. Signed-off-by: Kirill A. Shutemov Signed-off-by: Junio C Hamano --- builtin/config.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/builtin/config.c b/builtin/config.c index 92ebf23f0a..a7c55e6888 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -362,7 +362,7 @@ static int get_colorbool(int print) return get_colorbool_found ? 0 : 1; } -static void check_blob_write(void) +static void check_write(void) { if (given_config_blob) die("writing config blobs is not supported"); @@ -572,7 +572,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) } else if (actions == ACTION_SET) { int ret; - check_blob_write(); + check_write(); check_argc(argc, 2, 2); value = normalize_value(argv[0], argv[1]); ret = git_config_set_in_file(given_config_file, argv[0], value); @@ -582,21 +582,21 @@ int cmd_config(int argc, const char **argv, const char *prefix) return ret; } else if (actions == ACTION_SET_ALL) { - check_blob_write(); + check_write(); check_argc(argc, 2, 3); value = normalize_value(argv[0], argv[1]); return git_config_set_multivar_in_file(given_config_file, argv[0], value, argv[2], 0); } else if (actions == ACTION_ADD) { - check_blob_write(); + check_write(); check_argc(argc, 2, 2); value = normalize_value(argv[0], argv[1]); return git_config_set_multivar_in_file(given_config_file, argv[0], value, "^$", 0); } else if (actions == ACTION_REPLACE_ALL) { - check_blob_write(); + check_write(); check_argc(argc, 2, 3); value = normalize_value(argv[0], argv[1]); return git_config_set_multivar_in_file(given_config_file, @@ -623,7 +623,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) return get_urlmatch(argv[0], argv[1]); } else if (actions == ACTION_UNSET) { - check_blob_write(); + check_write(); check_argc(argc, 1, 2); if (argc == 2) return git_config_set_multivar_in_file(given_config_file, @@ -633,14 +633,14 @@ int cmd_config(int argc, const char **argv, const char *prefix) argv[0], NULL); } else if (actions == ACTION_UNSET_ALL) { - check_blob_write(); + check_write(); check_argc(argc, 1, 2); return git_config_set_multivar_in_file(given_config_file, argv[0], NULL, argv[1], 1); } else if (actions == ACTION_RENAME_SECTION) { int ret; - check_blob_write(); + check_write(); check_argc(argc, 2, 2); ret = git_config_rename_section_in_file(given_config_file, argv[0], argv[1]); @@ -651,7 +651,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) } else if (actions == ACTION_REMOVE_SECTION) { int ret; - check_blob_write(); + check_write(); check_argc(argc, 1, 1); ret = git_config_rename_section_in_file(given_config_file, argv[0], NULL); From c8985ce05360857733738561dd6cdf964470cbdf Mon Sep 17 00:00:00 2001 From: "Kirill A. Shutemov" Date: Wed, 19 Feb 2014 00:58:54 +0200 Subject: [PATCH 3/4] config: change git_config_with_options() interface We're going to have more options for config source. Let's alter git_config_with_options() interface to accept struct with all source options. Signed-off-by: Kirill A. Shutemov Signed-off-by: Junio C Hamano --- builtin/config.c | 75 ++++++++++++++++++++++-------------------------- cache.h | 8 ++++-- config.c | 13 ++++----- 3 files changed, 47 insertions(+), 49 deletions(-) diff --git a/builtin/config.c b/builtin/config.c index a7c55e6888..de41ba50e9 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -21,8 +21,7 @@ static char key_delim = ' '; static char term = '\n'; static int use_global_config, use_system_config, use_local_config; -static const char *given_config_file; -static const char *given_config_blob; +static struct git_config_source given_config_source; static int actions, types; static const char *get_color_slot, *get_colorbool_slot; static int end_null; @@ -55,8 +54,8 @@ static struct option builtin_config_options[] = { OPT_BOOL(0, "global", &use_global_config, N_("use global config file")), OPT_BOOL(0, "system", &use_system_config, N_("use system config file")), OPT_BOOL(0, "local", &use_local_config, N_("use repository config file")), - OPT_STRING('f', "file", &given_config_file, N_("file"), N_("use given config file")), - OPT_STRING(0, "blob", &given_config_blob, N_("blob-id"), N_("read config from given blob object")), + OPT_STRING('f', "file", &given_config_source.file, N_("file"), N_("use given config file")), + OPT_STRING(0, "blob", &given_config_source.blob, N_("blob-id"), N_("read config from given blob object")), OPT_GROUP(N_("Action")), OPT_BIT(0, "get", &actions, N_("get value: name [value-regex]"), ACTION_GET), OPT_BIT(0, "get-all", &actions, N_("get all values: key [value-regex]"), ACTION_GET_ALL), @@ -224,8 +223,7 @@ static int get_value(const char *key_, const char *regex_) } git_config_with_options(collect_config, &values, - given_config_file, given_config_blob, - respect_includes); + &given_config_source, respect_includes); ret = !values.nr; @@ -309,8 +307,7 @@ static void get_color(const char *def_color) get_color_found = 0; parsed_color[0] = '\0'; git_config_with_options(git_get_color_config, NULL, - given_config_file, given_config_blob, - respect_includes); + &given_config_source, respect_includes); if (!get_color_found && def_color) color_parse(def_color, "command line", parsed_color); @@ -339,8 +336,7 @@ static int get_colorbool(int print) get_diff_color_found = -1; get_color_ui_found = -1; git_config_with_options(git_get_colorbool_config, NULL, - given_config_file, given_config_blob, - respect_includes); + &given_config_source, respect_includes); if (get_colorbool_found < 0) { if (!strcmp(get_colorbool_slot, "color.diff")) @@ -364,7 +360,7 @@ static int get_colorbool(int print) static void check_write(void) { - if (given_config_blob) + if (given_config_source.blob) die("writing config blobs is not supported"); } @@ -435,7 +431,7 @@ static int get_urlmatch(const char *var, const char *url) } git_config_with_options(urlmatch_config_entry, &config, - given_config_file, NULL, respect_includes); + &given_config_source, respect_includes); for_each_string_list_item(item, &values) { struct urlmatch_current_candidate_value *matched = item->util; @@ -464,14 +460,14 @@ int cmd_config(int argc, const char **argv, const char *prefix) int nongit = !startup_info->have_repository; char *value; - given_config_file = getenv(CONFIG_ENVIRONMENT); + given_config_source.file = getenv(CONFIG_ENVIRONMENT); argc = parse_options(argc, argv, prefix, builtin_config_options, builtin_config_usage, PARSE_OPT_STOP_AT_NON_OPTION); if (use_global_config + use_system_config + use_local_config + - !!given_config_file + !!given_config_blob > 1) { + !!given_config_source.file + !!given_config_source.blob > 1) { error("only one config file at a time."); usage_with_options(builtin_config_usage, builtin_config_options); } @@ -493,24 +489,24 @@ int cmd_config(int argc, const char **argv, const char *prefix) if (access_or_warn(user_config, R_OK, 0) && xdg_config && !access_or_warn(xdg_config, R_OK, 0)) - given_config_file = xdg_config; + given_config_source.file = xdg_config; else - given_config_file = user_config; + given_config_source.file = user_config; } else if (use_system_config) - given_config_file = git_etc_gitconfig(); + given_config_source.file = git_etc_gitconfig(); else if (use_local_config) - given_config_file = git_pathdup("config"); - else if (given_config_file) { - if (!is_absolute_path(given_config_file) && prefix) - given_config_file = + given_config_source.file = git_pathdup("config"); + else if (given_config_source.file) { + if (!is_absolute_path(given_config_source.file) && prefix) + given_config_source.file = xstrdup(prefix_filename(prefix, strlen(prefix), - given_config_file)); + given_config_source.file)); } if (respect_includes == -1) - respect_includes = !given_config_file; + respect_includes = !given_config_source.file; if (end_null) { term = '\0'; @@ -549,25 +545,24 @@ int cmd_config(int argc, const char **argv, const char *prefix) if (actions == ACTION_LIST) { check_argc(argc, 0, 0); if (git_config_with_options(show_all_config, NULL, - given_config_file, - given_config_blob, + &given_config_source, respect_includes) < 0) { - if (given_config_file) + if (given_config_source.file) die_errno("unable to read config file '%s'", - given_config_file); + given_config_source.file); else die("error processing config file(s)"); } } else if (actions == ACTION_EDIT) { check_argc(argc, 0, 0); - if (!given_config_file && nongit) + if (!given_config_source.file && nongit) die("not in a git directory"); - if (given_config_blob) + if (given_config_source.blob) die("editing blobs is not supported"); git_config(git_default_config, NULL); - launch_editor(given_config_file ? - given_config_file : git_path("config"), + launch_editor(given_config_source.file ? + given_config_source.file : git_path("config"), NULL, NULL); } else if (actions == ACTION_SET) { @@ -575,7 +570,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) check_write(); check_argc(argc, 2, 2); value = normalize_value(argv[0], argv[1]); - ret = git_config_set_in_file(given_config_file, argv[0], value); + ret = git_config_set_in_file(given_config_source.file, argv[0], value); if (ret == CONFIG_NOTHING_SET) error("cannot overwrite multiple values with a single value\n" " Use a regexp, --add or --replace-all to change %s.", argv[0]); @@ -585,21 +580,21 @@ int cmd_config(int argc, const char **argv, const char *prefix) check_write(); check_argc(argc, 2, 3); value = normalize_value(argv[0], argv[1]); - return git_config_set_multivar_in_file(given_config_file, + return git_config_set_multivar_in_file(given_config_source.file, argv[0], value, argv[2], 0); } else if (actions == ACTION_ADD) { check_write(); check_argc(argc, 2, 2); value = normalize_value(argv[0], argv[1]); - return git_config_set_multivar_in_file(given_config_file, + return git_config_set_multivar_in_file(given_config_source.file, argv[0], value, "^$", 0); } else if (actions == ACTION_REPLACE_ALL) { check_write(); check_argc(argc, 2, 3); value = normalize_value(argv[0], argv[1]); - return git_config_set_multivar_in_file(given_config_file, + return git_config_set_multivar_in_file(given_config_source.file, argv[0], value, argv[2], 1); } else if (actions == ACTION_GET) { @@ -626,23 +621,23 @@ int cmd_config(int argc, const char **argv, const char *prefix) check_write(); check_argc(argc, 1, 2); if (argc == 2) - return git_config_set_multivar_in_file(given_config_file, + return git_config_set_multivar_in_file(given_config_source.file, argv[0], NULL, argv[1], 0); else - return git_config_set_in_file(given_config_file, + return git_config_set_in_file(given_config_source.file, argv[0], NULL); } else if (actions == ACTION_UNSET_ALL) { check_write(); check_argc(argc, 1, 2); - return git_config_set_multivar_in_file(given_config_file, + return git_config_set_multivar_in_file(given_config_source.file, argv[0], NULL, argv[1], 1); } else if (actions == ACTION_RENAME_SECTION) { int ret; check_write(); check_argc(argc, 2, 2); - ret = git_config_rename_section_in_file(given_config_file, + ret = git_config_rename_section_in_file(given_config_source.file, argv[0], argv[1]); if (ret < 0) return ret; @@ -653,7 +648,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) int ret; check_write(); check_argc(argc, 1, 1); - ret = git_config_rename_section_in_file(given_config_file, + ret = git_config_rename_section_in_file(given_config_source.file, argv[0], NULL); if (ret < 0) return ret; diff --git a/cache.h b/cache.h index dc040fb1aa..9d94bd69f7 100644 --- a/cache.h +++ b/cache.h @@ -1146,6 +1146,11 @@ extern int update_server_info(int); #define CONFIG_INVALID_PATTERN 6 #define CONFIG_GENERIC_ERROR 7 +struct git_config_source { + const char *file; + const char *blob; +}; + typedef int (*config_fn_t)(const char *, const char *, void *); extern int git_default_config(const char *, const char *, void *); extern int git_config_from_file(config_fn_t fn, const char *, void *); @@ -1155,8 +1160,7 @@ extern void git_config_push_parameter(const char *text); extern int git_config_from_parameters(config_fn_t fn, void *data); extern int git_config(config_fn_t fn, void *); extern int git_config_with_options(config_fn_t fn, void *, - const char *filename, - const char *blob_ref, + struct git_config_source *config_source, int respect_includes); extern int git_config_early(config_fn_t fn, void *, const char *repo_config); extern int git_parse_ulong(const char *, unsigned long *); diff --git a/config.c b/config.c index b295310d3c..a21b0ddade 100644 --- a/config.c +++ b/config.c @@ -1172,8 +1172,7 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config) } int git_config_with_options(config_fn_t fn, void *data, - const char *filename, - const char *blob_ref, + struct git_config_source *config_source, int respect_includes) { char *repo_config = NULL; @@ -1191,10 +1190,10 @@ int git_config_with_options(config_fn_t fn, void *data, * If we have a specific filename, use it. Otherwise, follow the * regular lookup sequence. */ - if (filename) - return git_config_from_file(fn, filename, data); - else if (blob_ref) - return git_config_from_blob_ref(fn, blob_ref, data); + if (config_source && config_source->file) + return git_config_from_file(fn, config_source->file, data); + else if (config_source && config_source->blob) + return git_config_from_blob_ref(fn, config_source->blob, data); repo_config = git_pathdup("config"); ret = git_config_early(fn, data, repo_config); @@ -1205,7 +1204,7 @@ int git_config_with_options(config_fn_t fn, void *data, int git_config(config_fn_t fn, void *data) { - return git_config_with_options(fn, data, NULL, NULL, 1); + return git_config_with_options(fn, data, NULL, 1); } /* From 3caec73b5568341c5d8f303692423a8e9fb0cb39 Mon Sep 17 00:00:00 2001 From: "Kirill A. Shutemov" Date: Wed, 19 Feb 2014 00:58:55 +0200 Subject: [PATCH 4/4] config: teach "git config --file -" to read from the standard input The patch extends git config --file interface to allow read config from stdin. Editing stdin or setting value in stdin is an error. Include by absolute path is allowed in stdin config, but not by relative path. Signed-off-by: Kirill A. Shutemov Signed-off-by: Junio C Hamano --- builtin/config.c | 11 ++++++++++ cache.h | 1 + config.c | 43 +++++++++++++++++++++++++-------------- t/t1300-repo-config.sh | 17 ++++++++++++++-- t/t1305-config-include.sh | 16 ++++++++++++++- 5 files changed, 70 insertions(+), 18 deletions(-) diff --git a/builtin/config.c b/builtin/config.c index de41ba50e9..5677c942b6 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -360,6 +360,9 @@ static int get_colorbool(int print) static void check_write(void) { + if (given_config_source.use_stdin) + die("writing to stdin is not supported"); + if (given_config_source.blob) die("writing config blobs is not supported"); } @@ -472,6 +475,12 @@ int cmd_config(int argc, const char **argv, const char *prefix) usage_with_options(builtin_config_usage, builtin_config_options); } + if (given_config_source.file && + !strcmp(given_config_source.file, "-")) { + given_config_source.file = NULL; + given_config_source.use_stdin = 1; + } + if (use_global_config) { char *user_config = NULL; char *xdg_config = NULL; @@ -558,6 +567,8 @@ int cmd_config(int argc, const char **argv, const char *prefix) check_argc(argc, 0, 0); if (!given_config_source.file && nongit) die("not in a git directory"); + if (given_config_source.use_stdin) + die("editing stdin is not supported"); if (given_config_source.blob) die("editing blobs is not supported"); git_config(git_default_config, NULL); diff --git a/cache.h b/cache.h index 9d94bd69f7..4db19b5370 100644 --- a/cache.h +++ b/cache.h @@ -1147,6 +1147,7 @@ extern int update_server_info(int); #define CONFIG_GENERIC_ERROR 7 struct git_config_source { + unsigned int use_stdin:1; const char *file; const char *blob; }; diff --git a/config.c b/config.c index a21b0ddade..7b1febdf4a 100644 --- a/config.c +++ b/config.c @@ -1031,24 +1031,35 @@ static int do_config_from(struct config_source *top, config_fn_t fn, void *data) return ret; } +static int do_config_from_file(config_fn_t fn, + const char *name, const char *path, FILE *f, void *data) +{ + struct config_source top; + + top.u.file = f; + top.name = name; + top.path = path; + top.die_on_error = 1; + top.do_fgetc = config_file_fgetc; + top.do_ungetc = config_file_ungetc; + top.do_ftell = config_file_ftell; + + return do_config_from(&top, fn, data); +} + +static int git_config_from_stdin(config_fn_t fn, void *data) +{ + return do_config_from_file(fn, "", NULL, stdin, data); +} + int git_config_from_file(config_fn_t fn, const char *filename, void *data) { - int ret; - FILE *f = fopen(filename, "r"); + int ret = -1; + FILE *f; - ret = -1; + f = fopen(filename, "r"); if (f) { - struct config_source top; - - top.u.file = f; - top.name = top.path = filename; - top.die_on_error = 1; - top.do_fgetc = config_file_fgetc; - top.do_ungetc = config_file_ungetc; - top.do_ftell = config_file_ftell; - - ret = do_config_from(&top, fn, data); - + ret = do_config_from_file(fn, filename, filename, f, data); fclose(f); } return ret; @@ -1190,7 +1201,9 @@ int git_config_with_options(config_fn_t fn, void *data, * If we have a specific filename, use it. Otherwise, follow the * regular lookup sequence. */ - if (config_source && config_source->file) + if (config_source && config_source->use_stdin) + return git_config_from_stdin(fn, data); + else if (config_source && config_source->file) return git_config_from_file(fn, config_source->file, data); else if (config_source && config_source->blob) return git_config_from_blob_ref(fn, config_source->blob, data); diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index 967359344d..c9c426c273 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -475,15 +475,28 @@ ein.bahn=strasse EOF test_expect_success 'alternative GIT_CONFIG' ' - GIT_CONFIG=other-config git config -l >output && + GIT_CONFIG=other-config git config --list >output && test_cmp expect output ' test_expect_success 'alternative GIT_CONFIG (--file)' ' - git config --file other-config -l > output && + git config --file other-config --list >output && test_cmp expect output ' +test_expect_success 'alternative GIT_CONFIG (--file=-)' ' + git config --file - --list output && + test_cmp expect output +' + +test_expect_success 'setting a value in stdin is an error' ' + test_must_fail git config --file - some.value foo +' + +test_expect_success 'editing stdin is an error' ' + test_must_fail git config --file - --edit +' + test_expect_success 'refer config from subdirectory' ' mkdir x && ( diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh index 6edd38b39a..9ba2ba11c3 100755 --- a/t/t1305-config-include.sh +++ b/t/t1305-config-include.sh @@ -113,7 +113,7 @@ test_expect_success 'missing include files are ignored' ' test_expect_success 'absolute includes from command line work' ' echo "[test]one = 1" >one && echo 1 >expect && - git -c include.path="$PWD/one" config test.one >actual && + git -c include.path="$(pwd)/one" config test.one >actual && test_cmp expect actual ' @@ -138,6 +138,20 @@ test_expect_success 'relative includes from blobs fail' ' test_must_fail git config --blob=$blob test.one ' +test_expect_success 'absolute includes from stdin work' ' + echo "[test]one = 1" >one && + echo 1 >expect && + echo "[include]path=\"$(pwd)/one\"" | + git config --file - test.one >actual && + test_cmp expect actual +' + +test_expect_success 'relative includes from stdin line fail' ' + echo "[test]one = 1" >one && + echo "[include]path=one" | + test_must_fail git config --file - test.one +' + test_expect_success 'include cycles are detected' ' cat >.gitconfig <<-\EOF && [test]value = gitconfig