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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> When the curl remote helper is called, e.g., as
> 
> git-remote-https foo https://john@xxxxxxx
> 
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]