{"thread":{"id":"47466","subject":"[PATCH] http: support CURLPROXY_HTTPS","startedAt":"2017-12-19T17:24:43Z","lastAt":"2017-12-20T11:41:44Z","messageCount":6,"participants":["Wei Shuyu","Jonathan Nieder","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"335030","messageId":"20171219172401.5263-1-wsy@dogben.com","threadId":"47466","inReplyTo":null,"subject":"[PATCH] http: support CURLPROXY_HTTPS","fromName":"Wei Shuyu","fromEmail":"wsy@dogben.com","sentAt":"2017-12-19T17:24:01Z","receivedAt":"2017-12-19T17:24:43Z","isPatch":true,"sender":{"key":"wsy@dogben.com","avatar":"https://avatars.githubusercontent.com/u/7259503?v=4"},"body":"HTTP proxy over SSL is supported by curl since 7.52.0.\nThis is very useful for networks with protocol whitelist.\n\nSigned-off-by: Wei Shuyu <wsy@dogben.com>\n---\n http.c | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/http.c b/http.c\nindex 215bebef1..32d33261c 100644\n--- a/http.c\n+++ b/http.c\n@@ -865,6 +865,11 @@ static CURL *get_curl_handle(void)\n \t\telse if (starts_with(curl_http_proxy, \"socks\"))\n \t\t\tcurl_easy_setopt(result,\n \t\t\t\tCURLOPT_PROXYTYPE, CURLPROXY_SOCKS4);\n+#endif\n+#if LIBCURL_VERSION_NUM >= 0x073400\n+\t\telse if (starts_with(curl_http_proxy, \"https\"))\n+\t\t\tcurl_easy_setopt(result,\n+\t\t\t\tCURLOPT_PROXYTYPE, CURLPROXY_HTTPS);\n #endif\n \t\tif (strstr(curl_http_proxy, \"://\"))\n \t\t\tcredential_from_url(&proxy_auth, curl_http_proxy);\n-- \n2.15.1\n\n"},{"id":"335041","messageId":"20171219205807.GC240141@aiede.mtv.corp.google.com","threadId":"47466","inReplyTo":"20171219172401.5263-1-wsy@dogben.com","subject":"Re: [PATCH] http: support CURLPROXY_HTTPS","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-12-19T20:59:21Z","receivedAt":"2017-12-19T20:59:49Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nWei Shuyu wrote:\n\n> HTTP proxy over SSL is supported by curl since 7.52.0.\n> This is very useful for networks with protocol whitelist.\n>\n> Signed-off-by: Wei Shuyu <wsy@dogben.com>\n> ---\n>  http.c | 5 +++++\n>  1 file changed, 5 insertions(+)\n\nThanks for writing this.  Can you give an example of how I'd use it\n(ideally in the form of a test in t/ so we avoid this functionality\nregressing, but if that's not straightforward then an example for the\ncommit message is fine as well)?\n\n> diff --git a/http.c b/http.c\n> index 215bebef1..32d33261c 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -865,6 +865,11 @@ static CURL *get_curl_handle(void)\n>  \t\telse if (starts_with(curl_http_proxy, \"socks\"))\n>  \t\t\tcurl_easy_setopt(result,\n>  \t\t\t\tCURLOPT_PROXYTYPE, CURLPROXY_SOCKS4);\n> +#endif\n> +#if LIBCURL_VERSION_NUM >= 0x073400\n\nCan this use #ifdef CURLPROXY_HTTPS instead?  That way, if someone's\ncopy of curl has backported support then they get the benefit of this\nchange as well.\n\n> +\t\telse if (starts_with(curl_http_proxy, \"https\"))\n> +\t\t\tcurl_easy_setopt(result,\n> +\t\t\t\tCURLOPT_PROXYTYPE, CURLPROXY_HTTPS);\n>  #endif\n>  \t\tif (strstr(curl_http_proxy, \"://\"))\n>  \t\t\tcredential_from_url(&proxy_auth, curl_http_proxy);\n\nThanks and hope that helps,\nJonathan\n"},{"id":"335043","messageId":"xmqq7etiv1n5.fsf@gitster.mtv.corp.google.com","threadId":"47466","inReplyTo":"20171219205807.GC240141@aiede.mtv.corp.google.com","subject":"Re: [PATCH] http: support CURLPROXY_HTTPS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-19T21:38:54Z","receivedAt":"2017-12-19T21:42:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Hi,\n>\n> Wei Shuyu wrote:\n>\n>> HTTP proxy over SSL is supported by curl since 7.52.0.\n>> This is very useful for networks with protocol whitelist.\n>>\n>> Signed-off-by: Wei Shuyu <wsy@dogben.com>\n>> ---\n>>  http.c | 5 +++++\n>>  1 file changed, 5 insertions(+)\n>\n> Thanks for writing this.  Can you give an example of how I'd use it\n> (ideally in the form of a test in t/ so we avoid this functionality\n> regressing, but if that's not straightforward then an example for the\n> commit message is fine as well)?\n\nJust FYI, here is an entry I added to the What's cooking report\n(which will be used as the log message for a merge commit that pulls\nthis topic in, and will become an entry in the release notes if this\ntopic ever becomes a part of a release).\n\n Git has been taught to support an https:// used for http.proxy when\n using recent versions of libcurl.\n\nThere are multiple ways other than http.proxy configuration variable\nthat a user can use to tell Git to use a proxy; I do not think the\nlog message of this change is a place to enumerate all of them, but\nshowing one of them to the readers would be good to remind them what\nwe are talking about, I would guess.\n\n>> diff --git a/http.c b/http.c\n>> index 215bebef1..32d33261c 100644\n>> --- a/http.c\n>> +++ b/http.c\n>> @@ -865,6 +865,11 @@ static CURL *get_curl_handle(void)\n>>  \t\telse if (starts_with(curl_http_proxy, \"socks\"))\n>>  \t\t\tcurl_easy_setopt(result,\n>>  \t\t\t\tCURLOPT_PROXYTYPE, CURLPROXY_SOCKS4);\n>> +#endif\n>> +#if LIBCURL_VERSION_NUM >= 0x073400\n>\n> Can this use #ifdef CURLPROXY_HTTPS instead?  That way, if someone's\n> copy of curl has backported support then they get the benefit of this\n> change as well.\n\nIt sounds like a worthwhile thing to do (assuming that these are\nalways implemented as preprocessor macros).\n\n>> +\t\telse if (starts_with(curl_http_proxy, \"https\"))\n>> +\t\t\tcurl_easy_setopt(result,\n>> +\t\t\t\tCURLOPT_PROXYTYPE, CURLPROXY_HTTPS);\n>>  #endif\n>>  \t\tif (strstr(curl_http_proxy, \"://\"))\n>>  \t\t\tcredential_from_url(&proxy_auth, curl_http_proxy);\n>\n> Thanks and hope that helps,\n> Jonathan\n"},{"id":"335044","messageId":"20171219215031.GE240141@aiede.mtv.corp.google.com","threadId":"47466","inReplyTo":"xmqq7etiv1n5.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] http: support CURLPROXY_HTTPS","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-12-19T21:50:31Z","receivedAt":"2017-12-19T21:50:39Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n>> Wei Shuyu wrote:\n\n>>> diff --git a/http.c b/http.c\n>>> index 215bebef1..32d33261c 100644\n>>> --- a/http.c\n>>> +++ b/http.c\n>>> @@ -865,6 +865,11 @@ static CURL *get_curl_handle(void)\n>>>  \t\telse if (starts_with(curl_http_proxy, \"socks\"))\n>>>  \t\t\tcurl_easy_setopt(result,\n>>>  \t\t\t\tCURLOPT_PROXYTYPE, CURLPROXY_SOCKS4);\n>>> +#endif\n>>> +#if LIBCURL_VERSION_NUM >= 0x073400\n>>\n>> Can this use #ifdef CURLPROXY_HTTPS instead?  That way, if someone's\n>> copy of curl has backported support then they get the benefit of this\n>> change as well.\n>\n> It sounds like a worthwhile thing to do (assuming that these are\n> always implemented as preprocessor macros).\n\nOh, good point!  It's an enumerator, not a preprocessor macro.  But\nthere is a preprocessor macro CURL_VERSION_HTTPS_PROXY.\n\nAnyway, using LIBCURL_VERSION_NUM is consistent with the surrounding\ncode.\n\nThanks,\nJonathan\n"},{"id":"335062","messageId":"a67c7c12ddbf3389dcc45ca09b4c5247@dogben.com","threadId":"47466","inReplyTo":"a572179929e666e4e598930ec774c4db@dogben.com","subject":"Re: [PATCH] http: support CURLPROXY_HTTPS","fromName":"Wei Shuyu","fromEmail":"wsy@dogben.com","sentAt":"2017-12-20T02:30:34Z","receivedAt":"2017-12-20T02:30:44Z","isPatch":true,"sender":{"key":"wsy@dogben.com","avatar":"https://avatars.githubusercontent.com/u/7259503?v=4"},"body":"On 2017-12-20 10:22, Wei Shuyu wrote:\n\n> CURLPROXY_HTTPS is intended for run-time detection. I don't think it's \n> a\n> good idea to use it with #ifdef.\n\ns/CURLPROXY_HTTPS/CURL_VERSION_HTTPS_PROXY/\n"},{"id":"335068","messageId":"20171220114131.GA16643@sigill.intra.peff.net","threadId":"47466","inReplyTo":"a572179929e666e4e598930ec774c4db@dogben.com","subject":"Re: [PATCH] http: support CURLPROXY_HTTPS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-12-20T11:41:31Z","receivedAt":"2017-12-20T11:41:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Dec 20, 2017 at 10:22:06AM +0800, Wei Shuyu wrote:\n\n> On 2017-12-20 04:59, Jonathan Nieder wrote:\n> \n> > Thanks for writing this.  Can you give an example of how I'd use it\n> > (ideally in the form of a test in t/ so we avoid this functionality\n> > regressing, but if that's not straightforward then an example for the\n> > commit message is fine as well)?\n> \n> Hi Jonathan,\n> Its usage is the same as other protocols. Just set http.proxy or\n> http_proxy/https_proxy\n> environment to https://url.\n> \n> To use apache server as a proxy, just add `ProxyRequests On` to an https\n> site.\n\nUnfortunately I don't think we have any proxy tests at all in our test\nsuite right now. The sticking point is that we need an actual proxy to\ntest against. :)\n\nIf it really is as simple as \"ProxyRequests On\", then we might be able\nto convince the existing apache process we run to proxy requests to\nitself (perhaps on a secondary port?).\n\n-Peff\n"}]}