Re: [PATCH v2 3/3] http: when proxy url has username but no password, ask for password

[Sam Vilain <sam@vilain.net>]

On 3/1/12 10:22 AM, Nelson Benitez Leon wrote:
> Support proxy urls with username but without a password, in which
> case we interactively ask for the password (using credential api).
> This makes possible to not have the password written down in
> http_proxy env var or in http.proxy config option.
>
> Signed-off-by: Nelson Benitez Leon<nbenitezl@gmail.com>
> ---
>   http.c |   16 +++++++++++++++-
>   1 files changed, 15 insertions(+), 1 deletions(-)
>
> diff --git a/http.c b/http.c
> index 8932da5..5916194 100644
> --- a/http.c
> +++ b/http.c
> @@ -43,6 +43,7 @@ static int curl_ftp_no_epsv;
>   static const char *curl_http_proxy;
>   static const char *curl_cookie_file;
>   static struct credential http_auth = CREDENTIAL_INIT;
> +static struct credential proxy_auth = CREDENTIAL_INIT;
>   static int http_proactive_auth;
>   static const char *user_agent;
>
> @@ -303,7 +304,20 @@ static CURL *get_curl_handle(void)
>   		}
>   	}
>   	if (curl_http_proxy) {
> -		curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
> +		credential_from_url(&proxy_auth, curl_http_proxy);
> +		if (proxy_auth.username != NULL&&  proxy_auth.password == NULL) {
> +			/* proxy string has username but no password, ask for password */
> +			struct strbuf pbuf = STRBUF_INIT;
> +			credential_fill(&proxy_auth);

Wouldn't it be better to wait until the proxy returns a 403 before 
assuming that the proxy setting is incorrect/missing a password?  What 
if the administrator expects the user to fill in both the username and 
password?  That is the behaviour of a web browser.

Also, I think you should wait until that 403 to detect whether the proxy 
setting came from the environment, and only load it explicitly then.

Sam

> +			strbuf_addf(&pbuf, "%s://%s:%s@%s",proxy_auth.protocol,
> +				    proxy_auth.username, proxy_auth.password,
> +				    proxy_auth.host);
> +			free ((void *)curl_http_proxy);
> +			curl_http_proxy =  strbuf_detach(&pbuf, NULL);
> +			curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
> +		} else {
> +			curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
> +		}
>   		curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
>   	}
>