"Glen Choo via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> diff --git a/upload-pack.c b/upload-pack.c
> index 3a851b36066..09f48317b02 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -1321,18 +1321,27 @@ static int upload_pack_config(const char *var, const char *value, void *cb_data)
> data->advertise_sid = git_config_bool(var, value);
> }
>
> - if (current_config_scope() != CONFIG_SCOPE_LOCAL &&
> - current_config_scope() != CONFIG_SCOPE_WORKTREE) {
> - if (!strcmp("uploadpack.packobjectshook", var))
> - return git_config_string(&data->pack_objects_hook, var, value);
> - }
> -
The lossage of this block is because this general git_config()
callback routine that is used to read from any scope is no longer
used to pick up the sensitive variable. Instead, we need to get it
with a different API, namely, git_protected_config().
It is probably is good that in the new code we are not encouraging
folks to write random comparisons on current_config_scope(), and
instead uniformly use a git_protected_config(). That may promote
consistency.
An obvious alternative to achieve the same consistency would be to
introduce a helper, and rewrite (instead of removing) the above part
like so:
if (in_protected_scope()) {
... parse sensitive variable ...
}
We would not need any other change to this file in this patch if we
go that route, I suspect.
> if (parse_object_filter_config(var, value, data) < 0)
> return -1;
>
> return parse_hide_refs_config(var, value, "uploadpack");
> }
>
> +static int upload_pack_protected_config(const char *var, const char *value, void *cb_data)
> +{
> + struct upload_pack_data *data = cb_data;
> +
> + if (!strcmp("uploadpack.packobjectshook", var))
> + return git_config_string(&data->pack_objects_hook, var, value);
> + return 0;
> +}
> +
> +static void get_upload_pack_config(struct upload_pack_data *data)
> +{
> + git_config(upload_pack_config, data);
> + git_protected_config(upload_pack_protected_config, data);
> +}
Where we used to just do git_config(upload_pack_config), we now need
to do a separate git_protected_config(). It feels a bit wasteful to
iterate over the same configset twice, but it is not like we are
doing the IO and text file parsing multiple times. This looks quite
straight-forward.
