summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJunio C Hamano <gitster@pobox.com>2014-06-16 19:18:35 (GMT)
committerJunio C Hamano <gitster@pobox.com>2014-06-16 19:18:36 (GMT)
commit2075a0c27fa5cf4f9f03964d407dc015c1749a7e (patch)
tree361c26f8579dcd888a9f4e5b54f79534a9c3b608
parentc37d3269d92eedbdf405a6f196253d120e8e2721 (diff)
parentc553fd1c1ef76688b6200e66a825b530b0b02140 (diff)
downloadgit-2075a0c27fa5cf4f9f03964d407dc015c1749a7e.zip
git-2075a0c27fa5cf4f9f03964d407dc015c1749a7e.tar.gz
git-2075a0c27fa5cf4f9f03964d407dc015c1749a7e.tar.bz2
Merge branch 'jk/http-errors'
Propagate the error messages from the webserver better to the client coming over the HTTP transport. * jk/http-errors: http: default text charset to iso-8859-1 remote-curl: reencode http error messages strbuf: add strbuf_reencode helper http: optionally extract charset parameter from content-type http: extract type/subtype portion of content-type t5550: test display of remote http error messages t/lib-httpd: use write_script to copy CGI scripts test-lib: preserve GIT_CURL_VERBOSE from the environment
-rw-r--r--Documentation/technical/api-strbuf.txt5
-rw-r--r--http.c87
-rw-r--r--http.h7
-rw-r--r--remote-curl.c19
-rw-r--r--strbuf.c17
-rw-r--r--strbuf.h1
-rw-r--r--t/lib-httpd.sh7
-rw-r--r--t/lib-httpd/apache.conf4
-rw-r--r--[-rwxr-xr-x]t/lib-httpd/broken-smart-http.sh1
-rwxr-xr-xt/lib-httpd/error.sh27
-rwxr-xr-xt/t5550-http-fetch-dumb.sh20
-rw-r--r--t/test-lib.sh1
12 files changed, 183 insertions, 13 deletions
diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt
index 5069018..077a709 100644
--- a/Documentation/technical/api-strbuf.txt
+++ b/Documentation/technical/api-strbuf.txt
@@ -134,6 +134,11 @@ Functions
Strip whitespace from the beginning of a string.
+`strbuf_reencode`::
+
+ Replace the contents of the strbuf with a reencoded form. Returns -1
+ on error, 0 on success.
+
`strbuf_tolower`::
Lowercase each character in the buffer using `tolower`.
diff --git a/http.c b/http.c
index 94e1afd..2b4f6a3 100644
--- a/http.c
+++ b/http.c
@@ -906,6 +906,83 @@ static CURLcode curlinfo_strbuf(CURL *curl, CURLINFO info, struct strbuf *buf)
return ret;
}
+/*
+ * Check for and extract a content-type parameter. "raw"
+ * should be positioned at the start of the potential
+ * parameter, with any whitespace already removed.
+ *
+ * "name" is the name of the parameter. The value is appended
+ * to "out".
+ */
+static int extract_param(const char *raw, const char *name,
+ struct strbuf *out)
+{
+ size_t len = strlen(name);
+
+ if (strncasecmp(raw, name, len))
+ return -1;
+ raw += len;
+
+ if (*raw != '=')
+ return -1;
+ raw++;
+
+ while (*raw && !isspace(*raw))
+ strbuf_addch(out, *raw++);
+ return 0;
+}
+
+/*
+ * 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.
+ *
+ * If the "charset" argument is not NULL, store the value of any
+ * charset parameter there.
+ *
+ * Example:
+ * "TEXT/PLAIN; charset=utf-8" -> "text/plain", "utf-8"
+ * "text / plain" -> "text/plain"
+ */
+static void extract_content_type(struct strbuf *raw, struct strbuf *type,
+ struct strbuf *charset)
+{
+ const char *p;
+
+ strbuf_reset(type);
+ strbuf_grow(type, raw->len);
+ for (p = raw->buf; *p; p++) {
+ if (isspace(*p))
+ continue;
+ if (*p == ';') {
+ p++;
+ break;
+ }
+ strbuf_addch(type, tolower(*p));
+ }
+
+ if (!charset)
+ return;
+
+ strbuf_reset(charset);
+ while (*p) {
+ while (isspace(*p))
+ p++;
+ if (!extract_param(p, "charset", charset))
+ return;
+ while (*p && !isspace(*p))
+ p++;
+ }
+
+ if (!charset->len && starts_with(type->buf, "text/"))
+ strbuf_addstr(charset, "ISO-8859-1");
+}
+
/* http_request() targets */
#define HTTP_REQUEST_STRBUF 0
#define HTTP_REQUEST_FILE 1
@@ -957,9 +1034,13 @@ 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,
+ options->charset);
+ strbuf_release(&raw);
+ }
if (options && options->effective_url)
curlinfo_strbuf(slot->curl, CURLINFO_EFFECTIVE_URL,
diff --git a/http.h b/http.h
index e64084f..473179b 100644
--- a/http.h
+++ b/http.h
@@ -144,6 +144,13 @@ struct http_get_options {
struct strbuf *content_type;
/*
+ * If non-NULL, and content_type above is non-NULL, returns
+ * the charset parameter from the content-type. If none is
+ * present, returns an empty string.
+ */
+ struct strbuf *charset;
+
+ /*
* If non-NULL, returns the URL we ended up at, including any
* redirects we followed.
*/
diff --git a/remote-curl.c b/remote-curl.c
index 52c2d96..4493b38 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -194,19 +194,19 @@ static void free_discovery(struct discovery *d)
}
}
-static int show_http_message(struct strbuf *type, struct strbuf *msg)
+static int show_http_message(struct strbuf *type, struct strbuf *charset,
+ struct strbuf *msg)
{
const char *p, *eol;
/*
* We only show text/plain parts, as other types are likely
* to be ugly to look at on the user's terminal.
- *
- * TODO should handle "; charset=XXX", and re-encode into
- * logoutputencoding
*/
- if (strcasecmp(type->buf, "text/plain"))
+ if (strcmp(type->buf, "text/plain"))
return -1;
+ if (charset->len)
+ strbuf_reencode(msg, charset->buf, get_log_output_encoding());
strbuf_trim(msg);
if (!msg->len)
@@ -225,6 +225,7 @@ static struct discovery* discover_refs(const char *service, int for_push)
{
struct strbuf exp = STRBUF_INIT;
struct strbuf type = STRBUF_INIT;
+ struct strbuf charset = STRBUF_INIT;
struct strbuf buffer = STRBUF_INIT;
struct strbuf refs_url = STRBUF_INIT;
struct strbuf effective_url = STRBUF_INIT;
@@ -249,6 +250,7 @@ static struct discovery* discover_refs(const char *service, int for_push)
memset(&options, 0, sizeof(options));
options.content_type = &type;
+ options.charset = &charset;
options.effective_url = &effective_url;
options.base_url = &url;
options.no_cache = 1;
@@ -259,13 +261,13 @@ static struct discovery* discover_refs(const char *service, int for_push)
case HTTP_OK:
break;
case HTTP_MISSING_TARGET:
- show_http_message(&type, &buffer);
+ show_http_message(&type, &charset, &buffer);
die("repository '%s' not found", url.buf);
case HTTP_NOAUTH:
- show_http_message(&type, &buffer);
+ show_http_message(&type, &charset, &buffer);
die("Authentication failed for '%s'", url.buf);
default:
- show_http_message(&type, &buffer);
+ show_http_message(&type, &charset, &buffer);
die("unable to access '%s': %s", url.buf, curl_errorstr);
}
@@ -310,6 +312,7 @@ static struct discovery* discover_refs(const char *service, int for_push)
strbuf_release(&refs_url);
strbuf_release(&exp);
strbuf_release(&type);
+ strbuf_release(&charset);
strbuf_release(&effective_url);
strbuf_release(&buffer);
last_discovery = last;
diff --git a/strbuf.c b/strbuf.c
index c821775..ac62982 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -1,5 +1,6 @@
#include "cache.h"
#include "refs.h"
+#include "utf8.h"
int starts_with(const char *str, const char *prefix)
{
@@ -99,6 +100,22 @@ void strbuf_ltrim(struct strbuf *sb)
sb->buf[sb->len] = '\0';
}
+int strbuf_reencode(struct strbuf *sb, const char *from, const char *to)
+{
+ char *out;
+ int len;
+
+ if (same_encoding(from, to))
+ return 0;
+
+ out = reencode_string_len(sb->buf, sb->len, to, from, &len);
+ if (!out)
+ return -1;
+
+ strbuf_attach(sb, out, len, len);
+ return 0;
+}
+
void strbuf_tolower(struct strbuf *sb)
{
char *p = sb->buf, *end = sb->buf + sb->len;
diff --git a/strbuf.h b/strbuf.h
index 25328b9..e9ad03e 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -45,6 +45,7 @@ static inline void strbuf_setlen(struct strbuf *sb, size_t len)
extern void strbuf_trim(struct strbuf *);
extern void strbuf_rtrim(struct strbuf *);
extern void strbuf_ltrim(struct strbuf *);
+extern int strbuf_reencode(struct strbuf *sb, const char *from, const char *to);
extern void strbuf_tolower(struct strbuf *sb);
extern int strbuf_cmp(const struct strbuf *, const struct strbuf *);
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 8b67021..272fcee 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -110,10 +110,15 @@ else
"Could not identify web server at '$LIB_HTTPD_PATH'"
fi
+install_script () {
+ write_script "$HTTPD_ROOT_PATH/$1" <"$TEST_PATH/$1"
+}
+
prepare_httpd() {
mkdir -p "$HTTPD_DOCUMENT_ROOT_PATH"
cp "$TEST_PATH"/passwd "$HTTPD_ROOT_PATH"
- cp "$TEST_PATH"/broken-smart-http.sh "$HTTPD_ROOT_PATH"
+ install_script broken-smart-http.sh
+ install_script error.sh
ln -s "$LIB_HTTPD_MODULE_PATH" "$HTTPD_ROOT_PATH/modules"
diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
index 3a03e82..b384d79 100644
--- a/t/lib-httpd/apache.conf
+++ b/t/lib-httpd/apache.conf
@@ -97,12 +97,16 @@ Alias /auth/dumb/ www/auth/dumb/
</LocationMatch>
ScriptAliasMatch /smart_*[^/]*/(.*) ${GIT_EXEC_PATH}/git-http-backend/$1
ScriptAlias /broken_smart/ broken-smart-http.sh/
+ScriptAlias /error/ error.sh/
<Directory ${GIT_EXEC_PATH}>
Options FollowSymlinks
</Directory>
<Files broken-smart-http.sh>
Options ExecCGI
</Files>
+<Files error.sh>
+ Options ExecCGI
+</Files>
<Files ${GIT_EXEC_PATH}/git-http-backend>
Options ExecCGI
</Files>
diff --git a/t/lib-httpd/broken-smart-http.sh b/t/lib-httpd/broken-smart-http.sh
index f7ebfff..82cc610 100755..100644
--- a/t/lib-httpd/broken-smart-http.sh
+++ b/t/lib-httpd/broken-smart-http.sh
@@ -1,4 +1,3 @@
-#!/bin/sh
printf "Content-Type: text/%s\n" "html"
echo
printf "%s\n" "001e# service=git-upload-pack"
diff --git a/t/lib-httpd/error.sh b/t/lib-httpd/error.sh
new file mode 100755
index 0000000..eafc9d2
--- /dev/null
+++ b/t/lib-httpd/error.sh
@@ -0,0 +1,27 @@
+#!/bin/sh
+
+printf "Status: 500 Intentional Breakage\n"
+
+printf "Content-Type: "
+charset=iso-8859-1
+case "$PATH_INFO" in
+*html*)
+ printf "text/html"
+ ;;
+*text*)
+ printf "text/plain"
+ ;;
+*charset*)
+ printf "text/plain; charset=utf-8"
+ charset=utf-8
+ ;;
+*utf16*)
+ printf "text/plain; charset=utf-16"
+ charset=utf-16
+ ;;
+esac
+printf "\n"
+
+printf "\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 1a3a2b6..01b8aae 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -171,5 +171,25 @@ test_expect_success 'did not use upload-pack service' '
test_cmp exp act
'
+test_expect_success 'git client shows text/plain errors' '
+ test_must_fail git clone "$HTTPD_URL/error/text" 2>stderr &&
+ grep "this is the error message" stderr
+'
+
+test_expect_success 'git client does not show html errors' '
+ test_must_fail git clone "$HTTPD_URL/error/html" 2>stderr &&
+ ! 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
+'
+
+test_expect_success 'http error messages are reencoded' '
+ test_must_fail git clone "$HTTPD_URL/error/utf16" 2>stderr &&
+ grep "this is the error message" stderr
+'
+
stop_httpd
test_done
diff --git a/t/test-lib.sh b/t/test-lib.sh
index c081668..f7e55b1 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -91,6 +91,7 @@ unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
VALGRIND
UNZIP
PERF_
+ CURL_VERBOSE
));
my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
print join("\n", @vars);