Re: [PATCH] http: Add Accept-Language header if possible
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Jul 8, 2014, 21:52 UTC
- Message-ID
- <CAPig+cTbhUKF7OwYJY2_1m9j=khPRaLM2kd5KqDGD=vjRz0qHg@mail.gmail.com>
- In-Reply-To
- <1404834846-11812-1-git-send-email-eungjun.yi@navercorp.com>
On Tue, Jul 8, 2014 at 11:54 AM, Yi EungJun <semtlenori@gmail.com> wrote:
Show 44 quoted lines
> From: Yi EungJun <eungjun.yi@navercorp.com>
>
> 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().
Show 20 quoted lines
> + 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
Show 11 quoted lines
> + } > +} > + > /* 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);Show 14 quoted lines
> + > 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.
Show 7 quoted lines
> + test_must_fail grep "^Accept-Language:" actual > +' > + > stop_httpd > test_done > -- > 2.0.1.473.gafdefd9.dirty