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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: May 22, 2018, 01:00 UTC
Message-ID: <20180522010008.GI10623@aiede.svl.corp.google.com>
In-Reply-To: <CAGZ79kZiyi_1nxvfLttD6HPyV66Wz3pLnuAe=L7FB9ak05dGAQ@mail.gmail.com>

Hi,

Stefan Beller wrote:

> On Mon, May 21, 2018 at 4:40 PM, Brandon Williams <bmwill@google.com> wrote:

Show 6 quoted lines

>> Configure curl to accept all encoding which curl supports instead of
>> only accepting gzip responses.
>
> This partially reverts aa90b9697f9 (Enable info/refs gzip decompression
> in HTTP client, 2012-09-19), as that specifically called out deflate not being
> a good option. Is that worth mentioning in the commit message?

More specifically, it mentions the wasted 9 extra bytes from including "deflate, " on the Accept-Encoding line. I think the extra bandwidth usage will be okay. :)

[...]

Show 10 quoted lines

>> -       curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "gzip");
>> +       curl_easy_setopt(slot->curl, CURLOPT_ENCODING, "");
>
> Looking at the code here, this succeeds if enough memory is available.
> There is no check if the given parameter is part of
> Curl_all_content_encodings();
> https://github.com/curl/curl/blob/e66cca046cef20d00fba89260dfa6b4a3997233d/lib/setopt.c#L429
> https://github.com/curl/curl/blob/c675c40295045d4988eeb6291c54eb48f138822f/lib/content_encoding.c#L686
>
> which may be worth checking first?

By "this" are you referring to the preimage or the postimage? Are you suggesting a change in git or in libcurl?

Curl_all_content_encodings() is an internal function in libcurl, so I'm assuming the latter.

Thanks, Jonathan