Merge branch 'jk/fsck-gitmodules-gently'
authorJunio C Hamano <gitster@pobox.com>
Thu, 2 Aug 2018 22:30:39 +0000 (15:30 -0700)
committerJunio C Hamano <gitster@pobox.com>
Thu, 2 Aug 2018 22:30:39 +0000 (15:30 -0700)
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

1  2 
config.c
config.h
fsck.c
submodule-config.c
diff --combined config.c
index 7968ef7566a1fc28c5d8e2ba99ae87c52fe23ece,60132e3774c281a9ccaa483122d614d3fe4e1460..35a9bc79555ade317e965b031a7a8a402bddf43f
+++ b/config.c
@@@ -14,7 -14,6 +14,7 @@@
  #include "quote.h"
  #include "hashmap.h"
  #include "string-list.h"
 +#include "object-store.h"
  #include "utf8.h"
  #include "dir.h"
  #include "color.h"
@@@ -32,7 -31,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 -809,21 +810,21 @@@ static int git_parse_source(config_fn_
                                      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 -1531,7 +1532,7 @@@ static int do_config_from_file(config_f
        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 -1569,10 +1570,10 @@@ int git_config_from_file(config_fn_t fn
        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;
  
        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,
                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;
@@@ -2173,6 -2186,23 +2187,6 @@@ int git_config_get_pathname(const char 
        return repo_config_get_pathname(the_repository, key, dest);
  }
  
 -/*
 - * Note: This function exists solely to maintain backward compatibility with
 - * 'fetch' and 'update_clone' storing configuration in '.gitmodules' and should
 - * NOT be used anywhere else.
 - *
 - * Runs the provided config function on the '.gitmodules' file found in the
 - * working directory.
 - */
 -void config_from_gitmodules(config_fn_t fn, void *data)
 -{
 -      if (the_repository->worktree) {
 -              char *file = repo_worktree_path(the_repository, GITMODULES_FILE);
 -              git_config_from_file(fn, file, data);
 -              free(file);
 -      }
 -}
 -
  int git_config_get_expiry(const char *key, const char **output)
  {
        int ret = git_config_get_string_const(key, output);
diff --combined config.h
index b95bb7649db993cb2802b21e44280f17cb160591,f2063ceb86f12d6bb6e56e62967a3d85195ca7fc..bb2f506b27137e8d66ab4cd96439143dced94780
+++ b/config.h
@@@ -54,6 -54,12 +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 +68,11 @@@ extern int git_config_from_file(config_
  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);
@@@ -215,6 -224,16 +224,6 @@@ extern int repo_config_get_maybe_bool(s
  extern int repo_config_get_pathname(struct repository *repo,
                                    const char *key, const char **dest);
  
 -/*
 - * Note: This function exists solely to maintain backward compatibility with
 - * 'fetch' and 'update_clone' storing configuration in '.gitmodules' and should
 - * NOT be used anywhere else.
 - *
 - * Runs the provided config function on the '.gitmodules' file found in the
 - * working directory.
 - */
 -extern void config_from_gitmodules(config_fn_t fn, void *data);
 -
  extern int git_config_get_value(const char *key, const char **value);
  extern const struct string_list *git_config_get_value_multi(const char *key);
  extern void git_config_clear(void);
diff --combined fsck.c
index ae4b1f3c09710eff4cfeb4be6e9b9d0488a2d249,69ea8d5321b032caf8bb3bf13d0be1ed7a5f62ce..cabca15b1f0ed5bdb8fe1a5d5439516264a8f9c8
--- 1/fsck.c
--- 2/fsck.c
+++ b/fsck.c
@@@ -1,5 -1,4 +1,5 @@@
  #include "cache.h"
 +#include "object-store.h"
  #include "object.h"
  #include "blob.h"
  #include "tree.h"
@@@ -63,7 -62,7 +63,7 @@@ static struct oidset gitmodules_done = 
        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 -76,7 +77,7 @@@
        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)
  
@@@ -316,13 -316,6 +317,13 @@@ static void append_msg_id(struct strbu
        strbuf_addstr(sb, ": ");
  }
  
 +static int object_on_skiplist(struct fsck_options *opts, struct object *obj)
 +{
 +      if (opts && opts->skiplist && obj)
 +              return oid_array_lookup(opts->skiplist, &obj->oid) >= 0;
 +      return 0;
 +}
 +
  __attribute__((format (printf, 4, 5)))
  static int report(struct fsck_options *options, struct object *object,
        enum fsck_msg_id id, const char *fmt, ...)
        if (msg_type == FSCK_IGNORE)
                return 0;
  
 -      if (options->skiplist && object &&
 -                      oid_array_lookup(options->skiplist, &object->oid) >= 0)
 +      if (object_on_skiplist(options, object))
                return 0;
  
        if (msg_type == FSCK_FATAL)
@@@ -802,7 -796,7 +803,7 @@@ static int fsck_commit_buffer(struct co
                buffer = p + 1;
                parent_line_count++;
        }
 -      graft = lookup_commit_graft(&commit->object.oid);
 +      graft = lookup_commit_graft(the_repository, &commit->object.oid);
        parent_count = commit_list_count(commit->parents);
        if (graft) {
                if (graft->nr_parent == -1 && !parent_count)
@@@ -999,14 -993,12 +1000,15 @@@ static int fsck_blob(struct blob *blob
                     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;
        oidset_insert(&gitmodules_done, &blob->object.oid);
  
 +      if (object_on_skiplist(options, &blob->object))
 +              return 0;
 +
        if (!buf) {
                /*
                 * A missing buffer here is a sign that the caller found the
                 * 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 --combined submodule-config.c
index 2a7259ba8b5297091c1f1917952cdf560e88a646,2ca3272dd1cfbc34ff1071c89aa8088ba01ebdc5..fc2c41b947cb471deef42323c83f8b28f42780d6
@@@ -4,7 -4,6 +4,7 @@@
  #include "submodule-config.h"
  #include "submodule.h"
  #include "strbuf.h"
 +#include "object-store.h"
  #include "parse-options.h"
  
  /*
@@@ -562,7 -561,7 +562,7 @@@ static const struct submodule *config_f
        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);
  
@@@ -592,23 -591,6 +592,23 @@@ static void submodule_cache_check_init(
        submodule_cache_init(repo->submodule_cache);
  }
  
 +/*
 + * Note: This function is private for a reason, the '.gitmodules' file should
 + * not be used as as a mechanism to retrieve arbitrary configuration stored in
 + * the repository.
 + *
 + * Runs the provided config function on the '.gitmodules' file found in the
 + * working directory.
 + */
 +static void config_from_gitmodules(config_fn_t fn, struct repository *repo, void *data)
 +{
 +      if (repo->worktree) {
 +              char *file = repo_worktree_path(repo, GITMODULES_FILE);
 +              git_config_from_file(fn, file, data);
 +              free(file);
 +      }
 +}
 +
  static int gitmodules_cb(const char *var, const char *value, void *data)
  {
        struct repository *repo = data;
@@@ -626,11 -608,19 +626,11 @@@ void repo_read_gitmodules(struct reposi
  {
        submodule_cache_check_init(repo);
  
 -      if (repo->worktree) {
 -              char *gitmodules;
 -
 -              if (repo_read_index(repo) < 0)
 -                      return;
 -
 -              gitmodules = repo_worktree_path(repo, GITMODULES_FILE);
 -
 -              if (!is_gitmodules_unmerged(repo->index))
 -                      git_config_from_file(gitmodules_cb, gitmodules, repo);
 +      if (repo_read_index(repo) < 0)
 +              return;
  
 -              free(gitmodules);
 -      }
 +      if (!is_gitmodules_unmerged(repo->index))
 +              config_from_gitmodules(gitmodules_cb, repo, repo);
  
        repo->submodule_cache->gitmodules_read = 1;
  }
@@@ -681,45 -671,3 +681,45 @@@ void submodule_free(struct repository *
        if (r->submodule_cache)
                submodule_cache_clear(r->submodule_cache);
  }
 +
 +struct fetch_config {
 +      int *max_children;
 +      int *recurse_submodules;
 +};
 +
 +static int gitmodules_fetch_config(const char *var, const char *value, void *cb)
 +{
 +      struct fetch_config *config = cb;
 +      if (!strcmp(var, "submodule.fetchjobs")) {
 +              *(config->max_children) = parse_submodule_fetchjobs(var, value);
 +              return 0;
 +      } else if (!strcmp(var, "fetch.recursesubmodules")) {
 +              *(config->recurse_submodules) = parse_fetch_recurse_submodules_arg(var, value);
 +              return 0;
 +      }
 +
 +      return 0;
 +}
 +
 +void fetch_config_from_gitmodules(int *max_children, int *recurse_submodules)
 +{
 +      struct fetch_config config = {
 +              .max_children = max_children,
 +              .recurse_submodules = recurse_submodules
 +      };
 +      config_from_gitmodules(gitmodules_fetch_config, the_repository, &config);
 +}
 +
 +static int gitmodules_update_clone_config(const char *var, const char *value,
 +                                        void *cb)
 +{
 +      int *max_jobs = cb;
 +      if (!strcmp(var, "submodule.fetchjobs"))
 +              *max_jobs = parse_submodule_fetchjobs(var, value);
 +      return 0;
 +}
 +
 +void update_clone_config_from_gitmodules(int *max_jobs)
 +{
 +      config_from_gitmodules(gitmodules_update_clone_config, the_repository, &max_jobs);
 +}