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