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 7968ef7566a1fc28c5d8e2ba99ae87c52fe23ece..35a9bc79555ade317e965b031a7a8a402bddf43f 100644 (file)
--- 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 b95bb7649db993cb2802b21e44280f17cb160591..bb2f506b27137e8d66ab4cd96439143dced94780 100644 (file)
--- 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 ae4b1f3c09710eff4cfeb4be6e9b9d0488a2d249..cabca15b1f0ed5bdb8fe1a5d5439516264a8f9c8 100644 (file)
--- 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 2a7259ba8b5297091c1f1917952cdf560e88a646..fc2c41b947cb471deef42323c83f8b28f42780d6 100644 (file)
--- 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, &parameter);
+                       config, config_size, &parameter, NULL);
        strbuf_release(&rev);
        free(config);
 

diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh
index b68c5f5e8510ddbf4e10f6cb608d9e789bef09b4..293e2e1963962e49afa10882439b5106607439a1 100755 (executable)
--- 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