Re: [PATCH] http: handle curl stripping creds from effective url

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> I've used curl's curl_url() interface to do the stripping here, mostly
> because its behavior should match the stripping it does internally. And
> also, though we have code to parse a URL, we don't have any to
> reconstruct it, making a single string comparison hard.
>
> One alternative would be to parse with url_parse() or similar, and
> compare the individual fields (skipping username/password). I think that
> would probably also work in practice, but it seemed to me that the
> simplest change would be sticking with string comparisons.

Very nice.

> The curl_url() interface appeared in 7.62.0. We document that 7.61.0 is
> still supported, so I've made it conditional here. Only new versions
> strip the result from CURLINFO_EFFECTIVE_URL, so it's OK for very old
> versions to skip the extra comparison.

;-)

> We could probably declare 7.62.0 the oldest supported version of curl,
> but it would really only save a few lines of #ifdef here. I'd prefer to
> consider that question separately.

That is very sensible.

> +#ifndef GIT_CURL_HAVE_CURL_URL
> +#define strip_url_credential(in) NULL
> +#else
> +static char *strip_url_credential(const char *in)
> +{
> +	char *ret = NULL;
> +	CURLU *url;
> +
> +	url = curl_url();
> +	if (!url)
> +		goto out;
> +
> +	if (curl_url_set(url, CURLUPART_URL, in, 0))
> +		goto out;
> +
> +	curl_url_set(url, CURLUPART_USER, NULL, 0);
> +	curl_url_set(url, CURLUPART_PASSWORD, NULL, 0);
> +	curl_url_get(url, CURLUPART_URL, &ret, 0);
> +
> +out:
> +	curl_url_cleanup(url);
> +	return ret;
> +}
> +#endif