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

From: Junio C Hamano <[email-masked]>
Date: Fri, 26 Jul 2024 10:45:13 -0700

Ryan Hendrickson <[email-masked]> 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.

>> 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.