On Tue, Jul 8, 2014 at 11:54 AM, Yi EungJun <semtlenori@xxxxxxxxx> wrote:
> From: Yi EungJun <eungjun.yi@xxxxxxxxxxxxx>
>
> Add an Accept-Language header which indicates the user's preferred
> languages defined by 'LANGUAGE' environment variable if the variable is
> not empty.
>
> Example:
> LANGUAGE= -> ""
> LANGUAGE=ko -> "Accept-Language: ko; q=1.000, *; q=0.001"
> LANGUAGE=ko:en -> "Accept-Language: ko; q=1.000, en; q=0.999, *; q=0.001"
>
> This gives git servers a chance to display remote error messages in
> the user's preferred language.
> ---
> http.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> t/t5550-http-fetch-dumb.sh | 10 ++++++++++
> 2 files changed, 53 insertions(+)
>
> diff --git a/http.c b/http.c
> index 3a28b21..c345616 100644
> --- a/http.c
> +++ b/http.c
> @@ -983,6 +983,47 @@ static void extract_content_type(struct strbuf *raw, struct strbuf *type,
> strbuf_addstr(charset, "ISO-8859-1");
> }
>
> +/*
> + * Add an Accept-Language header which indicates user's preferred languages
> + * defined by 'LANGUAGE' environment variable if the variable is not empty.
> + *
> + * Example:
> + * LANGUAGE= -> ""
> + * LANGUAGE=ko -> "Accept-Language: ko; q=1.000, *; q=0.001"
> + * LANGUAGE=ko:en -> "Accept-Language: ko; q=1.000, en; q=0.999, *; q=0.001"
> + */
> +static void add_accept_language(struct strbuf *buf)
> +{
> + const char *p1, *p2;
> + float q = 1.000;
> +
> + p1 = getenv("LANGUAGE");
> +
> + if (p1 != NULL && p1[0] != '\0') {
> + strbuf_reset(buf);
It seems wrong to clear 'buf' in a function named add_accept_language().
> + strbuf_addstr(buf, "Accept-Language: ");
> + for (p2 = p1; q > 0.001; p2++) {
> + if ((*p2 == ':' || *p2 == '\0') && p1 != p2) {
> + if (q < 1.0) {
> + strbuf_addstr(buf, ", ");
> + }
> + strbuf_add(buf, p1, p2 - p1);
> + strbuf_addf(buf, "; q=%.3f", q);
> + q -= 0.001;
> + p1 = p2 + 1;
> +
> + if (*p2 == '\0') {
> + break;
> + }
> + }
> + }
> + if (q < 1.0) {
> + strbuf_addstr(buf, ", ");
> + }
> + strbuf_addstr(buf, "*; q=0.001\r\n");
Manually adding "\r\n" is contraindicated. Headers passed to
curl_easy_setopt(c, CURLOPT_HTTPHEADER, headers) must not have "\r\n",
since curl will add terminators itself [1].
[1]: http://curl.haxx.se/libcurl/c/CURLOPT_HTTPHEADER.html
> + }
> +}
> +
> /* http_request() targets */
> #define HTTP_REQUEST_STRBUF 0
> #define HTTP_REQUEST_FILE 1
> @@ -1020,6 +1061,8 @@ static int http_request(const char *url,
> fwrite_buffer);
> }
>
> + add_accept_language(&buf);
This is inconsistent with how other headers are handled by this
function. The existing idiom is:
strbuf_add(&buf, ...); /* construct header */
headers = curl_slist_apend(headers, buf.buf);
strbuf_reset(&buf);
> +
> strbuf_addstr(&buf, "Pragma:");
> if (options && options->no_cache)
> strbuf_addstr(&buf, " no-cache");
> diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
> index ac71418..ea15158 100755
> --- a/t/t5550-http-fetch-dumb.sh
> +++ b/t/t5550-http-fetch-dumb.sh
> @@ -196,5 +196,15 @@ test_expect_success 'reencoding is robust to whitespace oddities' '
> grep "this is the error message" stderr
> '
>
> +test_expect_success 'git client sends Accept-Language' '
> + GIT_CURL_VERBOSE=1 LANGUAGE=ko:en git clone "$HTTPD_URL/accept/language" 2>actual
Broken &&-chain.
> + grep "^Accept-Language: ko; q=1.000, en; q=0.999, \*; q=0.001" actual
Do you want to \-escape the periods? (Or maybe use 'grep -F'?)
> +'
> +
> +test_expect_success 'git client does not send Accept-Language' '
> + GIT_CURL_VERBOSE=1 LANGUAGE= git clone "$HTTPD_URL/accept/language" 2>actual
Broken &&-chain.
> + test_must_fail grep "^Accept-Language:" actual
> +'
> +
> stop_httpd
> test_done
> --
> 2.0.1.473.gafdefd9.dirty
--
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