Re: [RFC/PATCH] remote-curl: Obey passed URL

From: Jeff King <peff@peff.net>
Date: Oct 6, 2011, 13:25 UTC
Message-ID: <20111006132500.GA1792@sigill.intra.peff.net>
In-Reply-To: <2f1eccfa3fa9e732e9bea344fd69dfd9b16697a9.1317906388.git.git@drmicha.warpmail.net>

On Thu, Oct 06, 2011 at 03:15:59PM +0200, Michael J Gruber wrote:

Show 18 quoted lines

> When the curl remote helper is called, e.g., as
> 
> git-remote-https foo https://john@doe.com
> 
> it looks up remote.foo.url rather than taking the provided url, at least
> as far as http_init() is concerned (authentication). (It does use the
> provided url at later stages.)
> [...]
> --- a/remote-curl.c
> +++ b/remote-curl.c
> @@ -846,6 +846,7 @@ int main(int argc, const char **argv)
>  
>  	if (argc > 2) {
>  		end_url_with_slash(&buf, argv[2]);
> +		remote->url[0] = xstrdup(argv[2]);
>  	} else {
>  		end_url_with_slash(&buf, remote->url[0]);
>  	}

Your analysis is correct, but tweaking the remote object seems kind of ugly. I think a nicer solution would be to pass the URL in separately to http_init. Of the three existing callers:

  1. http-fetch.c just passes a NULL remote. Which means we don't even
     look at the URL at all for grabbing the auth information. Passing
     the URL would be an improvement.

  2. http-push.c creates a fake remote just to stick the URL in. That
     ugly code could just go away.

  3. remote-curl.c needs to pass in the alternate URL anyway, as you
     described.

So it seems like all callsites would benefit.

That being said, I wonder if http_init is the right place to pull the auth information out. It's where we've always done it, and it makes sense if you are hitting one base URL. But what happens if we get a redirect to some other site? Shouldn't we be deciding at the time of making the request what the context of the http request is?

And then http_init can stop caring entirely what the URL is.

-Peff