Re: [PATCH v2 1/2] remote-curl: accept all encodings supported by curl

[Jonathan Nieder <jrnieder@gmail.com>]

Brandon Williams wrote:

> 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.

> 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.

> --- 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' '