{"thread":{"id":"30409","subject":"[PATCH 3/6] http: fix http_proxy specified without protocol part","startedAt":"2012-05-03T16:40:06Z","lastAt":"2012-05-04T07:22:20Z","messageCount":2,"participants":["Nelson Benitez Leon","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"190647","messageId":"4FA2B4E6.3080009@seap.minhap.es","threadId":"30409","inReplyTo":null,"subject":"[PATCH 3/6] http: fix http_proxy specified without protocol part","fromName":"Nelson Benitez Leon","fromEmail":"nelsonjesus.benitez@seap.minhap.es","sentAt":"2012-05-03T16:40:06Z","receivedAt":"2012-05-03T16:40:06Z","isPatch":true,"sender":{"key":"nelsonjesus.benitez@seap.minhap.es","avatar":null},"body":"An earlier patch broke http_proxy specified as : by abusing\ncredential_from_url(). Teach the function to parse that format, but the\ncaller needs to be updated to handle the case where there is no protocol\nin the parse result.\n\nAlso allow keyring implementations to differentiate authentication\nmaterial meant for http proxies and http destinations by using a\ndifferent token \"http-proxy\" to consult them for the former.\n\nSigned-off-by: Junio C Hamano \n---\n credential.c | 12 ++++++++----\n http.c | 13 ++++++++-----\n 2 files changed, 16 insertions(+), 9 deletions(-)\n\ndiff --git a/credential.c b/credential.c\nindex 62d1c56..813e3cf 100644\n--- a/credential.c\n+++ b/credential.c\n@@ -324,11 +324,15 @@ void credential_from_url(struct credential *c, const char *url)\n \t * (1) proto:///...\n \t * (2) proto://@/...\n \t * (3) proto://:@/...\n+\t * or \"proto://\"-less variants of the above. They are not technically\n+\t * URLs, but the caller may have some context-specific knowledge about\n+\t * what protocol is in use.\n \t */\n \tproto_end = strstr(url, \"://\");\n-\tif (!proto_end)\n-\t\treturn;\n-\tcp = proto_end + 3;\n+\tif (proto_end)\n+\t\tcp = proto_end + 3;\n+\telse\n+\t\tcp = url;\n \tat = strchr(cp, '@');\n \tcolon = strchr(cp, ':');\n \tslash = strchrnul(cp, '/');\n@@ -348,7 +352,7 @@ void credential_from_url(struct credential *c, const char *url)\n \t\thost = at + 1;\n \t}\n \n-\tif (proto_end - url > 0)\n+\tif (proto_end && proto_end != url)\n \t\tc->protocol = xmemdupz(url, proto_end - url);\n \tif (slash - host > 0)\n \t\tc->host = url_decode_mem(host, slash - host);\ndiff --git a/http.c b/http.c\nindex 02f9fcd..22ffe0c 100644\n--- a/http.c\n+++ b/http.c\n@@ -366,17 +366,20 @@ static CURL *get_curl_handle(const char *url)\n \t}\n \t\n \tif (curl_http_proxy) {\n-\t\tstruct strbuf proxyhost = STRBUF_INIT;\n-\n-\t\tif (!proxy_auth.host) /* check to parse only once */\n+\t\tif (!proxy_auth.host) {\n \t\t\tcredential_from_url(&proxy_auth, curl_http_proxy);\n+\t\t\tif (!proxy_auth.protocol ||\n+\t\t\t !strcmp(proxy_auth.protocol, \"http\")) {\n+\t\t\t\tfree(proxy_auth.protocol);\n+\t\t\t\tproxy_auth.protocol = xstrdup(\"http-proxy\");\n+\t\t\t}\n+\t\t}\n \n \t\tif (http_proactive_auth && proxy_auth.username && !proxy_auth.password)\n \t\t\t/* proxy string has username but no password, ask for password */\n \t\t\tcredential_fill(&proxy_auth);\n \n-\t\tstrbuf_addf(&proxyhost, \"%s://%s\", proxy_auth.protocol, proxy_auth.host);\n-\t\tcurl_easy_setopt(result, CURLOPT_PROXY, strbuf_detach(&proxyhost, NULL));\n+\t\tcurl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);\n \t\tcurl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);\n \t\tset_proxy_auth(result);\n \t}\n-- \n1.7.7.6\n"},{"id":"190729","messageId":"20120504072220.GC21895@sigill.intra.peff.net","threadId":"30409","inReplyTo":"4FA2B4E6.3080009@seap.minhap.es","subject":"Re: [PATCH 3/6] http: fix http_proxy specified without protocol part","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-04T07:22:20Z","receivedAt":"2012-05-04T07:22:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 03, 2012 at 06:40:06PM +0200, Nelson Benitez Leon wrote:\n\n> An earlier patch broke http_proxy specified as : by abusing\n> credential_from_url(). Teach the function to parse that format, but the\n> caller needs to be updated to handle the case where there is no protocol\n> in the parse result.\n> \n> Also allow keyring implementations to differentiate authentication\n> material meant for http proxies and http destinations by using a\n> different token \"http-proxy\" to consult them for the former.\n> \n> Signed-off-by: Junio C Hamano \n\nShould this be:\n\n From: Junio C Hamano \n\n?\n\nAlso, any time we read \"an earlier patch broke...\" and that earlier\npatch is in this series, I have to wonder why the patches are not simply\nreordered. Shouldn't the first half of this one come first, and just\nexplain the rationale as \"teach credential_from_url to handle\nprotocol-less URLs, since those are used for proxy specifications, which\nwe will need to parse in a later patch\".\n\n> diff --git a/http.c b/http.c\n> index 02f9fcd..22ffe0c 100644\n> --- a/http.c\n> +++ b/http.c\n> @@ -366,17 +366,20 @@ static CURL *get_curl_handle(const char *url)\n> \t}\n> \t\n> \tif (curl_http_proxy) {\n> -\t\tstruct strbuf proxyhost = STRBUF_INIT;\n> -\n> -\t\tif (!proxy_auth.host) /* check to parse only once */\n> +\t\tif (!proxy_auth.host) {\n> \t\t\tcredential_from_url(&proxy_auth, curl_http_proxy);\n> +\t\t\tif (!proxy_auth.protocol ||\n> +\t\t\t !strcmp(proxy_auth.protocol, \"http\")) {\n> +\t\t\t\tfree(proxy_auth.protocol);\n> +\t\t\t\tproxy_auth.protocol = xstrdup(\"http-proxy\");\n> +\t\t\t}\n> +\t\t}\n\nAnd then this hunk would just get squashed in in the first place.\n\n> \t\tif (http_proactive_auth && proxy_auth.username && !proxy_auth.password)\n> \t\t\t/* proxy string has username but no password, ask for password */\n> \t\t\tcredential_fill(&proxy_auth);\n> \n> -\t\tstrbuf_addf(&proxyhost, \"%s://%s\", proxy_auth.protocol, proxy_auth.host);\n> -\t\tcurl_easy_setopt(result, CURLOPT_PROXY, strbuf_detach(&proxyhost, NULL));\n> +\t\tcurl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);\n\nAnd this too, which gets rid of my complaints about the previous patch.\n\n-Peff\n"}]}