Re: [PATCH 06/21] builtin/config: check for writeability after source is set up

>
> The `check_write()` function verifies that we do not try to write to a
> config source that cannot be written to, like for example stdin. But
> while the new subcommands do call this function, they do so before
> calling `handle_config_location()`. Consequently, we only end up
> checking the default config location for writeability, not the location
> that was actually specified by the caller of git-config(1).
>
> Fix this by calling `check_write()` after `handle_config_location()`. We
> will further clarify the relationship between those two functions in a
> subsequent commit where we remove the global state that both implicitly
> rely on.

>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/config.c  | 10 +++++-----
>  t/t1300-config.sh |  6 ++++++
>  2 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/config.c b/builtin/config.c
> index 8f3342b593..9295a2f737 100644
> --- a/builtin/config.c
> +++ b/builtin/config.c
> @@ -843,7 +843,6 @@ static int cmd_config_set(int argc, const char **argv, const char *prefix)
>
>         argc = parse_options(argc, argv, prefix, opts, builtin_config_set_usage,
>                              PARSE_OPT_STOP_AT_NON_OPTION);
> -       check_write();
>         check_argc(argc, 2, 2);
>
>         if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
> @@ -856,6 +855,7 @@ static int cmd_config_set(int argc, const char **argv, const char *prefix)
>         comment = git_config_prepare_comment_string(comment_arg);
>
>         handle_config_location(prefix);
> +       check_write();
>
>         value = normalize_value(argv[0], argv[1], &default_kvi);
>
> @@ -891,13 +891,13 @@ static int cmd_config_unset(int argc, const char **argv, const char *prefix)
>
>         argc = parse_options(argc, argv, prefix, opts, builtin_config_unset_usage,
>                              PARSE_OPT_STOP_AT_NON_OPTION);
> -       check_write();
>         check_argc(argc, 1, 1);
>
>         if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
>                 die(_("--fixed-value only applies with 'value-pattern'"));
>
>         handle_config_location(prefix);
> +       check_write();
>
>         if ((flags & CONFIG_FLAGS_MULTI_REPLACE) || value_pattern)
>                 return git_config_set_multivar_in_file_gently(given_config_source.file,
> @@ -918,10 +918,10 @@ static int cmd_config_rename_section(int argc, const char **argv, const char *pr
>
>         argc = parse_options(argc, argv, prefix, opts, builtin_config_rename_section_usage,
>                              PARSE_OPT_STOP_AT_NON_OPTION);
> -       check_write();
>         check_argc(argc, 2, 2);
>
>         handle_config_location(prefix);
> +       check_write();
>
>         ret = git_config_rename_section_in_file(given_config_source.file,
>                                                 argv[0], argv[1]);
> @@ -943,10 +943,10 @@ static int cmd_config_remove_section(int argc, const char **argv, const char *pr
>
>         argc = parse_options(argc, argv, prefix, opts, builtin_config_remove_section_usage,
>                              PARSE_OPT_STOP_AT_NON_OPTION);
> -       check_write();
>         check_argc(argc, 1, 1);
>
>         handle_config_location(prefix);
> +       check_write();
>
>         ret = git_config_rename_section_in_file(given_config_source.file,
>                                                 argv[0], NULL);
> @@ -997,10 +997,10 @@ static int cmd_config_edit(int argc, const char **argv, const char *prefix)
>         };
>
>         argc = parse_options(argc, argv, prefix, opts, builtin_config_edit_usage, 0);
> -       check_write();
>         check_argc(argc, 0, 0);
>
>         handle_config_location(prefix);
> +       check_write();
>
>         return show_editor();
>  }
> diff --git a/t/t1300-config.sh b/t/t1300-config.sh
> index d90a69b29f..9de2d95f06 100755
> --- a/t/t1300-config.sh
> +++ b/t/t1300-config.sh
> @@ -2835,6 +2835,12 @@ test_expect_success 'specifying multiple modes causes failure' '
>         test_cmp expect err
>  '
>
> +test_expect_success 'writing to stdin is rejected' '
> +       echo "fatal: writing to stdin is not supported" >expect &&
> +       test_must_fail git config ${mode_set} --file - foo.bar baz 2>err &&
> +       test_cmp expect err
> +'
> +
>  done
>
>  test_done
> --
> 2.45.GIT
>