http: extract type/subtype portion of content-type

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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/http.c b/http.c
index 94e1afdee7890825dfda0946b999f524265603fc..6bfd0934b3cc977436f5955992918d8991270dec 100644 (file)
--- a/http.c
+++ b/http.c
@@ -906,6 +906,35 @@ 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.
+ *
+ * Note that we will silently remove even invalid whitespace. For
+ * example, "text / plain" is specifically forbidden by RFC 2616,
+ * but "text/plain" is the only reasonable output, and this keeps
+ * our code simple.
+ *
+ * Example:
+ *   "TEXT/PLAIN; charset=utf-8" -> "text/plain"
+ *   "text / plain" -> "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 +986,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 52c2d96ce6183ca8b3498794304fa8dc98f8f42a..a5ab977306ab19ad0def98ea11a6c0826acf7bbc 100644 (file)
--- 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 786f2816bb66b6dc21c593f861365924d0d1ecf1..23cec97cc315c935e308d0b8794c01a06a1630db 100755 (executable)
--- 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 13defd38866f4fff09b1dc607b2b8f64eaf733db..b35b2610e3d9bca5ac23fc2a663444a919a888e0 100755 (executable)
--- 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