[PATCH 1/3] http: handle proxy authentication failure (error 407)

Nelson Benitez Leon· May 11, 2012, 13:13 UTC · lore

Handle http 407 error code by asking for credentials and retrying request in case credentials were not present, or marking credentials as rejected if they were already provided.

Signed-off-by: Nelson Benitez Leon <nbenitezl@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 http.c |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

Show changes to http.c +24 −0

diff --git a/http.c b/http.c
index 5cb87f1..624d1a0 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;
 
@@ -241,6 +242,20 @@ static int has_cert_password(void)
 	return 1;
 }
 
+static void set_proxy_auth(CURL *result)
+{
+	if (proxy_auth.username && proxy_auth.password) {
+#if LIBCURL_VERSION_NUM >= 0x071301
+		curl_easy_setopt(result, CURLOPT_PROXYUSERNAME, proxy_auth.username);
+		curl_easy_setopt(result, CURLOPT_PROXYPASSWORD, proxy_auth.password);
+#else
+		struct strbuf userpwd = STRBUF_INIT;
+		strbuf_addf(&userpwd, "%s:%s", proxy_auth.username, proxy_auth.password);
+		curl_easy_setopt(result, CURLOPT_PROXYUSERPWD, strbuf_detach(&userpwd, NULL));
+#endif
+	}
+}
+
 static CURL *get_curl_handle(void)
 {
 	CURL *result = curl_easy_init();
@@ -804,6 +819,15 @@ static int http_request(const char *url, void *result, int target, int options)
 				init_curl_http_auth(slot->curl);
 				ret = HTTP_REAUTH;
 			}
+		} else if (results.http_code == 407) { /* Proxy authentication failure */
+			if (proxy_auth.username && proxy_auth.password) {
+				credential_reject(&proxy_auth);
+				ret = HTTP_NOAUTH;
+			} else {
+				credential_fill(&proxy_auth);
+				set_proxy_auth(slot->curl);
+				ret = HTTP_REAUTH;
+			}
 		} else {
 			if (!curl_errorstr[0])
 				strlcpy(curl_errorstr,

-- 
1.7.7.6

Jeff King· May 11, 2012, 17:15 UTC · re: Nelson Benitez Leon · lore

Re: [PATCH 1/3] http: handle proxy authentication failure (error 407)

On Fri, May 11, 2012 at 03:13:40PM +0200, Nelson Benitez Leon wrote:

Show 13 quoted lines

> @@ -804,6 +819,15 @@ static int http_request(const char *url, void *result, int target, int options)
>  				init_curl_http_auth(slot->curl);
>  				ret = HTTP_REAUTH;
>  			}
> +		} else if (results.http_code == 407) { /* Proxy authentication failure */
> +			if (proxy_auth.username && proxy_auth.password) {
> +				credential_reject(&proxy_auth);
> +				ret = HTTP_NOAUTH;
> +			} else {
> +				credential_fill(&proxy_auth);
> +				set_proxy_auth(slot->curl);
> +				ret = HTTP_REAUTH;
> +			}

This part will fill in the username/password based on the proxy URL. But we never set the proxy URL ahead of time, so there is no chance for credential helpers to act, and the prompts will be confusing (they will just say "Password" instead of "Password for ...", which will make it unclear that we want the proxy password, not the remote server's password).

So it's OK to drop the environment-parsing bits, but:

  1. When we _do_ get the proxy via config, should we still parse it? I
     could go either way. It's a nice feature, and I think we don't have
     to care about how the environment parsing or NO_PROXY works. On the
     other hand, we could just wait for the callback-based
     authentication that will come in newer versions of curl, and code
     to that, which will be even simpler. In the meantime, people can
     just accept it.

  2. When we don't know the proxy name beforehand, we should probably
     say something to stderr to indicate that it was a proxy
     authentication failure.

Also, what about the dumb http-push code-paths? They would need us to handle http_proactive_auth in the same way. Which obviously won't work for environment-based proxies, but could work for config-based proxies. I'm not sure if it's worth caring about. In the long run, the callback-based authentication is the way forward (though of course it will take time for that feature to get released in curl, and then for people to start having a curl that uses it, and so on).

If we just punt on (1) and the proactive auth thing, then I think as a minimum we can get away with squashing this into your patch:

Show changes to http.c +1 −1

diff --git a/http.c b/http.c
index 0023119..86e68ee 100644
--- a/http.c
+++ b/http.c
@@ -824,6 +824,7 @@ static int http_request(const char *url, void *result, int target, int options)
 				credential_reject(&proxy_auth);
 				ret = HTTP_NOAUTH;
 			} else {
+				warning(_("http proxy did not accept our credentials, retrying"))
 				credential_fill(&proxy_auth);
 				set_proxy_auth(slot->curl);
 				ret = HTTP_AUTH_RETRY;

-Peff

patch, 3 partshttp: handle proxy authentication failure (error 407)