Re: [PATCH 7/9] remote-curl: recognize text/plain with a charset parameter
- From
Kyle J. McKay <mackyle@gmail.com>
- Date
- May 22, 2014, 00:07 UTC
- Message-ID
- <90CE006F-EE74-40D2-8847-507E37021D84@gmail.com>
- In-Reply-To
- <20140521103301.GG30464@sigill.intra.peff.net>
On May 21, 2014, at 03:33, Jeff King wrote:
Show 37 quoted lines
> 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@peff.net>
> ---
> 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.
Show 67 quoted lines
> 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
>