{"thread":{"id":"30407","subject":"[PATCH 1/6] http: try http_proxy env var when http.proxy config option is not set","startedAt":"2012-05-03T16:39:47Z","lastAt":"2012-05-04T17:18:29Z","messageCount":9,"participants":["Nelson Benitez Leon","Junio C Hamano","Jeff King","Daniel Stenberg"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"190645","messageId":"4FA2B4D3.90809@seap.minhap.es","threadId":"30407","inReplyTo":null,"subject":"[PATCH 1/6] http: try http_proxy env var when http.proxy config option is not set","fromName":"Nelson Benitez Leon","fromEmail":"nelsonjesus.benitez@seap.minhap.es","sentAt":"2012-05-03T16:39:47Z","receivedAt":"2012-05-03T16:39:47Z","isPatch":true,"sender":{"key":"nelsonjesus.benitez@seap.minhap.es","avatar":null},"body":"cURL already reads it, but if $http_proxy has username but no password\ncURL will not ask you for the password and so failed to authenticate\nreturning a 407 error code. So we read it ourselves to detect that and\nask for the password. Also we read it prior to connection to be able to\nmake a proactive authentication in case the flag http_proactive_auth is\nset.\n\nWe also take care to read env proxy var according to protocol being\nused in the destination url, e.g.  when the url to retrieve is a https\none, then the proxy env var we look at is https_proxy. We also look at\nthe uppercase version of these if the lowercase is not found, with the\nexception of HTTP_PROXY because cURL ignores it. To make this possible\nwe now passed destination url parameter to get_active_slot() and\nget_curl_handle() functions.\n\nWe also read no_proxy env var so to ignore aforementioned proxy env var\nif no_proxy contains an asterisk ('*') or contains the host used in url\ndestination.\n\nSigned-off-by: Nelson Benitez Leon <nbenitezl@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n http-push.c   |   24 +++++++++++-----------\n http-walker.c |    2 +-\n http.c        |   60 ++++++++++++++++++++++++++++++++++++++++++++++++++------\n http.h        |    2 +-\n remote-curl.c |    4 +-\n 5 files changed, 69 insertions(+), 23 deletions(-)\n\ndiff --git a/http-push.c b/http-push.c\nindex 1df7ab5..4e23d00 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -297,7 +297,7 @@ static void start_mkcol(struct transfer_request *request)\n \n \trequest->url = get_remote_object_url(repo->url, hex, 1);\n \n-\tslot = get_active_slot();\n+\tslot = get_active_slot(request->url);\n \tslot->callback_func = process_response;\n \tslot->callback_data = request;\n \tcurl_setup_http_get(slot->curl, request->url, DAV_MKCOL);\n@@ -417,7 +417,7 @@ static void start_put(struct transfer_request *request)\n \tstrbuf_add(&buf, request->lock->tmpfile_suffix, 41);\n \trequest->url = strbuf_detach(&buf, NULL);\n \n-\tslot = get_active_slot();\n+\tslot = get_active_slot(request->url);\n \tslot->callback_func = process_response;\n \tslot->callback_data = request;\n \tcurl_setup_http(slot->curl, request->url, DAV_PUT,\n@@ -438,7 +438,7 @@ static void start_move(struct transfer_request *request)\n \tstruct active_request_slot *slot;\n \tstruct curl_slist *dav_headers = NULL;\n \n-\tslot = get_active_slot();\n+\tslot = get_active_slot(request->url);\n \tslot->callback_func = process_response;\n \tslot->callback_data = request;\n \tcurl_setup_http_get(slot->curl, request->url, DAV_MOVE);\n@@ -467,7 +467,7 @@ static int refresh_lock(struct remote_lock *lock)\n \n \tdav_headers = get_dav_token_headers(lock, DAV_HEADER_IF | DAV_HEADER_TIMEOUT);\n \n-\tslot = get_active_slot();\n+\tslot = get_active_slot(lock->url);\n \tslot->results = &results;\n \tcurl_setup_http_get(slot->curl, lock->url, DAV_LOCK);\n \tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, dav_headers);\n@@ -882,7 +882,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)\n \twhile (ep) {\n \t\tchar saved_character = ep[1];\n \t\tep[1] = '\\0';\n-\t\tslot = get_active_slot();\n+\t\tslot = get_active_slot(url);\n \t\tslot->results = &results;\n \t\tcurl_setup_http_get(slot->curl, url, DAV_MKCOL);\n \t\tif (start_active_slot(slot)) {\n@@ -912,7 +912,7 @@ static struct remote_lock *lock_remote(const char *path, long timeout)\n \tdav_headers = curl_slist_append(dav_headers, timeout_header);\n \tdav_headers = curl_slist_append(dav_headers, \"Content-Type: text/xml\");\n \n-\tslot = get_active_slot();\n+\tslot = get_active_slot(url);\n \tslot->results = &results;\n \tcurl_setup_http(slot->curl, url, DAV_LOCK, &out_buffer, fwrite_buffer);\n \tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, dav_headers);\n@@ -980,7 +980,7 @@ static int unlock_remote(struct remote_lock *lock)\n \n \tdav_headers = get_dav_token_headers(lock, DAV_HEADER_LOCK);\n \n-\tslot = get_active_slot();\n+\tslot = get_active_slot(lock->url);\n \tslot->results = &results;\n \tcurl_setup_http_get(slot->curl, lock->url, DAV_UNLOCK);\n \tcurl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, dav_headers);\n@@ -1158,7 +1158,7 @@ static void remote_ls(const char *path, int flags,\n \tdav_headers = curl_slist_append(dav_headers, \"Depth: 1\");\n \tdav_headers = curl_slist_append(dav_headers, \"Content-Type: text/xml\");\n \n-\tslot = get_active_slot();\n+\tslot = get_active_slot(url);\n \tslot->results = &results;\n \tcurl_setup_http(slot->curl, url, DAV_PROPFIND,\n \t\t\t&out_buffer, fwrite_buffer);\n@@ -1232,7 +1232,7 @@ static int locking_available(void)\n \tdav_headers = curl_slist_append(dav_headers, \"Depth: 0\");\n \tdav_headers = curl_slist_append(dav_headers, \"Content-Type: text/xml\");\n \n-\tslot = get_active_slot();\n+\tslot = get_active_slot(repo->url);\n \tslot->results = &results;\n \tcurl_setup_http(slot->curl, repo->url, DAV_PROPFIND,\n \t\t\t&out_buffer, fwrite_buffer);\n@@ -1409,7 +1409,7 @@ static int update_remote(unsigned char *sha1, struct remote_lock *lock)\n \n \tstrbuf_addf(&out_buffer.buf, \"%s\\n\", sha1_to_hex(sha1));\n \n-\tslot = get_active_slot();\n+\tslot = get_active_slot(lock->url);\n \tslot->results = &results;\n \tcurl_setup_http(slot->curl, lock->url, DAV_PUT,\n \t\t\t&out_buffer, fwrite_null);\n@@ -1535,7 +1535,7 @@ static void update_remote_info_refs(struct remote_lock *lock)\n \tif (!aborted) {\n \t\tdav_headers = get_dav_token_headers(lock, DAV_HEADER_IF);\n \n-\t\tslot = get_active_slot();\n+\t\tslot = get_active_slot(lock->url);\n \t\tslot->results = &results;\n \t\tcurl_setup_http(slot->curl, lock->url, DAV_PUT,\n \t\t\t\t&buffer, fwrite_null);\n@@ -1695,7 +1695,7 @@ static int delete_remote_branch(const char *pattern, int force)\n \t\treturn 0;\n \turl = xmalloc(strlen(repo->url) + strlen(remote_ref->name) + 1);\n \tsprintf(url, \"%s%s\", repo->url, remote_ref->name);\n-\tslot = get_active_slot();\n+\tslot = get_active_slot(url);\n \tslot->results = &results;\n \tcurl_setup_http_get(slot->curl, url, DAV_DELETE);\n \tif (start_active_slot(slot)) {\ndiff --git a/http-walker.c b/http-walker.c\nindex 51a906e..5d5ae34 100644\n--- a/http-walker.c\n+++ b/http-walker.c\n@@ -348,7 +348,7 @@ static void fetch_alternates(struct walker *walker, const char *base)\n \t * Use a callback to process the result, since another request\n \t * may fail and need to have alternates loaded before continuing\n \t */\n-\tslot = get_active_slot();\n+\tslot = get_active_slot(url);\n \tslot->callback_func = process_alternates_response;\n \talt_req.walker = walker;\n \tslot->callback_data = &alt_req;\ndiff --git a/http.c b/http.c\nindex 5cb87f1..64df7b1 100644\n--- a/http.c\n+++ b/http.c\n@@ -229,6 +229,37 @@ static void init_curl_http_auth(CURL *result)\n #endif\n }\n \n+static const char *read_prot_proxy_env(const char *protocol)\n+{\n+\tconst char *env_proxy;\n+\tstruct strbuf var = STRBUF_INIT;\n+\n+\tstrbuf_addf(&var, \"%s_proxy\", protocol);\n+\tenv_proxy = getenv(var.buf);\n+\tif (!env_proxy && strcmp(\"http_proxy\", var.buf)) {\n+\t\tchar *p;\n+\t\tfor (p = var.buf; *p; p++)\n+\t\t\t*p = toupper(*p);\n+\t\tenv_proxy = getenv(var.buf);\n+\t}\n+\tstrbuf_release(&var);\n+\t\n+\treturn env_proxy;\n+}\n+\n+static int host_allowed_by_noproxy_env (const char *host)\n+{\n+\tconst char *no_proxy = getenv(\"no_proxy\");\n+\tif (!no_proxy)\n+\t\tno_proxy = getenv(\"NO_PROXY\");\n+\tif (!no_proxy ||\n+\t    (strcmp(\"*\", no_proxy) &&\n+\t     !strstr(no_proxy, host)))\n+\t\treturn 1;\n+\t\n+\treturn 0;\n+}\n+\n static int has_cert_password(void)\n {\n \tif (ssl_cert == NULL || ssl_cert_password_required != 1)\n@@ -241,7 +272,7 @@ static int has_cert_password(void)\n \treturn 1;\n }\n \n-static CURL *get_curl_handle(void)\n+static CURL *get_curl_handle(const char *url)\n {\n \tCURL *result = curl_easy_init();\n \n@@ -304,6 +335,21 @@ static CURL *get_curl_handle(void)\n \tif (curl_ftp_no_epsv)\n \t\tcurl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0);\n \n+\tif (!curl_http_proxy) {\n+\t\tstatic struct credential parsed_url = CREDENTIAL_INIT;\n+\t\tcredential_from_url(&parsed_url, url);\n+\n+\t\tif (parsed_url.protocol) {\n+\t\t\tconst char *env_proxy;\n+\t\t\tenv_proxy = read_prot_proxy_env(parsed_url.protocol);\n+\n+\t\t\tif (env_proxy && host_allowed_by_noproxy_env(parsed_url.host))\n+\t\t\t\tcurl_http_proxy = xstrdup(env_proxy);\n+\t\t}\n+\n+\t\tcredential_clear(&parsed_url);\n+\t}\n+\t\n \tif (curl_http_proxy) {\n \t\tcurl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);\n \t\tcurl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);\n@@ -394,7 +440,7 @@ void http_init(struct remote *remote, const char *url, int proactive_auth)\n \t}\n \n #ifndef NO_CURL_EASY_DUPHANDLE\n-\tcurl_default = get_curl_handle();\n+\tcurl_default = get_curl_handle(url);\n #endif\n }\n \n@@ -443,7 +489,7 @@ void http_cleanup(void)\n \tssl_cert_password_required = 0;\n }\n \n-struct active_request_slot *get_active_slot(void)\n+struct active_request_slot *get_active_slot(const char *url)\n {\n \tstruct active_request_slot *slot = active_queue_head;\n \tstruct active_request_slot *newslot;\n@@ -481,7 +527,7 @@ struct active_request_slot *get_active_slot(void)\n \n \tif (slot->curl == NULL) {\n #ifdef NO_CURL_EASY_DUPHANDLE\n-\t\tslot->curl = get_curl_handle();\n+\t\tslot->curl = get_curl_handle(url);\n #else\n \t\tslot->curl = curl_easy_duphandle(curl_default);\n #endif\n@@ -756,7 +802,7 @@ static int http_request(const char *url, void *result, int target, int options)\n \tstruct strbuf buf = STRBUF_INIT;\n \tint ret;\n \n-\tslot = get_active_slot();\n+\tslot = get_active_slot(url);\n \tslot->results = &results;\n \tcurl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1);\n \n@@ -1101,7 +1147,7 @@ struct http_pack_request *new_http_pack_request(\n \t\tgoto abort;\n \t}\n \n-\tpreq->slot = get_active_slot();\n+\tpreq->slot = get_active_slot(preq->url);\n \tcurl_easy_setopt(preq->slot->curl, CURLOPT_FILE, preq->packfile);\n \tcurl_easy_setopt(preq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite);\n \tcurl_easy_setopt(preq->slot->curl, CURLOPT_URL, preq->url);\n@@ -1261,7 +1307,7 @@ struct http_object_request *new_http_object_request(const char *base_url,\n \t\t}\n \t}\n \n-\tfreq->slot = get_active_slot();\n+\tfreq->slot = get_active_slot(freq->url);\n \n \tcurl_easy_setopt(freq->slot->curl, CURLOPT_FILE, freq);\n \tcurl_easy_setopt(freq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite_sha1_file);\ndiff --git a/http.h b/http.h\nindex 915c286..483e3ed 100644\n--- a/http.h\n+++ b/http.h\n@@ -73,7 +73,7 @@ extern curlioerr ioctl_buffer(CURL *handle, int cmd, void *clientp);\n #endif\n \n /* Slot lifecycle functions */\n-extern struct active_request_slot *get_active_slot(void);\n+extern struct active_request_slot *get_active_slot(const char *url);\n extern int start_active_slot(struct active_request_slot *slot);\n extern void run_active_slot(struct active_request_slot *slot);\n extern void finish_active_slot(struct active_request_slot *slot);\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 0896221..6ceba7a 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -384,7 +384,7 @@ static int probe_rpc(struct rpc_state *rpc)\n \tstruct strbuf buf = STRBUF_INIT;\n \tint err;\n \n-\tslot = get_active_slot();\n+\tslot = get_active_slot(rpc->service_url);\n \n \theaders = curl_slist_append(headers, rpc->hdr_content_type);\n \theaders = curl_slist_append(headers, rpc->hdr_accept);\n@@ -441,7 +441,7 @@ static int post_rpc(struct rpc_state *rpc)\n \t\t\treturn err;\n \t}\n \n-\tslot = get_active_slot();\n+\tslot = get_active_slot(rpc->service_url);\n \n \tcurl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0);\n \tcurl_easy_setopt(slot->curl, CURLOPT_POST, 1);\n-- \n1.7.7.6\n"},{"id":"190660","messageId":"7vk40tf8cy.fsf@alter.siamese.dyndns.org","threadId":"30407","inReplyTo":"4FA2B4D3.90809@seap.minhap.es","subject":"Re: [PATCH 1/6] http: try http_proxy env var when http.proxy config option is not set","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-03T18:05:33Z","receivedAt":"2012-05-03T18:05:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nelson Benitez Leon <nelsonjesus.benitez@seap.minhap.es> writes:\n\n> cURL already reads it, but if $http_proxy has username but no password\n> cURL will not ask you for the password and so failed to authenticate\n> returning a 407 error code. So we read it ourselves to detect that and\n> ask for the password. Also we read it prior to connection to be able to\n> make a proactive authentication in case the flag http_proactive_auth is\n> set.\n>\n> We also take care to read env proxy var according to protocol being\n> used in the destination url, e.g.  when the url to retrieve is a https\n> one, then the proxy env var we look at is https_proxy. We also look at\n> the uppercase version of these if the lowercase is not found, with the\n> exception of HTTP_PROXY because cURL ignores it. To make this possible\n> we now passed destination url parameter to get_active_slot() and\n> get_curl_handle() functions.\n>\n> We also read no_proxy env var so to ignore aforementioned proxy env var\n> if no_proxy contains an asterisk ('*') or contains the host used in url\n> destination.\n>\n> Signed-off-by: Nelson Benitez Leon <nbenitezl@gmail.com>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\nI'll trust Peff to point out anything I missed, but from a cursory look,\nthe result looks much cleaner than the previous round.\n\n> diff --git a/http.c b/http.c\n> index 5cb87f1..64df7b1 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -229,6 +229,37 @@ static void init_curl_http_auth(CURL *result)\n> ...\n> +static int host_allowed_by_noproxy_env (const char *host)\n> +{\n\nI'll queue the updated series with s/_env (/_env(/; here, and also add a\nmissing explanation on the bulk of \"noise\" in the patch at the end of the\nlog message:\n\n    In order to be able to determine what proxy settings is needed from\n    the very beginning of a request, get_active_slot() learns to take the\n    destination URL, as it needs to pass it to get_curl_handle() that\n    implements the logic to pick proxies based on the protocol used.\n\n\nThanks.\n"},{"id":"190726","messageId":"20120504070802.GA21895@sigill.intra.peff.net","threadId":"30407","inReplyTo":"4FA2B4D3.90809@seap.minhap.es","subject":"Re: [PATCH 1/6] http: try http_proxy env var when http.proxy config option is not set","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-04T07:08:02Z","receivedAt":"2012-05-04T07:08:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 03, 2012 at 06:39:47PM +0200, Nelson Benitez Leon wrote:\n\n> +static const char *read_prot_proxy_env(const char *protocol)\n> +{\n> +\tconst char *env_proxy;\n> +\tstruct strbuf var = STRBUF_INIT;\n> +\n> +\tstrbuf_addf(&var, \"%s_proxy\", protocol);\n> +\tenv_proxy = getenv(var.buf);\n> +\tif (!env_proxy && strcmp(\"http_proxy\", var.buf)) {\n> +\t\tchar *p;\n> +\t\tfor (p = var.buf; *p; p++)\n> +\t\t\t*p = toupper(*p);\n> +\t\tenv_proxy = getenv(var.buf);\n> +\t}\n> +\tstrbuf_release(&var);\n> +\t\n> +\treturn env_proxy;\n> +}\n\nThanks, this is way more readable than the previous iteration.\n\n> +static int host_allowed_by_noproxy_env (const char *host)\n> +{\n> +\tconst char *no_proxy = getenv(\"no_proxy\");\n> +\tif (!no_proxy)\n> +\t\tno_proxy = getenv(\"NO_PROXY\");\n> +\tif (!no_proxy ||\n> +\t    (strcmp(\"*\", no_proxy) &&\n> +\t     !strstr(no_proxy, host)))\n> +\t\treturn 1;\n> +\t\n> +\treturn 0;\n> +}\n\nThis simplified parsing misses a lot of corner cases. Three I can see\nright off the bat:\n\n  1. If your NO_PROXY is \"no-proxy.com\", and your host is\n     \"proxy.com\", your code will consider that a match, but curl does\n     not.\n\n  2. If your NO_PROXY contains \"no-proxy.com\", but your host is\n     \"www.no-proxy.com\", curl will consider that a match, but your code\n     does not.\n\n  3. If your NO_PROXY contains \"no-proxy.com\", but your host is\n     \"no-proxy.com:80\", curl will consider that a match, but your code\n     does not.\n\nI don't see any way around it besides implementing curl's full\ntokenizing and matching algorithm, which is about a page of code. I'd\nreally prefer not to re-implement bits of curl (especially because they\nmay change later), but AFAIK there is no way to ask curl \"is there a\nproxy configured, and if so, what is it?\".\n\nThe rest of this patch looks OK to me, though.\n\n-Peff\n"},{"id":"190735","messageId":"alpine.DEB.2.00.1205040921090.12158@tvnag.unkk.fr","threadId":"30407","inReplyTo":"20120504070802.GA21895@sigill.intra.peff.net","subject":"Re: [PATCH 1/6] http: try http_proxy env var when http.proxy config option is not set","fromName":"Daniel Stenberg","fromEmail":"daniel@haxx.se","sentAt":"2012-05-04T07:27:16Z","receivedAt":"2012-05-04T07:27:16Z","isPatch":true,"sender":{"key":"daniel@haxx.se","avatar":"https://gravatar.com/avatar/69fdca87edd17cee21ca2e79fc2ff671d644603c3dc27167430f3cd3dbab7ba8?d=mp&s=160"},"body":"On Fri, 4 May 2012, Jeff King wrote:\n\n> I don't see any way around it besides implementing curl's full tokenizing \n> and matching algorithm, which is about a page of code. I'd really prefer not \n> to re-implement bits of curl (especially because they may change later), but \n> AFAIK there is no way to ask curl \"is there a proxy configured, and if so, \n> what is it?\".\n\nSorry for being thick, but I lost track on this thread. Why does it need this \ninfo again?\n\nOr perhaps put another way: if there was an ideal way to get this done or \nprovide this to libcurl other than the current way, how would you suggest it \nwould be done from a git internal point of view?\n\nWe're currently discussing new ways of providing authentication info in \nlibcurl and I want to make sure I can get useful bits from this exercise into \nthat talk to possibly offer something smoother in the future.\n\n-- \n\n  / daniel.haxx.se\n"},{"id":"190736","messageId":"20120504073913.GA22388@sigill.intra.peff.net","threadId":"30407","inReplyTo":"alpine.DEB.2.00.1205040921090.12158@tvnag.unkk.fr","subject":"Re: [PATCH 1/6] http: try http_proxy env var when http.proxy config option is not set","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-04T07:39:13Z","receivedAt":"2012-05-04T07:39:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 04, 2012 at 09:27:16AM +0200, Daniel Stenberg wrote:\n\n> On Fri, 4 May 2012, Jeff King wrote:\n> \n> >I don't see any way around it besides implementing curl's full\n> >tokenizing and matching algorithm, which is about a page of code.\n> >I'd really prefer not to re-implement bits of curl (especially\n> >because they may change later), but AFAIK there is no way to ask\n> >curl \"is there a proxy configured, and if so, what is it?\".\n> \n> Sorry for being thick, but I lost track on this thread. Why does it\n> need this info again?\n\nWe need to know information about the proxy in order to look up the\nusername and password in our credential database. Before the request is\nmade in some cases, and in others, after we see a 407. If we fed the\nproxy to curl via CURLOPT_PROXY, it's easy. But if the proxy came from\nthe environment, we have to replicate curl's lookup rules.\n\n> Or perhaps put another way: if there was an ideal way to get this\n> done or provide this to libcurl other than the current way, how would\n> you suggest it would be done from a git internal point of view?\n\nThe absolute simplest way for us would be to stop using\nCURLOPT_PROXYUSERNAME/PASSWORD to set it ahead of time, and instead\nprovide a callback that curl would call on a 407. That callback would\njust need the URL of the proxy, and would return the username/password\n(or even just set them on the curl object via\nCURLOPT_PROXYUSERNAME/PASSWORD).\n\nFor that matter, it would simplify our code to do the same for regular\nhttp auth, too. And though we usually know our URL in that case, we\nmight not if we got a 302 with FOLLOWLOCATION set.\n\n-Peff\n"},{"id":"190757","messageId":"alpine.DEB.2.00.1205041710490.12158@tvnag.unkk.fr","threadId":"30407","inReplyTo":"20120504073913.GA22388@sigill.intra.peff.net","subject":"Re: [PATCH 1/6] http: try http_proxy env var when http.proxy config option is not set","fromName":"Daniel Stenberg","fromEmail":"daniel@haxx.se","sentAt":"2012-05-04T15:12:00Z","receivedAt":"2012-05-04T15:12:00Z","isPatch":true,"sender":{"key":"daniel@haxx.se","avatar":"https://gravatar.com/avatar/69fdca87edd17cee21ca2e79fc2ff671d644603c3dc27167430f3cd3dbab7ba8?d=mp&s=160"},"body":"On Fri, 4 May 2012, Jeff King wrote:\n\n> The absolute simplest way for us would be to stop using\n> CURLOPT_PROXYUSERNAME/PASSWORD to set it ahead of time, and instead\n> provide a callback that curl would call on a 407. That callback would\n> just need the URL of the proxy, and would return the username/password\n> (or even just set them on the curl object via\n> CURLOPT_PROXYUSERNAME/PASSWORD).\n>\n> For that matter, it would simplify our code to do the same for regular http \n> auth, too. And though we usually know our URL in that case, we might not if \n> we got a 302 with FOLLOWLOCATION set.\n\nThanks a lot. That is in fact almost exactly the solution we're discussing \nright now.\n\n-- \n\n  / daniel.haxx.se\n"},{"id":"190759","messageId":"7v4nrwc61e.fsf@alter.siamese.dyndns.org","threadId":"30407","inReplyTo":"20120504073913.GA22388@sigill.intra.peff.net","subject":"Re: [PATCH 1/6] http: try http_proxy env var when http.proxy config option is not set","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-04T15:36:13Z","receivedAt":"2012-05-04T15:36:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Fri, May 04, 2012 at 09:27:16AM +0200, Daniel Stenberg wrote:\n>\n>> On Fri, 4 May 2012, Jeff King wrote:\n>> \n>> >I don't see any way around it besides implementing curl's full\n>> >tokenizing and matching algorithm, which is about a page of code.\n>> >I'd really prefer not to re-implement bits of curl (especially\n>> >because they may change later), but AFAIK there is no way to ask\n>> >curl \"is there a proxy configured, and if so, what is it?\".\n>> \n>> Sorry for being thick, but I lost track on this thread. Why does it\n>> need this info again?\n>\n> We need to know information about the proxy in order to look up the\n> username and password in our credential database. Before the request is\n> made in some cases, and in others, after we see a 407. If we fed the\n> proxy to curl via CURLOPT_PROXY, it's easy. But if the proxy came from\n> the environment, we have to replicate curl's lookup rules.\n>\n>> Or perhaps put another way: if there was an ideal way to get this\n>> done or provide this to libcurl other than the current way, how would\n>> you suggest it would be done from a git internal point of view?\n>\n> The absolute simplest way for us would be to stop using\n> CURLOPT_PROXYUSERNAME/PASSWORD to set it ahead of time, and instead\n> provide a callback that curl would call on a 407. That callback would\n> just need the URL of the proxy, and would return the username/password\n> (or even just set them on the curl object via\n> CURLOPT_PROXYUSERNAME/PASSWORD).\n>\n> For that matter, it would simplify our code to do the same for regular\n> http auth, too. And though we usually know our URL in that case, we\n> might not if we got a 302 with FOLLOWLOCATION set.\n>\n> -Peff\n\nThanks for a nice summary, and I agree with your list of what we wish we\nhad from the cURL library.  With such a change, it becomes irrelevant how\nthe user fed cURL provisional (partial) authentication information, either\nin http.proxy (which we turn into CURLOPT_PROXY), or from the environment\n(without Git having to know anything about it), and a lot of complexity\nthat led to bugs in this series will become unnecessary.\n\nI am tempted to suggest that the current series should be rerolled without\nall the guessing and preauth tricks until such an update to the cURL\nlibrary materializes.\n"},{"id":"190763","messageId":"20120504160543.GB1331@sigill.intra.peff.net","threadId":"30407","inReplyTo":"7v4nrwc61e.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/6] http: try http_proxy env var when http.proxy config option is not set","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-04T16:05:43Z","receivedAt":"2012-05-04T16:05:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 04, 2012 at 08:36:13AM -0700, Junio C Hamano wrote:\n\n> Thanks for a nice summary, and I agree with your list of what we wish we\n> had from the cURL library.  With such a change, it becomes irrelevant how\n> the user fed cURL provisional (partial) authentication information, either\n> in http.proxy (which we turn into CURLOPT_PROXY), or from the environment\n> (without Git having to know anything about it), and a lot of complexity\n> that led to bugs in this series will become unnecessary.\n> \n> I am tempted to suggest that the current series should be rerolled without\n> all the guessing and preauth tricks until such an update to the cURL\n> library materializes.\n\nI am very tempted by that, too. In the meantime (and even once that curl\nversion comes out and we write the new code, people will still be on the\nolder version of curl), we have a fallback: they can use http.proxy if\nthey want auth support. It's not as nice as supporting auth on the\nenvironment variables, but I think it will end up being a lot cleaner.\n\n-Peff\n"},{"id":"190770","messageId":"7vwr4ramqi.fsf@alter.siamese.dyndns.org","threadId":"30407","inReplyTo":"20120504160543.GB1331@sigill.intra.peff.net","subject":"Re: [PATCH 1/6] http: try http_proxy env var when http.proxy config option is not set","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-04T17:18:29Z","receivedAt":"2012-05-04T17:18:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I am very tempted by that, too. In the meantime (and even once that curl\n> version comes out and we write the new code, people will still be on the\n> older version of curl), we have a fallback: they can use http.proxy if\n> they want auth support. It's not as nice as supporting auth on the\n> environment variables, but I think it will end up being a lot cleaner.\n\nSounds good.\n"}]}