Re: [PATCH 7/9] remote-curl: recognize text/plain with a charset parameter

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On May 21, 2014, at 03:33, Jeff King wrote:

Commit 426e70d (remote-curl: show server content on http
errors, 2013-04-05) tried to recognize text/plain content
types, but fails to do so if they have any parameters.

This patches makes our parsing more liberal, though we still
do not do anything useful with a charset parameter.

Signed-off-by: Jeff King <peff@xxxxxxxx>
---
remote-curl.c              | 26 ++++++++++++++++++--------
t/lib-httpd/error.sh       |  8 +++++++-
t/t5550-http-fetch-dumb.sh |  5 +++++
3 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index a5ab977..6d1b206 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -194,20 +194,30 @@ static void free_discovery(struct discovery *d)
	}
}

+/*
+ * We only show text/plain parts, as other types are likely
+ * to be ugly to look at on the user's terminal.
+ */
+static int content_type_is_terminal_friendly(struct strbuf *type)
+{
+	const char *p;
+
+	p = skip_prefix(type->buf, "text/plain");
+	if (!p || (*p && *p != ';'))
+		return 0;
+
+	return 1;
+}
+

I think that a strict reading of RFC 2616 allows "text/plain ; charset=utf-8" as well as "text/plain;charset=utf-8" and "text/plain; charset=utf-8". So perhaps this if line instead:

+	if (!p || (*p && *p != ';' && *p != ' ' && *p != '\t'))

See RFC 2616 section 2.2 for the definition of linear white space (LWS) and the end of section 2.1 for the "implied *LWS" rule that allows it.

static int show_http_message(struct strbuf *type, 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 (strcmp(type->buf, "text/plain"))
+	if (!content_type_is_terminal_friendly(type))
		return -1;

+ /* TODO should record charset and reencode msg to logOutputEncoding */
+
	strbuf_trim(msg);
	if (!msg->len)
		return -1;
diff --git a/t/lib-httpd/error.sh b/t/lib-httpd/error.sh
index 786f281..02e80b3 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=iso8859-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


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]