From: Junio C Hamano Date: Thu, 2 Aug 2018 22:30:39 +0000 (-0700) Subject: Merge branch 'jk/fsck-gitmodules-gently' X-Git-Tag: v2.19.0-rc0~111 X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/bd1a32d5c8ec5987824acb63694361d082ce9092?hp=37aac3e408fa2348983e964f8bda2de581f2c44e Merge branch 'jk/fsck-gitmodules-gently' Recent "security fix" to pay attention to contents of ".gitmodules" while accepting "git push" was a bit overly strict than necessary, which has been adjusted. * jk/fsck-gitmodules-gently: fsck: downgrade gitmodulesParse default to "info" fsck: split ".gitmodules too large" error from parse failure fsck: silence stderr when parsing .gitmodules config: add options parameter to git_config_from_mem config: add CONFIG_ERROR_SILENT handler config: turn die_on_error into caller-facing enum --- diff --git a/config.c b/config.c index 7968ef7566..35a9bc7955 100644 --- a/config.c +++ b/config.c @@ -32,7 +32,7 @@ struct config_source { enum config_origin_type origin_type; const char *name; const char *path; - int die_on_error; + enum config_error_action default_error_action; int linenr; int eof; struct strbuf value; @@ -810,10 +810,21 @@ static int git_parse_source(config_fn_t fn, void *data, cf->linenr, cf->name); } - if (cf->die_on_error) + switch (opts && opts->error_action ? + opts->error_action : + cf->default_error_action) { + case CONFIG_ERROR_DIE: die("%s", error_msg); - else + break; + case CONFIG_ERROR_ERROR: error_return = error("%s", error_msg); + break; + case CONFIG_ERROR_SILENT: + error_return = -1; + break; + case CONFIG_ERROR_UNSET: + BUG("config error action unset"); + } free(error_msg); return error_return; @@ -1521,7 +1532,7 @@ static int do_config_from_file(config_fn_t fn, top.origin_type = origin_type; top.name = name; top.path = path; - top.die_on_error = 1; + top.default_error_action = CONFIG_ERROR_DIE; top.do_fgetc = config_file_fgetc; top.do_ungetc = config_file_ungetc; top.do_ftell = config_file_ftell; @@ -1559,8 +1570,10 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data) return git_config_from_file_with_options(fn, filename, data, NULL); } -int git_config_from_mem(config_fn_t fn, const enum config_origin_type origin_type, - const char *name, const char *buf, size_t len, void *data) +int git_config_from_mem(config_fn_t fn, + const enum config_origin_type origin_type, + const char *name, const char *buf, size_t len, + void *data, const struct config_options *opts) { struct config_source top; @@ -1570,12 +1583,12 @@ int git_config_from_mem(config_fn_t fn, const enum config_origin_type origin_typ top.origin_type = origin_type; top.name = name; top.path = NULL; - top.die_on_error = 0; + top.default_error_action = CONFIG_ERROR_ERROR; top.do_fgetc = config_buf_fgetc; top.do_ungetc = config_buf_ungetc; top.do_ftell = config_buf_ftell; - return do_config_from(&top, fn, data, NULL); + return do_config_from(&top, fn, data, opts); } int git_config_from_blob_oid(config_fn_t fn, @@ -1596,7 +1609,8 @@ int git_config_from_blob_oid(config_fn_t fn, return error("reference '%s' does not point to a blob", name); } - ret = git_config_from_mem(fn, CONFIG_ORIGIN_BLOB, name, buf, size, data); + ret = git_config_from_mem(fn, CONFIG_ORIGIN_BLOB, name, buf, size, + data, NULL); free(buf); return ret; diff --git a/config.h b/config.h index b95bb7649d..bb2f506b27 100644 --- a/config.h +++ b/config.h @@ -54,6 +54,12 @@ struct config_options { const char *git_dir; config_parser_event_fn_t event_fn; void *event_fn_data; + enum config_error_action { + CONFIG_ERROR_UNSET = 0, /* use source-specific default */ + CONFIG_ERROR_DIE, /* die() on error */ + CONFIG_ERROR_ERROR, /* error() on error, return -1 */ + CONFIG_ERROR_SILENT, /* return -1 */ + } error_action; }; typedef int (*config_fn_t)(const char *, const char *, void *); @@ -62,8 +68,11 @@ extern int git_config_from_file(config_fn_t fn, const char *, void *); extern int git_config_from_file_with_options(config_fn_t fn, const char *, void *, const struct config_options *); -extern int git_config_from_mem(config_fn_t fn, const enum config_origin_type, - const char *name, const char *buf, size_t len, void *data); +extern int git_config_from_mem(config_fn_t fn, + const enum config_origin_type, + const char *name, + const char *buf, size_t len, + void *data, const struct config_options *opts); extern int git_config_from_blob_oid(config_fn_t fn, const char *name, const struct object_id *oid, void *data); extern void git_config_push_parameter(const char *text); diff --git a/fsck.c b/fsck.c index ae4b1f3c09..cabca15b1f 100644 --- a/fsck.c +++ b/fsck.c @@ -63,7 +63,7 @@ static struct oidset gitmodules_done = OIDSET_INIT; FUNC(ZERO_PADDED_DATE, ERROR) \ FUNC(GITMODULES_MISSING, ERROR) \ FUNC(GITMODULES_BLOB, ERROR) \ - FUNC(GITMODULES_PARSE, ERROR) \ + FUNC(GITMODULES_LARGE, ERROR) \ FUNC(GITMODULES_NAME, ERROR) \ FUNC(GITMODULES_SYMLINK, ERROR) \ /* warnings */ \ @@ -77,6 +77,7 @@ static struct oidset gitmodules_done = OIDSET_INIT; FUNC(ZERO_PADDED_FILEMODE, WARN) \ FUNC(NUL_IN_COMMIT, WARN) \ /* infos (reported as warnings, but ignored by default) */ \ + FUNC(GITMODULES_PARSE, INFO) \ FUNC(BAD_TAG_NAME, INFO) \ FUNC(MISSING_TAGGER_ENTRY, INFO) @@ -999,6 +1000,7 @@ static int fsck_blob(struct blob *blob, const char *buf, unsigned long size, struct fsck_options *options) { struct fsck_gitmodules_data data; + struct config_options config_opts = { 0 }; if (!oidset_contains(&gitmodules_found, &blob->object.oid)) return 0; @@ -1014,15 +1016,16 @@ static int fsck_blob(struct blob *blob, const char *buf, * that an error. */ return report(options, &blob->object, - FSCK_MSG_GITMODULES_PARSE, + FSCK_MSG_GITMODULES_LARGE, ".gitmodules too large to parse"); } data.obj = &blob->object; data.options = options; data.ret = 0; + config_opts.error_action = CONFIG_ERROR_SILENT; if (git_config_from_mem(fsck_gitmodules_fn, CONFIG_ORIGIN_BLOB, - ".gitmodules", buf, size, &data)) + ".gitmodules", buf, size, &data, &config_opts)) data.ret |= report(options, &blob->object, FSCK_MSG_GITMODULES_PARSE, "could not parse gitmodules blob"); diff --git a/submodule-config.c b/submodule-config.c index 2a7259ba8b..fc2c41b947 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -562,7 +562,7 @@ static const struct submodule *config_from(struct submodule_cache *cache, parameter.gitmodules_oid = &oid; parameter.overwrite = 0; git_config_from_mem(parse_config, CONFIG_ORIGIN_SUBMODULE_BLOB, rev.buf, - config, config_size, ¶meter); + config, config_size, ¶meter, NULL); strbuf_release(&rev); free(config); diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh index b68c5f5e85..293e2e1963 100755 --- a/t/t7415-submodule-names.sh +++ b/t/t7415-submodule-names.sh @@ -176,4 +176,19 @@ test_expect_success 'fsck detects non-blob .gitmodules' ' ) ' +test_expect_success 'fsck detects corrupt .gitmodules' ' + git init corrupt && + ( + cd corrupt && + + echo "[broken" >.gitmodules && + git add .gitmodules && + git commit -m "broken gitmodules" && + + git fsck 2>output && + grep gitmodulesParse output && + test_i18ngrep ! "bad config" output + ) +' + test_done