Re: [PATCH v2 1/2] remote-curl: accept all encodings supported by curl
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- May 22, 2018, 18:48 UTC
- Message-ID
- <20180522184821.GL10623@aiede.svl.corp.google.com>
- In-Reply-To
- <20180522184204.47332-1-bmwill@google.com>
Brandon Williams wrote:
Show 15 quoted lines
> Configure curl to accept all encodings which curl supports instead of > only accepting gzip responses. > > This fixes an issue when using an installation of curl which is built > without the "zlib" feature. Since aa90b9697 (Enable info/refs gzip > decompression in HTTP client, 2012-09-19) we end up requesting "gzip" > encoding anyway despite libcurl not being able to decode it. Worse, > instead of getting a clear error message indicating so, we end up > falling back to "dumb" http, producing a confusing and difficult to > debug result. > > Since curl doesn't do any checking to verify that it supports the a > requested encoding, instead set the curl option `CURLOPT_ENCODING` with > an empty string indicating that curl should send an "Accept-Encoding" > header containing only the encodings supported by curl.
Even better, this means we get the benefit of future of even better compression algorithms once libcurl learns them.
Show 10 quoted lines
> Reported-by: Anton Golubev <anton.golubev@gmail.com> > Signed-off-by: Brandon Williams <bmwill@google.com> > --- > Version 2 of this series just tweaks the commit message and the test per > Jonathan's suggestion. > > http.c | 2 +- > remote-curl.c | 2 +- > t/t5551-http-fetch-smart.sh | 13 +++++++++---- > 3 files changed, 11 insertions(+), 6 deletions(-)
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks for fixing it.
Patch left unsnipped for reference.
Show 61 quoted lines
> --- a/http.c
> +++ b/http.c
> @@ -1788,7 +1788,7 @@ static int http_request(const char *url,
>
> curl_easy_setopt(slot->curl, CURLOPT_URL, url);
> curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers);
> - curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "gzip");
> + curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "");
>
> ret = run_one_slot(slot, &results);
>
> diff --git a/remote-curl.c b/remote-curl.c
> index ceb05347b..565bba104 100644
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -684,7 +684,7 @@ static int post_rpc(struct rpc_state *rpc)
> curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);
> curl_easy_setopt(slot->curl, CURLOPT_POST, 1);
> curl_easy_setopt(slot->curl, CURLOPT_URL, rpc->service_url);
> - curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "gzip");
> + curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "");
>
> if (large_request) {
> /* The request body is large and the size cannot be predicted.
> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
> index f5721b4a5..913089b14 100755
> --- a/t/t5551-http-fetch-smart.sh
> +++ b/t/t5551-http-fetch-smart.sh
> @@ -26,14 +26,14 @@ setup_askpass_helper
> cat >exp <<EOF
> > GET /smart/repo.git/info/refs?service=git-upload-pack HTTP/1.1
> > Accept: */*
> -> Accept-Encoding: gzip
> +> Accept-Encoding: ENCODINGS
> > Pragma: no-cache
> < HTTP/1.1 200 OK
> < Pragma: no-cache
> < Cache-Control: no-cache, max-age=0, must-revalidate
> < Content-Type: application/x-git-upload-pack-advertisement
> > POST /smart/repo.git/git-upload-pack HTTP/1.1
> -> Accept-Encoding: gzip
> +> Accept-Encoding: ENCODINGS
> > Content-Type: application/x-git-upload-pack-request
> > Accept: application/x-git-upload-pack-result
> > Content-Length: xxx
> @@ -79,8 +79,13 @@ test_expect_success 'clone http repository' '
> /^< Date: /d
> /^< Content-Length: /d
> /^< Transfer-Encoding: /d
> - " >act &&
> - test_cmp exp act
> + " >actual &&
> + sed -e "s/^> Accept-Encoding: .*/> Accept-Encoding: ENCODINGS/" \
> + actual >actual.smudged &&
> + test_cmp exp actual.smudged &&
> +
> + grep "Accept-Encoding:.*gzip" actual >actual.gzip &&
> + test_line_count = 2 actual.gzip
> '
>
> test_expect_success 'fetch changes via http' '