Re: [PATCH v2] http: honnor empty http.proxy option to bypass proxy

> On Tue, Apr 11, 2017 at 12:20:50PM +0300, Sergey Ryazanov wrote:
>> diff --git a/http.c b/http.c
>> index 96d84bb..8be75b2 100644
>> --- a/http.c
>> +++ b/http.c
>> @@ -836,8 +836,14 @@ static CURL *get_curl_handle(void)
>>               }
>>       }
>>
>> -     if (curl_http_proxy) {
>> -             curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
>> +     if (curl_http_proxy && curl_http_proxy[0] == '\0') {
>> +             /*
>> +              * Handle case with the empty http.proxy value here to keep
>> +              * common code clean.
>> +              * NB: empty option disables proxying at all.
>> +              */
>> +             curl_easy_setopt(result, CURLOPT_PROXY, "");
>> +     } else if (curl_http_proxy) {
>
> This seems pretty reasonable. You could bump this under the existing "if
> (curl_http_proxy)" condition, but that would add an extra level of
> indentation to the rest of the parsing.
>

> One thing I do wonder, though: can credential_from_url() ever return a
> NULL host field for other inputs, and what should we do on those inputs?
>
> For example, if I set the value to just "https://" with no hostname,
> then I think we'll end up with a NULL proxy_auth.host field. And we'll
> feed that NULL to CURLOPT_PROXY, which will cause it to use the
> defaults.
>
> I don't know _what_ "https://" should do. It's clearly bogus. But
> telling curl to use the defaults seems funny. In that sense, your
> original was much better (we'd feed it to curl, which would be free to
> complain).
>