Re: [PATCH] http: do not ignore proxy path

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 26, 2024, 17:45 UTC
Message-ID: <xmqq5xssvyhi.fsf@gitster.g>
In-Reply-To: <6b408a07-f2ff-1e04-5b3a-fd287fb1c151@alum.mit.edu>

Ryan Hendrickson <ryan.hendrickson@alum.mit.edu> writes:

> For what it's worth, I was following the precedent set by the
> if-statement starting at line 1256 (a few lines above this patch):

It is worth nothing. Existing violation does not make it a good idea to add more of them. It makes it more work to clean them all up later.

Show 8 quoted lines

>> Could existing users have been taking advantage of the fact that the
>> extra /path at the end of http.proxy (and $http_proxy and friends)
>> are ignored?  For them, this change will appear as a regression.
>
> That is possible, though I have difficulty imagining a scenario in
> which it would be intentional.
>
> What do you recommend I do about that possibility?

I have no idea. I already said that I am not familiar with this code path, and it is likely I am a worse person than you are to find a potential creative (ab)uses of the mechanism to take advantage of the fact that the current code ignores the path part.

Having said that, given that http.proxy falls back to environment variables that have been established a lot longer than Git itself, like HTTPS_PROXY etc., if we _know_ that setting HTTPS_PROXY to such a value with /path part causes curl (or other popular programs) to try using such a value without stripping the path part (and even better if that causes them to fail), then such an observation would make a very good argument why it is extremely unlikely that existing users used such a setting, hence it is unlikely this patch would make a regression.

Thanks.