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

From: Julien Dusser <julien.dusser@free.fr>
Date: Dec 22, 2017, 23:49 UTC
Message-ID: <9073fcc7-c751-5563-81c8-d4992eefd298@free.fr>
In-Reply-To: <xmqqk1xeph6r.fsf@gitster.mtv.corp.google.com>

Thank you for review. I didn't find any other error. Code in http.c:quote_ref_url() is almost the same but ch is a signed int, so there's no issue.

Le 22/12/2017 à 22:48, Junio C Hamano a écrit :

Show 41 quoted lines

> Julien Dusser <julien.dusser@free.fr> writes:
> 
>> 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.
>