Message ID | pull.1251.v3.git.1655054421697.gitgitgadget@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | [v3] remote-curl: send Accept-Language header to server | expand |
"Li Linchao via GitGitGadget" <gitgitgadget@gmail.com> writes: > Range-diff vs v2: > > 1: a2dd9d4070e ! 1: 99a4e23ceb1 remote-curl: send Accept-Language header to server > @@ Commit message > remote-curl: send Accept-Language header to server > > Git server end's ability to accept Accept-Language header was introduced > - in f18604bbf2(http: add Accept-Language header if possible), but this is > - only used by very early phase of the transfer, that's HTTP GET request to > - discover references. For other phases, like POST request in the smart HTTP > - the server side don't know what language the client speaks. > + in f18604bbf2 (http: add Accept-Language header if possible, 2015-01-28), > + but this is only used by very early phase of the transfer, which is HTTP > + GET request to discover references. For other phases, like POST request > + in the smart HTTP, the server does not know what language the client > + speaks. OK. > -+ accept_language = http_get_accept_language_header(); > -+ if (accept_language) { > -+ strbuf_addstr(&buf, accept_language); > -+ rpc->hdr_accept_language = strbuf_detach(&buf, NULL); > -+ } > ++ rpc->hdr_accept_language = xstrdup_or_null(http_get_accept_language_header()); Nice. > +@@ remote-curl.c: static int rpc_service(struct rpc_state *rpc, struct discovery *heads, > + free(rpc->service_url); > + free(rpc->hdr_content_type); > + free(rpc->hdr_accept); > ++ free(rpc->hdr_accept_language); > + free(rpc->protocol_header); > + free(rpc->buf); > + strbuf_release(&buf); OK. > +@@ remote-curl.c: static int stateless_connect(const char *service_name) > + free(rpc.service_url); > + free(rpc.hdr_content_type); > + free(rpc.hdr_accept); > ++ free(rpc.hdr_accept_language); > + free(rpc.protocol_header); > + free(rpc.buf); > + strbuf_release(&buf); OK. Thanks. Will queue.
"Li Linchao via GitGitGadget" <gitgitgadget@gmail.com> writes: > @@ -932,6 +933,10 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece > headers = curl_slist_append(headers, needs_100_continue ? > "Expect: 100-continue" : "Expect:"); > > + /* Add Accept-Language header */ > + if (rpc->hdr_accept_language) > + headers = curl_slist_append(headers, rpc->hdr_accept_language); > + > /* Add the extra Git-Protocol header */ > if (rpc->protocol_header) > headers = curl_slist_append(headers, rpc->protocol_header); > @@ -1080,6 +1085,8 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads, > strbuf_addf(&buf, "%s%s", url.buf, svc); > rpc->service_url = strbuf_detach(&buf, NULL); > > + rpc->hdr_accept_language = xstrdup_or_null(http_get_accept_language_header()); > + > strbuf_addf(&buf, "Content-Type: application/x-%s-request", svc); > rpc->hdr_content_type = strbuf_detach(&buf, NULL); > > @@ -1118,6 +1125,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads, > free(rpc->service_url); > free(rpc->hdr_content_type); > free(rpc->hdr_accept); > + free(rpc->hdr_accept_language); > free(rpc->protocol_header); > free(rpc->buf); > strbuf_release(&buf); > @@ -1400,6 +1408,7 @@ static int stateless_connect(const char *service_name) > struct discovery *discover; > struct rpc_state rpc; > struct strbuf buf = STRBUF_INIT; > + const char *accept_language; > > /* > * Run the info/refs request and see if the server supports protocol > @@ -1418,6 +1427,9 @@ static int stateless_connect(const char *service_name) > printf("\n"); > fflush(stdout); > } > + accept_language = http_get_accept_language_header(); > + if (accept_language) > + rpc.hdr_accept_language = xstrfmt("%s", accept_language); Isn't rpc.hdr_accept_language left uninitialized garbage if accept_language is NULL? It is the same bug I pointed out earlier, whose fix may have to be different. Has this been tested? I got immediate segfault with this patch in 'seen'.
Junio C Hamano <gitster@pobox.com> writes: >> + accept_language = http_get_accept_language_header(); >> + if (accept_language) >> + rpc.hdr_accept_language = xstrfmt("%s", accept_language); > > Isn't rpc.hdr_accept_language left uninitialized garbage if > accept_language is NULL? It is the same bug I pointed out earlier, > whose fix may have to be different. > > Has this been tested? I got immediate segfault with this patch in > 'seen'. Having said all that, I wonder if we want to use something like this to make it hard to use an uninitialized data. The smart-http is quite outside of my area of expertise, and I do not know what Shawn was thinking when de1a2fdd (Smart push over HTTP: client side, 2009-10-30) was written (it could be that filling all members explicitly was the more prevalent stype back then?). I'd appreciate input from folks who regularly deal with smart-http on the approach. Thanks. remote-curl.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git i/remote-curl.c w/remote-curl.c index 251d4ee64f..ba6f76a0c1 100644 --- i/remote-curl.c +++ w/remote-curl.c @@ -608,6 +608,8 @@ struct rpc_state { unsigned flush_read_but_not_sent : 1; }; +#define RPC_STATE_INIT { 0, } + /* * Appends the result of reading from rpc->out to the string represented by * rpc->buf and rpc->len if there is enough space. Returns 1 if there was @@ -1161,7 +1163,7 @@ static int fetch_dumb(int nr_heads, struct ref **to_fetch) static int fetch_git(struct discovery *heads, int nr_heads, struct ref **to_fetch) { - struct rpc_state rpc; + struct rpc_state rpc = RPC_STATE_INIT; struct strbuf preamble = STRBUF_INIT; int i, err; struct strvec args = STRVEC_INIT; @@ -1307,7 +1309,7 @@ static int push_dav(int nr_spec, const char **specs) static int push_git(struct discovery *heads, int nr_spec, const char **specs) { - struct rpc_state rpc; + struct rpc_state rpc = RPC_STATE_INIT; int i, err; struct strvec args; struct string_list_item *cas_option; @@ -1406,7 +1408,7 @@ static void parse_push(struct strbuf *buf) static int stateless_connect(const char *service_name) { struct discovery *discover; - struct rpc_state rpc; + struct rpc_state rpc = RPC_STATE_INIT; struct strbuf buf = STRBUF_INIT; const char *accept_language;
Junio C Hamano <gitster@pobox.com> writes:
> +#define RPC_STATE_INIT { 0, }
Make it
#define RPC_STATE_INIT { 0 }
Curiously, the former form with trailing comma makes sparse unhappy,
while it seems that the latter is taken as a special form "idiom".
diff --git a/http.c b/http.c index 11c6f69facd..33301d8d5d5 100644 --- a/http.c +++ b/http.c @@ -1775,7 +1775,7 @@ static void write_accept_language(struct strbuf *buf) * LANGUAGE= LANG=en_US.UTF-8 -> "Accept-Language: en-US, *; q=0.1" * LANGUAGE= LANG=C -> "" */ -static const char *get_accept_language(void) +const char *http_get_accept_language_header(void) { if (!cached_accept_language) { struct strbuf buf = STRBUF_INIT; @@ -1829,7 +1829,7 @@ static int http_request(const char *url, fwrite_buffer); } - accept_language = get_accept_language(); + accept_language = http_get_accept_language_header(); if (accept_language) headers = curl_slist_append(headers, accept_language); diff --git a/http.h b/http.h index ba303cfb372..3c94c479100 100644 --- a/http.h +++ b/http.h @@ -178,6 +178,9 @@ int http_fetch_ref(const char *base, struct ref *ref); int http_get_info_packs(const char *base_url, struct packed_git **packs_head); +/* Helper for getting Accept-Language header */ +const char *http_get_accept_language_header(void); + struct http_pack_request { char *url; diff --git a/remote-curl.c b/remote-curl.c index 67f178b1120..251d4ee64f6 100644 --- a/remote-curl.c +++ b/remote-curl.c @@ -580,6 +580,7 @@ struct rpc_state { char *service_url; char *hdr_content_type; char *hdr_accept; + char *hdr_accept_language; char *protocol_header; char *buf; size_t alloc; @@ -932,6 +933,10 @@ static int post_rpc(struct rpc_state *rpc, int stateless_connect, int flush_rece headers = curl_slist_append(headers, needs_100_continue ? "Expect: 100-continue" : "Expect:"); + /* Add Accept-Language header */ + if (rpc->hdr_accept_language) + headers = curl_slist_append(headers, rpc->hdr_accept_language); + /* Add the extra Git-Protocol header */ if (rpc->protocol_header) headers = curl_slist_append(headers, rpc->protocol_header); @@ -1080,6 +1085,8 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads, strbuf_addf(&buf, "%s%s", url.buf, svc); rpc->service_url = strbuf_detach(&buf, NULL); + rpc->hdr_accept_language = xstrdup_or_null(http_get_accept_language_header()); + strbuf_addf(&buf, "Content-Type: application/x-%s-request", svc); rpc->hdr_content_type = strbuf_detach(&buf, NULL); @@ -1118,6 +1125,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads, free(rpc->service_url); free(rpc->hdr_content_type); free(rpc->hdr_accept); + free(rpc->hdr_accept_language); free(rpc->protocol_header); free(rpc->buf); strbuf_release(&buf); @@ -1400,6 +1408,7 @@ static int stateless_connect(const char *service_name) struct discovery *discover; struct rpc_state rpc; struct strbuf buf = STRBUF_INIT; + const char *accept_language; /* * Run the info/refs request and see if the server supports protocol @@ -1418,6 +1427,9 @@ static int stateless_connect(const char *service_name) printf("\n"); fflush(stdout); } + accept_language = http_get_accept_language_header(); + if (accept_language) + rpc.hdr_accept_language = xstrfmt("%s", accept_language); rpc.service_name = service_name; rpc.service_url = xstrfmt("%s%s", url.buf, rpc.service_name); @@ -1467,6 +1479,7 @@ static int stateless_connect(const char *service_name) free(rpc.service_url); free(rpc.hdr_content_type); free(rpc.hdr_accept); + free(rpc.hdr_accept_language); free(rpc.protocol_header); free(rpc.buf); strbuf_release(&buf); diff --git a/t/t5541-http-push-smart.sh b/t/t5541-http-push-smart.sh index 2f09ff4fac6..fbad2d5ff5e 100755 --- a/t/t5541-http-push-smart.sh +++ b/t/t5541-http-push-smart.sh @@ -80,6 +80,25 @@ test_expect_success 'push to remote repository (standard)' ' test $HEAD = $(git rev-parse --verify HEAD)) ' +test_expect_success 'push to remote repository (standard) with sending Accept-Language' ' + cat >exp <<-\EOF && + => Send header: Accept-Language: ko-KR, *;q=0.9 + => Send header: Accept-Language: ko-KR, *;q=0.9 + EOF + + cd "$ROOT_PATH"/test_repo_clone && + : >path_lang && + git add path_lang && + test_tick && + git commit -m path_lang && + HEAD=$(git rev-parse --verify HEAD) && + GIT_TRACE_CURL=true LANGUAGE="ko_KR.UTF-8" git push -v -v 2>err && + ! grep "Expect: 100-continue" err && + + grep "=> Send header: Accept-Language:" err >err.language && + test_cmp exp err.language +' + test_expect_success 'push already up-to-date' ' git push ' diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh index f0d9cd584d3..bc308519af5 100755 --- a/t/t5550-http-fetch-dumb.sh +++ b/t/t5550-http-fetch-dumb.sh @@ -369,7 +369,7 @@ ja;q=0.95, zh;q=0.94, sv;q=0.93, pt;q=0.92, nb;q=0.91, *;q=0.90" \ ko_KR.EUC-KR:en_US.UTF-8:fr_CA:de.UTF-8@euro:sr@latin:ja:zh:sv:pt:nb ' -test_expect_success 'git client does not send an empty Accept-Language' ' +test_expect_success 'git client send an empty Accept-Language' ' GIT_TRACE_CURL=true LANGUAGE= git ls-remote "$HTTPD_URL/dumb/repo.git" 2>stderr && ! grep "^=> Send header: Accept-Language:" stderr ' diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh index b9351a732f6..245532df881 100755 --- a/t/t5551-http-fetch-smart.sh +++ b/t/t5551-http-fetch-smart.sh @@ -31,6 +31,7 @@ test_expect_success 'clone http repository' ' > GET /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1 > Accept: */* > Accept-Encoding: ENCODINGS + > Accept-Language: ko-KR, *;q=0.9 > Pragma: no-cache < HTTP/1.1 200 OK < Pragma: no-cache @@ -40,13 +41,15 @@ test_expect_success 'clone http repository' ' > Accept-Encoding: ENCODINGS > Content-Type: application/x-git-upload-pack-request > Accept: application/x-git-upload-pack-result + > Accept-Language: ko-KR, *;q=0.9 > Content-Length: xxx < HTTP/1.1 200 OK < Pragma: no-cache < Cache-Control: no-cache, max-age=0, must-revalidate < Content-Type: application/x-git-upload-pack-result EOF - GIT_TRACE_CURL=true GIT_TEST_PROTOCOL_VERSION=0 \ + + GIT_TRACE_CURL=true GIT_TEST_PROTOCOL_VERSION=0 LANGUAGE="ko_KR.UTF-8" \ git clone --quiet $HTTPD_URL/smart/repo.git clone 2>err && test_cmp file clone/file && tr '\''\015'\'' Q <err | @@ -94,7 +97,10 @@ test_expect_success 'clone http repository' ' test_cmp exp actual.smudged && grep "Accept-Encoding:.*gzip" actual >actual.gzip && - test_line_count = 2 actual.gzip + test_line_count = 2 actual.gzip && + + grep "Accept-Language: ko-KR, *" actual >actual.language && + test_line_count = 2 actual.language fi '