git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 4/8] http: extract type/subtype portion of content-type

From
Jeff King <peff@peff.net>
Date
May 22, 2014, 09:29 UTC
Message-ID
<20140522092947.GD15032@sigill.intra.peff.net>
In-Reply-To
<20140522092824.GA14530@sigill.intra.peff.net>

When we get a content-type from curl, we get the whole header line, including any parameters, and without any normalization (like downcasing or whitespace) applied. If we later try to match it with strcmp() or even strcasecmp(), we may get false negatives.

This could cause two visible behaviors:
  1. We might fail to recognize a smart-http server by its
     content-type.
  2. We might fail to relay text/plain error messages to
     users (especially if they contain a charset parameter).

This patch teaches the http code to extract and normalize just the type/subtype portion of the string. This is technically passing out less information to the callers, who can no longer see the parameters. But none of the current callers cares, and a future patch will add back an easier-to-use method for accessing those parameters.

Signed-off-by: Jeff King <peff@peff.net>
---
 http.c                     | 32 +++++++++++++++++++++++++++++---
 remote-curl.c              |  2 +-
 t/lib-httpd/error.sh       |  8 +++++++-
 t/t5550-http-fetch-dumb.sh |  5 +++++
 4 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/http.c b/http.c
index 94e1afd..4edf5b9 100644
--- a/http.c
+++ b/http.c
@@ -906,6 +906,29 @@ static CURLcode curlinfo_strbuf(CURL *curl, CURLINFO info, struct strbuf *buf)
 	return ret;
 }
 
+/*
+ * Extract a normalized version of the content type, with any
+ * spaces suppressed, all letters lowercased, and no trailing ";"
+ * or parameters.
+ *
+ * Example:
+ *   "TEXT/PLAIN; charset=utf-8" -> "text/plain"
+ */
+static void extract_content_type(struct strbuf *raw, struct strbuf *type)
+{
+	const char *p;
+
+	strbuf_reset(type);
+	strbuf_grow(type, raw->len);
+	for (p = raw->buf; *p; p++) {
+		if (isspace(*p))
+			continue;
+		if (*p == ';')
+			break;
+		strbuf_addch(type, tolower(*p));
+	}
+}
+
 /* http_request() targets */
 #define HTTP_REQUEST_STRBUF	0
 #define HTTP_REQUEST_FILE	1
@@ -957,9 +980,12 @@ static int http_request(const char *url,
 
 	ret = run_one_slot(slot, &results);
 
-	if (options && options->content_type)
-		curlinfo_strbuf(slot->curl, CURLINFO_CONTENT_TYPE,
-				options->content_type);
+	if (options && options->content_type) {
+		struct strbuf raw = STRBUF_INIT;
+		curlinfo_strbuf(slot->curl, CURLINFO_CONTENT_TYPE, &raw);
+		extract_content_type(&raw, options->content_type);
+		strbuf_release(&raw);
+	}
 
 	if (options && options->effective_url)
 		curlinfo_strbuf(slot->curl, CURLINFO_EFFECTIVE_URL,
diff --git a/remote-curl.c b/remote-curl.c
index 52c2d96..a5ab977 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -205,7 +205,7 @@ static int show_http_message(struct strbuf *type, struct strbuf *msg)
 	 * TODO should handle "; charset=XXX", and re-encode into
 	 * logoutputencoding
 	 */
-	if (strcasecmp(type->buf, "text/plain"))
+	if (strcmp(type->buf, "text/plain"))
 		return -1;
 
 	strbuf_trim(msg);
diff --git a/t/lib-httpd/error.sh b/t/lib-httpd/error.sh
index 786f281..23cec97 100755
--- a/t/lib-httpd/error.sh
+++ b/t/lib-httpd/error.sh
@@ -3,6 +3,7 @@
 printf "Status: 500 Intentional Breakage\n"
 
 printf "Content-Type: "
+charset=iso-8859-1
 case "$PATH_INFO" in
 *html*)
 	printf "text/html"
@@ -10,8 +11,13 @@ case "$PATH_INFO" in
 *text*)
 	printf "text/plain"
 	;;
+*charset*)
+	printf "text/plain; charset=utf-8"
+	charset=utf-8
+	;;
 esac
 printf "\n"
 
 printf "\n"
-printf "this is the error message\n"
+printf "this is the error message\n" |
+iconv -f us-ascii -t $charset
diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index 13defd3..b35b261 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -181,5 +181,10 @@ test_expect_success 'git client does not show html errors' '
 	! grep "this is the error message" stderr
 '
 
+test_expect_success 'git client shows text/plain with a charset' '
+	test_must_fail git clone "$HTTPD_URL/error/charset" 2>stderr &&
+	grep "this is the error message" stderr
+'
+
 stop_httpd
 test_done

-- 
2.0.0.rc1.436.g03cb729
Previous: Jeff KingNext: Kyle J. McKay
Message 34 of 42 in “handle alternate charsets for remote http errors”
  1. 0/9 handle alternate charsets for remote http errorsJeff King, May 21, 2014
  2. 1/9 test-lib: preserve GIT_CURL_VERBOSE from the environmentJeff King, May 21, 2014
  3. 2/9 strbuf: add strbuf_tolower functionJeff King, May 21, 2014
  4. Kyle J. McKayMay 22, 2014
  5. Jeff KingMay 22, 2014
  6. Junio C HamanoMay 22, 2014
  7. Jeff KingMay 22, 2014
  8. Junio C HamanoMay 22, 2014
  9. Jeff KingMay 23, 2014
  10. Kyle J. McKayMay 22, 2014
  11. Jeff KingMay 23, 2014
  12. Kyle J. McKayMay 23, 2014
  13. 3/9 daemon/config: factor out duplicate xstrdup_tolowerJeff King, May 21, 2014
  14. 4/9 http: normalize case of returned content-typeJeff King, May 21, 2014
  15. 5/9 t/lib-httpd: use write_script to copy CGI scriptsJeff King, May 21, 2014
  16. 6/9 t5550: test display of remote http error messagesJeff King, May 21, 2014
  17. 7/9 remote-curl: recognize text/plain with a charset parameterJeff King, May 21, 2014
  18. Kyle J. McKayMay 22, 2014
  19. Jeff KingMay 22, 2014
  20. Kyle J. McKayMay 22, 2014
  21. Jeff KingMay 22, 2014
  22. Peter KreftingMay 22, 2014
  23. Jeff KingMay 22, 2014
  24. Peter KreftingMay 22, 2014
  25. 8/9 strbuf: add strbuf_reencode helperJeff King, May 21, 2014
  26. 9/9 remote-curl: reencode http error messagesJeff King, May 21, 2014
  27. Kyle J. McKayMay 22, 2014
  28. Jeff KingMay 22, 2014
  29. Peter KreftingMay 22, 2014
  30. 0/9 handle alternate charsets for remote http errorsJeff King, May 22, 2014
  31. 1/8 test-lib: preserve GIT_CURL_VERBOSE from the environmentJeff King, May 22, 2014
  32. 2/8 t/lib-httpd: use write_script to copy CGI scriptsJeff King, May 22, 2014
  33. 3/8 t5550: test display of remote http error messagesJeff King, May 22, 2014
  34. 4/8 http: extract type/subtype portion of content-typeJeff King, May 22, 2014
  35. Kyle J. McKayMay 22, 2014
  36. Jeff KingMay 23, 2014
  37. Kyle J. McKayMay 23, 2014
  38. 5/8 http: optionally extract charset parameter from content-typeJeff King, May 22, 2014
  39. 6/8 strbuf: add strbuf_reencode helperJeff King, May 22, 2014
  40. 7/8 remote-curl: reencode http error messagesJeff King, May 22, 2014
  41. 8/8 http: default text charset to iso-8859-1Jeff King, May 22, 2014
  42. brian m. carlsonMay 23, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.