{"thread":{"id":"29772","subject":"[PATCH 3/3] http: when proxy url has username but no password, ask for password","startedAt":"2012-02-28T12:56:29Z","lastAt":"2012-02-29T10:46:08Z","messageCount":3,"participants":["Nelson Benitez Leon","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"185604","messageId":"4F4CCEFD.90402@seap.minhap.es","threadId":"29772","inReplyTo":null,"subject":"[PATCH 3/3] http: when proxy url has username but no password, ask for password","fromName":"Nelson Benitez Leon","fromEmail":"nelsonjesus.benitez@seap.minhap.es","sentAt":"2012-02-28T12:56:29Z","receivedAt":"2012-02-28T12:56:29Z","isPatch":true,"sender":{"key":"nelsonjesus.benitez@seap.minhap.es","avatar":null},"body":"Signed-off-by: Nelson Benitez Leon <nbenitezl@gmail.com>\n---\n http.c |   36 +++++++++++++++++++++++++++++++++++-\n 1 files changed, 35 insertions(+), 1 deletions(-)\n\ndiff --git a/http.c b/http.c\nindex 79cbe50..68e3f7d 100644\n--- a/http.c\n+++ b/http.c\n@@ -306,7 +306,41 @@ static CURL *get_curl_handle(void)\n \t\t}\n \t}\n \tif (curl_http_proxy) {\n-\t\tcurl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);\n+\t\tchar *at, *colon, *proxyuser;\n+\t\tconst char *cp;\n+\t\tcp = strstr(curl_http_proxy, \"://\");\n+\t\tif (cp == NULL) {\n+\t\t\tcp = curl_http_proxy;\n+\t\t} else {\n+\t\t\tcp += 3;\n+\t\t}\n+\t\tat = strchr(cp, '@');\n+\t\tcolon = strchr(cp, ':');\n+\t\tif (at && (!colon || at < colon)) {\n+\t\t\t/* proxy string has username but no password, ask for password */\n+\t\t\tchar *ask_str, *proxyuser, *proxypass;\n+\t\t\tint len;\n+\t\t\tstruct strbuf pbuf = STRBUF_INIT;\n+\t\t\tlen = at - cp;\n+\t\t\tproxyuser = xmalloc(len + 1);\n+\t\t\tmemcpy(proxyuser, cp, len);\n+\t\t\tproxyuser[len] = '\\0';\n+\t\t\t\n+\t\t\tstrbuf_addf(&pbuf, \"Enter password for proxy %s...\", at+1);\n+\t\t\task_str = strbuf_detach(&pbuf, NULL);\n+\t\t\tproxypass = xstrdup(git_getpass(ask_str));\n+\t\t\t\n+\t\t\tstrbuf_insert(&pbuf, 0, curl_http_proxy, cp - curl_http_proxy);\n+\t\t\tstrbuf_addf(&pbuf, \"%s:%s\", proxyuser, proxypass);\n+\t\t\tstrbuf_add(&pbuf, at, strlen(at));\n+\t\t\tcurl_easy_setopt(result, CURLOPT_PROXY, strbuf_detach(&pbuf, NULL));\n+\t\t\t\n+\t\t\tfree(ask_str);\n+\t\t\tfree(proxyuser);\n+\t\t\tfree(proxypass);\n+\t\t} else {\n+\t\t\tcurl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);\n+\t\t}\n \t\tcurl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);\n \t}\n \n-- \n1.7.7.6\n"},{"id":"185661","messageId":"20120228193125.GA11725@sigill.intra.peff.net","threadId":"29772","inReplyTo":"4F4CCEFD.90402@seap.minhap.es","subject":"Re: [PATCH 3/3] http: when proxy url has username but no password, ask for password","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-02-28T19:31:25Z","receivedAt":"2012-02-28T19:31:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 28, 2012 at 01:56:29PM +0100, Nelson Benitez Leon wrote:\n\n> diff --git a/http.c b/http.c\n> index 79cbe50..68e3f7d 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -306,7 +306,41 @@ static CURL *get_curl_handle(void)\n>  \t\t}\n>  \t}\n>  \tif (curl_http_proxy) {\n> -\t\tcurl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);\n> +\t\tchar *at, *colon, *proxyuser;\n> +\t\tconst char *cp;\n> +\t\tcp = strstr(curl_http_proxy, \"://\");\n> +\t\tif (cp == NULL) {\n> +\t\t\tcp = curl_http_proxy;\n> +\t\t} else {\n> +\t\t\tcp += 3;\n> +\t\t}\n> +\t\tat = strchr(cp, '@');\n> +\t\tcolon = strchr(cp, ':');\n> +\t\tif (at && (!colon || at < colon)) {\n> +\t\t\t/* proxy string has username but no password, ask for password */\n\nDon't parse the URL by hand. Use credential_from_url, which will do it\nfor you (and will properly handle things like unquoting the various\ncomponents).\n\n> +\t\t\tchar *ask_str, *proxyuser, *proxypass;\n\nShouldn't these be static globals? If we have multiple curl handles, you\nwould want them to share the authentication information we collect here,\nand not have to ask the user again, no?\n\n> +\t\t\tstrbuf_addf(&pbuf, \"Enter password for proxy %s...\", at+1);\n> +\t\t\task_str = strbuf_detach(&pbuf, NULL);\n> +\t\t\tproxypass = xstrdup(git_getpass(ask_str));\n\nAnd this should be using credential_fill(), which will let it use\ncredential helpers to save passwords, give it the same type of prompt as\nelsewhere, etc.\n\nSee Documentation/technical/api-credential.txt, and see how regular http\nauth is handled for an example.\n\n-Peff\n"},{"id":"185718","messageId":"4F4E01F0.8050503@seap.minhap.es","threadId":"29772","inReplyTo":"20120228193125.GA11725@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] http: when proxy url has username but no password, ask for password","fromName":"Nelson Benitez Leon","fromEmail":"nelsonjesus.benitez@seap.minhap.es","sentAt":"2012-02-29T10:46:08Z","receivedAt":"2012-02-29T10:46:08Z","isPatch":true,"sender":{"key":"nelsonjesus.benitez@seap.minhap.es","avatar":null},"body":"On 02/28/2012 08:31 PM, Jeff King wrote:\n> On Tue, Feb 28, 2012 at 01:56:29PM +0100, Nelson Benitez Leon wrote:\n> \n>> diff --git a/http.c b/http.c\n>> index 79cbe50..68e3f7d 100644\n>> --- a/http.c\n>> +++ b/http.c\n>> @@ -306,7 +306,41 @@ static CURL *get_curl_handle(void)\n>>  \t\t}\n>>  \t}\n>>  \tif (curl_http_proxy) {\n>> -\t\tcurl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);\n>> +\t\tchar *at, *colon, *proxyuser;\n>> +\t\tconst char *cp;\n>> +\t\tcp = strstr(curl_http_proxy, \"://\");\n>> +\t\tif (cp == NULL) {\n>> +\t\t\tcp = curl_http_proxy;\n>> +\t\t} else {\n>> +\t\t\tcp += 3;\n>> +\t\t}\n>> +\t\tat = strchr(cp, '@');\n>> +\t\tcolon = strchr(cp, ':');\n>> +\t\tif (at && (!colon || at < colon)) {\n>> +\t\t\t/* proxy string has username but no password, ask for password */\n> \n> Don't parse the URL by hand. Use credential_from_url, which will do it\n> for you (and will properly handle things like unquoting the various\n> components).\n\nWill do that\n\n>> +\t\t\tchar *ask_str, *proxyuser, *proxypass;\n> \n> Shouldn't these be static globals? If we have multiple curl handles, you\n> would want them to share the authentication information we collect here,\n> and not have to ask the user again, no?\n\nI didn't think about multiple curl handles, will look at make those static..\n\n>> +\t\t\tstrbuf_addf(&pbuf, \"Enter password for proxy %s...\", at+1);\n>> +\t\t\task_str = strbuf_detach(&pbuf, NULL);\n>> +\t\t\tproxypass = xstrdup(git_getpass(ask_str));\n> \n> And this should be using credential_fill(), which will let it use\n> credential helpers to save passwords, give it the same type of prompt as\n> elsewhere, etc.\n> \n> See Documentation/technical/api-credential.txt, and see how regular http\n> auth is handled for an example.\n\nI will try the credential api, I did my patch based on the fedora 16 git\nversion, which didn't have the credential api (I couldn't git clone at\nthat moment for the proxy problem so I had to use the source rpm from\nfedora).\n"}]}