Re: [PATCH v5 2/5] http: handle proxy proactive authentication

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 12, 2012, 15:54 UTC
Message-ID: <7vwr5leyj5.fsf@alter.siamese.dyndns.org>
In-Reply-To: <7vsjgcs8pq.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

Show 10 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>
>> I haven't formed an opinion on what the proper solution should be, but
>> either the credential_from_url() function needs to be updated to accept
>> the scp style [user@]<host>:<port> argument, or this specific caller
>> should take the responsibility to do special case the syntax.
>
> Well, calling the above "scp" style is a mistake (it is not scp style at
> all), but the patch to teach the credentail_from_url() to handle the proxy
> specification may look like this:

Jeff, do you have an opinion on this? I briefly wondered if we also want to teach the traditional [user@]host:/path/to/repo to this function (it is not a URL in RFC1738 sense, but it is in the remote.$name.url sense), but because SSH does its own thing interacting with agents, perhaps it may not help to teach our credential layer to store and supply cached passphrases (or passwords, if the authentication is done by merely sending password over the encrypted channel).

A safer approach might be to keep externally visible API to this function as before, but add another function only for the use of http_proxy and friends (whose kosher format is "host:address" without the "<scheme>://" part), and call it from the codepath broken by the patch.

Show 33 quoted lines

>  credential.c |   10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/credential.c b/credential.c
> index 62d1c56..482ae88 100644
> --- a/credential.c
> +++ b/credential.c
> @@ -324,11 +324,13 @@ void credential_from_url(struct credential *c, const char *url)
>  	 *   (1) proto://<host>/...
>  	 *   (2) proto://<user>@<host>/...
>  	 *   (3) proto://<user>:<pass>@<host>/...
> +	 * or "proto://"-less variants of the above for *_proxy variables.
>  	 */
>  	proto_end = strstr(url, "://");
> -	if (!proto_end)
> -		return;
> -	cp = proto_end + 3;
> +	if (proto_end)
> +		cp = proto_end + 3;
> +	else
> +		cp = url;
>  	at = strchr(cp, '@');
>  	colon = strchr(cp, ':');
>  	slash = strchrnul(cp, '/');
> @@ -348,7 +350,7 @@ void credential_from_url(struct credential *c, const char *url)
>  		host = at + 1;
>  	}
>  
> -	if (proto_end - url > 0)
> +	if (proto_end && proto_end != url)
>  		c->protocol = xmemdupz(url, proto_end - url);
>  	if (slash - host > 0)
>  		c->host = url_decode_mem(host, slash - host);