On Thu, Dec 07, 2023 at 02:11:35AM -0500, Jeff King wrote:
> When parsing fsck.*, receive.fsck.*, or fetch.fsck.*, we don't check for
> an implicit bool. So any of:
>
> [fsck]
> badTree
> [receive "fsck"]
> badTree
> [fetch "fsck"]
> badTree
>
> will cause us to segfault. We can fix it with config_error_nonbool() in
> the usual way, but we have to make a few more changes to get good error
> messages. The problem is that all three spots do:
>
> if (skip_prefix(var, "fsck.", &var))
>
> to match and parse the actual message id. But that means that "var" now
> just says "badTree" instead of "receive.fsck.badTree", making the
> resulting message confusing. We can fix that by storing the parsed
> message id in its own separate variable.
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> builtin/receive-pack.c | 11 +++++++----
> fetch-pack.c | 12 ++++++++----
> fsck.c | 8 ++++++--
> 3 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 8c4f0cb90a..ccf9738bce 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -142,6 +142,7 @@ static enum deny_action parse_deny_action(const char *var, const char *value)
> static int receive_pack_config(const char *var, const char *value,
> const struct config_context *ctx, void *cb)
> {
> + const char *msg_id;
> int status = parse_hide_refs_config(var, value, "receive", &hidden_refs);
>
> if (status)
> @@ -178,12 +179,14 @@ static int receive_pack_config(const char *var, const char *value,
> return 0;
> }
>
> - if (skip_prefix(var, "receive.fsck.", &var)) {
> - if (is_valid_msg_type(var, value))
> + if (skip_prefix(var, "receive.fsck.", &msg_id)) {
> + if (!value)
> + return config_error_nonbool(var);
> + if (is_valid_msg_type(msg_id, value))
> strbuf_addf(&fsck_msg_types, "%c%s=%s",
> - fsck_msg_types.len ? ',' : '=', var, value);
> + fsck_msg_types.len ? ',' : '=', msg_id, value);
> else
> - warning("skipping unknown msg id '%s'", var);
> + warning("skipping unknown msg id '%s'", msg_id);
> return 0;
> }
Same remark here. We would only generate warnings for an actual boolean,
so I'd think we might consider doing the same for implicit booleans.
Partick
Attachment:
signature.asc
Description: PGP signature
