Re: [PATCH] http: fix charset detection of extract_content_type()

[Jeff King <peff@peff.net>]

On Sun, Jun 15, 2014 at 03:49:34AM +0900, nori wrote:

> extract_content_type() could not extract a charset parameter if the
> parameter is not the first one and there is a whitespace and a following
> semicolon just before the parameter. For example:
> 
>     text/plain; format=fixed ;charset=utf-8

Thanks, I think your patch does the right thing. We also have a similar
situation going the other way. If we have:

   text/plain; charset=utf-8; format=fixed

we will parse the charset as "utf-8;". My version of iconv actually
seems to accept that, but we should probably be more careful.

I think parameter values can actually be fully quoted. So you could
have something as nasty as:

  text/plain; some-param="a long value with ;semicolons;"; charset=utf-8

I'd rather not get into parsing that level, as I don't expect it to
happen in the real world. And besides, we actually _would_ find the
charset here with the current code; the only thing we might do wrong is
to parse:

  text/plain; tricky="param; charset=foo"; charset=bar

with charset "foo" rather than "bar". But that's highly unlikely, and
the stakes are fairly low (we just show the error message in the wrong
charset).

Anyway, when you re-roll with the correct "From:" header, do you mind
squashing in the extra change and the tests below?

diff --git a/http.c b/http.c
index 05e8b91..3a28b21 100644
--- a/http.c
+++ b/http.c
@@ -927,7 +927,7 @@ static int extract_param(const char *raw, const char *name,
 		return -1;
 	raw++;
 
-	while (*raw && !isspace(*raw))
+	while (*raw && !isspace(*raw) && *raw != ';')
 		strbuf_addch(out, *raw++);
 	return 0;
 }
diff --git a/t/lib-httpd/error.sh b/t/lib-httpd/error.sh
index eafc9d2..a77b8e5 100755
--- a/t/lib-httpd/error.sh
+++ b/t/lib-httpd/error.sh
@@ -19,6 +19,10 @@ case "$PATH_INFO" in
 	printf "text/plain; charset=utf-16"
 	charset=utf-16
 	;;
+*odd-spacing*)
+	printf "text/plain; foo=bar ;charset=utf-16; other=nonsense"
+	charset=utf-16
+	;;
 esac
 printf "\n"
 
diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index 01b8aae..ac71418 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -191,5 +191,10 @@ test_expect_success 'http error messages are reencoded' '
 	grep "this is the error message" stderr
 '
 
+test_expect_success 'reencoding is robust to whitespace oddities' '
+	test_must_fail git clone "$HTTPD_URL/error/odd-spacing" 2>stderr &&
+	grep "this is the error message" stderr
+'
+
 stop_httpd
 test_done