Re: [PATCH] http: Add Accept-Language header if possible
From: Eric Sunshine <hidden>
Date: 2016-06-15 23:01:50
On Tue, Jul 8, 2014 at 11:54 AM, Yi EungJun [off-list ref] wrote:
quoted hunk ↗ jump to hunk
From: Yi EungJun <redacted> 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
quoted hunk ↗ jump to hunk
+ } +} + /* 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);
quoted hunk ↗ jump to hunk
+ 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