# [PATCH] http: support CURLPROXY_HTTPS 6 messages from 2017-12-19 to 2017-12-20. Participants: Wei Shuyu, Jonathan Nieder, Junio C Hamano, Jeff King. Thread: https://gitlist.dev/t/47466 ## Wei Shuyu, 2017-12-19 17:24 Subject: [PATCH] http: support CURLPROXY_HTTPS Message-ID: <20171219172401.5263-1-wsy@dogben.com> URL: https://gitlist.dev/e/20171219172401.5263-1-wsy%40dogben.com ``` HTTP proxy over SSL is supported by curl since 7.52.0. This is very useful for networks with protocol whitelist. Signed-off-by: Wei Shuyu --- http.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/http.c b/http.c index 215bebef1..32d33261c 100644 --- a/http.c +++ b/http.c @@ -865,6 +865,11 @@ static CURL *get_curl_handle(void) else if (starts_with(curl_http_proxy, "socks")) curl_easy_setopt(result, CURLOPT_PROXYTYPE, CURLPROXY_SOCKS4); +#endif +#if LIBCURL_VERSION_NUM >= 0x073400 + else if (starts_with(curl_http_proxy, "https")) + curl_easy_setopt(result, + CURLOPT_PROXYTYPE, CURLPROXY_HTTPS); #endif if (strstr(curl_http_proxy, "://")) credential_from_url(&proxy_auth, curl_http_proxy); -- 2.15.1 ``` ## Jonathan Nieder, 2017-12-19 20:59 Subject: Re: [PATCH] http: support CURLPROXY_HTTPS Message-ID: <20171219205807.GC240141@aiede.mtv.corp.google.com> URL: https://gitlist.dev/e/20171219205807.GC240141%40aiede.mtv.corp.google.com In-Reply-To: <20171219172401.5263-1-wsy@dogben.com> ``` Hi, Wei Shuyu wrote: > HTTP proxy over SSL is supported by curl since 7.52.0. > This is very useful for networks with protocol whitelist. > > Signed-off-by: Wei Shuyu > --- > http.c | 5 +++++ > 1 file changed, 5 insertions(+) Thanks for writing this. Can you give an example of how I'd use it (ideally in the form of a test in t/ so we avoid this functionality regressing, but if that's not straightforward then an example for the commit message is fine as well)? > diff --git a/http.c b/http.c > index 215bebef1..32d33261c 100644 > --- a/http.c > +++ b/http.c > @@ -865,6 +865,11 @@ static CURL *get_curl_handle(void) > else if (starts_with(curl_http_proxy, "socks")) > curl_easy_setopt(result, > CURLOPT_PROXYTYPE, CURLPROXY_SOCKS4); > +#endif > +#if LIBCURL_VERSION_NUM >= 0x073400 Can this use #ifdef CURLPROXY_HTTPS instead? That way, if someone's copy of curl has backported support then they get the benefit of this change as well. > + else if (starts_with(curl_http_proxy, "https")) > + curl_easy_setopt(result, > + CURLOPT_PROXYTYPE, CURLPROXY_HTTPS); > #endif > if (strstr(curl_http_proxy, "://")) > credential_from_url(&proxy_auth, curl_http_proxy); Thanks and hope that helps, Jonathan ``` ## Junio C Hamano, 2017-12-19 21:38 Subject: Re: [PATCH] http: support CURLPROXY_HTTPS Message-ID: URL: https://gitlist.dev/e/xmqq7etiv1n5.fsf%40gitster.mtv.corp.google.com In-Reply-To: <20171219205807.GC240141@aiede.mtv.corp.google.com> ``` Jonathan Nieder writes: > Hi, > > Wei Shuyu wrote: > >> HTTP proxy over SSL is supported by curl since 7.52.0. >> This is very useful for networks with protocol whitelist. >> >> Signed-off-by: Wei Shuyu >> --- >> http.c | 5 +++++ >> 1 file changed, 5 insertions(+) > > Thanks for writing this. Can you give an example of how I'd use it > (ideally in the form of a test in t/ so we avoid this functionality > regressing, but if that's not straightforward then an example for the > commit message is fine as well)? Just FYI, here is an entry I added to the What's cooking report (which will be used as the log message for a merge commit that pulls this topic in, and will become an entry in the release notes if this topic ever becomes a part of a release). Git has been taught to support an https:// used for http.proxy when using recent versions of libcurl. There are multiple ways other than http.proxy configuration variable that a user can use to tell Git to use a proxy; I do not think the log message of this change is a place to enumerate all of them, but showing one of them to the readers would be good to remind them what we are talking about, I would guess. >> diff --git a/http.c b/http.c >> index 215bebef1..32d33261c 100644 >> --- a/http.c >> +++ b/http.c >> @@ -865,6 +865,11 @@ static CURL *get_curl_handle(void) >> else if (starts_with(curl_http_proxy, "socks")) >> curl_easy_setopt(result, >> CURLOPT_PROXYTYPE, CURLPROXY_SOCKS4); >> +#endif >> +#if LIBCURL_VERSION_NUM >= 0x073400 > > Can this use #ifdef CURLPROXY_HTTPS instead? That way, if someone's > copy of curl has backported support then they get the benefit of this > change as well. It sounds like a worthwhile thing to do (assuming that these are always implemented as preprocessor macros). >> + else if (starts_with(curl_http_proxy, "https")) >> + curl_easy_setopt(result, >> + CURLOPT_PROXYTYPE, CURLPROXY_HTTPS); >> #endif >> if (strstr(curl_http_proxy, "://")) >> credential_from_url(&proxy_auth, curl_http_proxy); > > Thanks and hope that helps, > Jonathan ``` ## Jonathan Nieder, 2017-12-19 21:50 Subject: Re: [PATCH] http: support CURLPROXY_HTTPS Message-ID: <20171219215031.GE240141@aiede.mtv.corp.google.com> URL: https://gitlist.dev/e/20171219215031.GE240141%40aiede.mtv.corp.google.com In-Reply-To: ``` Junio C Hamano wrote: > Jonathan Nieder writes: >> Wei Shuyu wrote: >>> diff --git a/http.c b/http.c >>> index 215bebef1..32d33261c 100644 >>> --- a/http.c >>> +++ b/http.c >>> @@ -865,6 +865,11 @@ static CURL *get_curl_handle(void) >>> else if (starts_with(curl_http_proxy, "socks")) >>> curl_easy_setopt(result, >>> CURLOPT_PROXYTYPE, CURLPROXY_SOCKS4); >>> +#endif >>> +#if LIBCURL_VERSION_NUM >= 0x073400 >> >> Can this use #ifdef CURLPROXY_HTTPS instead? That way, if someone's >> copy of curl has backported support then they get the benefit of this >> change as well. > > It sounds like a worthwhile thing to do (assuming that these are > always implemented as preprocessor macros). Oh, good point! It's an enumerator, not a preprocessor macro. But there is a preprocessor macro CURL_VERSION_HTTPS_PROXY. Anyway, using LIBCURL_VERSION_NUM is consistent with the surrounding code. Thanks, Jonathan ``` ## Wei Shuyu, 2017-12-20 02:30 Subject: Re: [PATCH] http: support CURLPROXY_HTTPS Message-ID: URL: https://gitlist.dev/e/a67c7c12ddbf3389dcc45ca09b4c5247%40dogben.com In-Reply-To: ``` On 2017-12-20 10:22, Wei Shuyu wrote: > CURLPROXY_HTTPS is intended for run-time detection. I don't think it's > a > good idea to use it with #ifdef. s/CURLPROXY_HTTPS/CURL_VERSION_HTTPS_PROXY/ ``` ## Jeff King, 2017-12-20 11:41 Subject: Re: [PATCH] http: support CURLPROXY_HTTPS Message-ID: <20171220114131.GA16643@sigill.intra.peff.net> URL: https://gitlist.dev/e/20171220114131.GA16643%40sigill.intra.peff.net In-Reply-To: ``` On Wed, Dec 20, 2017 at 10:22:06AM +0800, Wei Shuyu wrote: > On 2017-12-20 04:59, Jonathan Nieder wrote: > > > Thanks for writing this. Can you give an example of how I'd use it > > (ideally in the form of a test in t/ so we avoid this functionality > > regressing, but if that's not straightforward then an example for the > > commit message is fine as well)? > > Hi Jonathan, > Its usage is the same as other protocols. Just set http.proxy or > http_proxy/https_proxy > environment to https://url. > > To use apache server as a proxy, just add `ProxyRequests On` to an https > site. Unfortunately I don't think we have any proxy tests at all in our test suite right now. The sticking point is that we need an actual proxy to test against. :) If it really is as simple as "ProxyRequests On", then we might be able to convince the existing apache process we run to proxy requests to itself (perhaps on a secondary port?). -Peff ```