Re: [PATCH] Fix urlencode format string on signed char.

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 22, 2017, 21:48 UTC
Message-ID: <xmqqk1xeph6r.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20171222172437.19505-1-julien.dusser@free.fr>

Julien Dusser <julien.dusser@free.fr> writes:

Show 27 quoted lines

> Git credential fails with special char in password.
> remote: Invalid username or password.
> fatal: Authentication failed for
>
> File ~/.git-credential contains badly urlencoded characters
> %ffffffXX%ffffffYY instead of %XX%YY.
>
> Add a cast to an unsigned char to fix urlencode use of %02x
> on a char.
>
> Signed-off-by: Julien Dusser <julien.dusser@free.fr>
> ---
>  strbuf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/strbuf.c b/strbuf.c
> index 323c49ceb..4d5a9ce55 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -658,7 +658,7 @@ static void strbuf_add_urlencode(struct strbuf *sb, const char *s, size_t len,
>  		    (!reserved && is_rfc3986_reserved(ch)))
>  			strbuf_addch(sb, ch);
>  		else
> -			strbuf_addf(sb, "%%%02x", ch);
> +			strbuf_addf(sb, "%%%02x", (unsigned char)ch);
>  	}
>  }

The issue is not limited to credential but anywhere where we need to show a byte with hi-bit set, and it is obvious and straight-forward.

I briefly wondered if the data type for the strings involved in the codepaths that reach this place should all be "uchar*" but it feels strange to have "unsigned char *username" etc., and the signeness matters only here, so the patch smells like the best one among other possibilities.

Thanks.