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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Dec 23, 2017, 00:29 UTC
Message-ID: <alpine.DEB.2.21.1.1712230125341.406@MININT-6BKU6QN.europe.corp.microsoft.com>
In-Reply-To: <9073fcc7-c751-5563-81c8-d4992eefd298@free.fr>

Hi Julien,

On Sat, 23 Dec 2017, Julien Dusser wrote:

> 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.

But that ch comes from a signed char *, so it actually *is* an issue: if you cast a signed char of value -1 to an int, it will still be -1. So we'll also need:

-- snipsnap --

diff --git a/http.c b/http.c
index 117ddae..ed8221f 100644
--- a/http.c
+++ b/http.c
@@ -1347,7 +1347,7 @@ void finish_all_active_slots(void)
 }
 
 /* Helpers for modifying and creating URLs */
-static inline int needs_quote(int ch)
+static inline int needs_quote(unsigned char ch)
 {
 	if (((ch >= 'A') && (ch <= 'Z'))
 			|| ((ch >= 'a') && (ch <= 'z'))
@@ -1363,11 +1363,11 @@ static char *quote_ref_url(const char *base, const char *ref)
 {
 	struct strbuf buf = STRBUF_INIT;
 	const char *cp;
-	int ch;
+	unsigned char ch;
 
 	end_url_with_slash(&buf, base);
 
-	for (cp = ref; (ch = *cp) != 0; cp++)
+	for (cp = ref; (ch = (unsigned char)*cp); cp++)
 		if (needs_quote(ch))
 			strbuf_addf(&buf, "%%%02x", ch);
 		else