{"thread":{"id":"47347","subject":"imap-send with gmail: curl_easy_perform() failed: URL using bad/illegal format or missing URL","startedAt":"2017-11-29T17:13:12Z","lastAt":"2017-12-18T22:32:30Z","messageCount":17,"participants":["Doron Behar","Jonathan Nieder","Jeff King","Nicolas Morey-Chaisemartin","Daniel Stenberg","Eric Sunshine","Junio C Hamano","Kaartic Sivaraam"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"333790","messageId":"20171129171301.l3coiflkfyy533yz@NUC.localdomain","threadId":"47347","inReplyTo":null,"subject":"imap-send with gmail: curl_easy_perform() failed: URL using bad/illegal format or missing URL","fromName":"Doron Behar","fromEmail":"doron.behar@gmail.com","sentAt":"2017-11-29T17:13:01Z","receivedAt":"2017-11-29T17:13:12Z","isPatch":false,"sender":{"key":"doron.behar@gmail.com","avatar":null},"body":"Hi,\n\nI'm trying to send a patch with the command `git imap-send`, I used the\nexamples in the manual page as the main reference for my configuration:\n\n```\n[imap]\n\tfolder = \"[Gmail]/Drafts\"\n\thost = imaps://imap.gmail.com\n\tuser = doron.behar@gmail.com\n\tport = 993\n\tsslverify = false\n```\n\nThis is my `cat patch.out | git imap-send` output:\n\n```\nPassword for 'imaps://doron.behar@gmail.com@imap.gmail.com':\nsending 3 messages\ncurl_easy_perform() failed: URL using bad/illegal format or missing URL\n```\n\nThe URI doesn't seem OK to me, I tried using `imap.user = doron.behar` and the\nURI was `imaps://doron.behar@imap.gmail.com` but that ended up with the same\nerror as in the previous case.\n\nI would love to get some help here, a Google Search didn't help as well.\n\nThanks.\n"},{"id":"333829","messageId":"20171130020445.GF15098@aiede.mtv.corp.google.com","threadId":"47347","inReplyTo":"20171129171301.l3coiflkfyy533yz@NUC.localdomain","subject":"Re: imap-send with gmail: curl_easy_perform() failed: URL using bad/illegal format or missing URL","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-11-30T02:04:45Z","receivedAt":"2017-11-30T02:04:56Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"(+cc: Nicolas)\nHi,\n\nDoron Behar wrote:\n\n> I'm trying to send a patch with the command `git imap-send`, I used the\n> examples in the manual page as the main reference for my configuration:\n>\n> ```\n> [imap]\n> \tfolder = \"[Gmail]/Drafts\"\n> \thost = imaps://imap.gmail.com\n> \tuser = doron.behar@gmail.com\n> \tport = 993\n> \tsslverify = false\n> ```\n>\n> This is my `cat patch.out | git imap-send` output:\n>\n> ```\n> Password for 'imaps://doron.behar@gmail.com@imap.gmail.com':\n> sending 3 messages\n> curl_easy_perform() failed: URL using bad/illegal format or missing URL\n> ```\n\nThanks for reporting this. I suspect this is related to\nv2.15.0-rc0~63^2 (imap-send: use curl by default when possible,\n2017-09-14) --- e.g. perhaps our custom IMAP code was doing some\nescaping on the username that libcurl does not do.\n\n\"man git imap-send\" says this is a recommended configuration, so I\ndon't think it's a configuration error.\n\nWhat platform are you on? What version of libcurl are you using?\n\nIn libcurl::lib/easy.c I am also seeing\n\n if(mcode)\n return CURLE_URL_MALFORMAT; /* TODO: return a proper error! */\n\nwhich looks suspicious.\n\nNicolas, am I on the right track?\n\nThanks,\nJonathan\n\n> The URI doesn't seem OK to me, I tried using `imap.user = doron.behar` and the\n> URI was `imaps://doron.behar@imap.gmail.com` but that ended up with the same\n> error as in the previous case.\n>\n> I would love to get some help here, a Google Search didn't help as well.\n>\n> Thanks.\n"},{"id":"333833","messageId":"20171130032832.GA25577@sigill.intra.peff.net","threadId":"47347","inReplyTo":"20171130020445.GF15098@aiede.mtv.corp.google.com","subject":"Re: imap-send with gmail: curl_easy_perform() failed: URL using bad/illegal format or missing URL","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-30T03:28:32Z","receivedAt":"2017-11-30T03:28:39Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 29, 2017 at 06:04:45PM -0800, Jonathan Nieder wrote:\n\n> > Password for 'imaps://doron.behar@gmail.com@imap.gmail.com':\n> > sending 3 messages\n> > curl_easy_perform() failed: URL using bad/illegal format or missing URL\n> > ```\n> \n> Thanks for reporting this. I suspect this is related to\n> v2.15.0-rc0~63^2 (imap-send: use curl by default when possible,\n> 2017-09-14) --- e.g. perhaps our custom IMAP code was doing some\n> escaping on the username that libcurl does not do.\n> \n> \"man git imap-send\" says this is a recommended configuration, so I\n> don't think it's a configuration error.\n> \n> What platform are you on? What version of libcurl are you using?\n\nAll good thoughts/questions. I have two suggestions to add:\n\n 1. As an immediate work-around, running \"imap-send --no-curl\" may work. \n That will at least get this case working while we debug.\n\n 2. Setting GIT_TRACE_CURL=1 may dump more verbose information. But one\n caveat: if you get as far as authenticating, then the trace will\n contain your password. We redact HTTP auth from the trace output,\n but not imap ones.\n\n-Peff\n"},{"id":"333834","messageId":"20171130035527.GB25577@sigill.intra.peff.net","threadId":"47347","inReplyTo":"20171130032832.GA25577@sigill.intra.peff.net","subject":"Re: imap-send with gmail: curl_easy_perform() failed: URL using bad/illegal format or missing URL","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-30T03:55:27Z","receivedAt":"2017-11-30T03:55:35Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 29, 2017 at 10:28:32PM -0500, Jeff King wrote:\n\n> 2. Setting GIT_TRACE_CURL=1 may dump more verbose information. But one\n> caveat: if you get as far as authenticating, then the trace will\n> contain your password. We redact HTTP auth from the trace output,\n> but not imap ones.\n\nI tried my hand at a patch, but it ended up a lot more complicated than\nI would have liked. Thoughts?\n\ndiff --git a/http.c b/http.c\nindex 713525f38e..fcc9001af6 100644\n--- a/http.c\n+++ b/http.c\n@@ -560,7 +560,7 @@ static void set_curl_keepalive(CURL *c)\n }\n #endif\n \n-static void redact_sensitive_header(struct strbuf *header)\n+static void redact_http_header(struct strbuf *header)\n {\n \tconst char *sensitive_header;\n \n@@ -577,7 +577,60 @@ static void redact_sensitive_header(struct strbuf *header)\n \t}\n }\n \n-static void curl_dump_header(const char *text, unsigned char *ptr, size_t size, int hide_sensitive_header)\n+static void redact_imap_header(struct strbuf *header)\n+{\n+\tconst char *p;\n+\n+\t/* skip past the command tag */\n+\tp = strchr(header->buf, ' ');\n+\tif (!p)\n+\t\treturn; /* no tag */\n+\tp++;\n+\n+\tif (skip_prefix(p, \"AUTHENTICATE \", &p)) {\n+\t\t/* the first token is the auth type, which is OK to log */\n+\t\twhile (*p && !isspace(*p))\n+\t\t\tp++;\n+\t\t/* the rest is an opaque blob; fall through to redact */\n+\t} else if (skip_prefix(p, \"LOGIN \", &p)) {\n+\t\t/* fall through to redact both login and password */\n+\t} else {\n+\t\t/* not a sensitive header */\n+\t\treturn;\n+\t}\n+\n+\tstrbuf_setlen(header, p - header->buf);\n+\tstrbuf_addstr(header, \" \");\n+}\n+\n+static void redact_sensitive_header(CURL *handle, struct strbuf *header)\n+{\n+\tconst char *url;\n+\tint ret;\n+\n+\tret = curl_easy_getinfo(handle, CURLINFO_EFFECTIVE_URL, &url);\n+\tif (!ret && url) {\n+\t\tif (starts_with(url, \"http\")) {\n+\t\t\tredact_http_header(header);\n+\t\t\treturn;\n+\t\t}\n+\t\tif (starts_with(url, \"imap\")) {\n+\t\t\tredact_imap_header(header);\n+\t\t\treturn;\n+\t\t}\n+\t}\n+\n+\t/*\n+\t * We weren't able to figure out the protocol. Err on the side of\n+\t * redacting too much.\n+\t */\n+\tredact_http_header(header);\n+\tredact_imap_header(header);\n+}\n+\n+static void curl_dump_header(CURL *handle, const char *text,\n+\t\t\t unsigned char *ptr, size_t size,\n+\t\t\t int hide_sensitive_header)\n {\n \tstruct strbuf out = STRBUF_INIT;\n \tstruct strbuf **headers, **header;\n@@ -591,7 +644,7 @@ static void curl_dump_header(const char *text, unsigned char *ptr, size_t size,\n \n \tfor (header = headers; *header; header++) {\n \t\tif (hide_sensitive_header)\n-\t\t\tredact_sensitive_header(*header);\n+\t\t\tredact_sensitive_header(handle, *header);\n \t\tstrbuf_insert((*header), 0, text, strlen(text));\n \t\tstrbuf_insert((*header), strlen(text), \": \", 2);\n \t\tstrbuf_rtrim((*header));\n@@ -641,7 +694,7 @@ static int curl_trace(CURL *handle, curl_infotype type, char *data, size_t size,\n \t\tbreak;\n \tcase CURLINFO_HEADER_OUT:\n \t\ttext = \"=> Send header\";\n-\t\tcurl_dump_header(text, (unsigned char *)data, size, DO_FILTER);\n+\t\tcurl_dump_header(handle, text, (unsigned char *)data, size, DO_FILTER);\n \t\tbreak;\n \tcase CURLINFO_DATA_OUT:\n \t\ttext = \"=> Send data\";\n@@ -653,7 +706,7 @@ static int curl_trace(CURL *handle, curl_infotype type, char *data, size_t size,\n \t\tbreak;\n \tcase CURLINFO_HEADER_IN:\n \t\ttext = \"<= Recv header\";\n-\t\tcurl_dump_header(text, (unsigned char *)data, size, NO_FILTER);\n+\t\tcurl_dump_header(handle, text, (unsigned char *)data, size, NO_FILTER);\n \t\tbreak;\n \tcase CURLINFO_DATA_IN:\n \t\ttext = \"<= Recv data\";\n"},{"id":"333840","messageId":"50fb321e-bbfc-adae-992d-eea1b818171e@suse.de","threadId":"47347","inReplyTo":"20171130020445.GF15098@aiede.mtv.corp.google.com","subject":"Re: imap-send with gmail: curl_easy_perform() failed: URL using bad/illegal format or missing URL","fromName":"Nicolas Morey-Chaisemartin","fromEmail":"nmoreychaisemartin@suse.de","sentAt":"2017-11-30T09:39:46Z","receivedAt":"2017-11-30T09:39:53Z","isPatch":false,"sender":{"key":"nmoreychaisemartin@suse.de","avatar":"https://gravatar.com/avatar/5546322ccb9067f56b6939d9d5c758a40cab5b978b1379fd6ec8ab9b8a6a12b1?d=mp&s=160"},"body":"\n\nLe 30/11/2017 à 03:04, Jonathan Nieder a écrit :\n> (+cc: Nicolas)\n> Hi,\n>\n> Doron Behar wrote:\n>\n>> I'm trying to send a patch with the command `git imap-send`, I used the\n>> examples in the manual page as the main reference for my configuration:\n>>\n>> ```\n>> [imap]\n>> \tfolder = \"[Gmail]/Drafts\"\n>> \thost = imaps://imap.gmail.com\n>> \tuser = doron.behar@gmail.com\n>> \tport = 993\n>> \tsslverify = false\n>> ```\n>>\n>> This is my `cat patch.out | git imap-send` output:\n>>\n>> ```\n>> Password for 'imaps://doron.behar@gmail.com@imap.gmail.com':\n>> sending 3 messages\n>> curl_easy_perform() failed: URL using bad/illegal format or missing URL\n>> ```\n> Thanks for reporting this. I suspect this is related to\n> v2.15.0-rc0~63^2 (imap-send: use curl by default when possible,\n> 2017-09-14) --- e.g. perhaps our custom IMAP code was doing some\n> escaping on the username that libcurl does not do.\n>\n> \"man git imap-send\" says this is a recommended configuration, so I\n> don't think it's a configuration error.\n>\n> What platform are you on? What version of libcurl are you using?\n>\n> In libcurl::lib/easy.c I am also seeing\n>\n> if(mcode)\n> return CURLE_URL_MALFORMAT; /* TODO: return a proper error! */\n>\n> which looks suspicious.\n>\n> Nicolas, am I on the right track?\n>\n> Thanks,\n> Jonathan\n>\n\nThis is due to the weird \"[Gmail]\" prefix in the folder.\nI tried manually replacing it with:\n        folder = %5BGmail%5D/Drafts\nin .git/config and it works.\n\ncurl is doing some fancy handling with brackets and braces. It make sense for multiple FTP downloads like ftp://ftp.numericals.com/file[1-100].txt, not in our case.\nThe curl command line has a --globoff argument to disable this \"regexp\" support and it seems to fix the gmail case.\nHowever I couldn't find a way to change this value through the API...\n\nI guess we should open a bug upstream to get access to this setting through the API and add a patch that HTTP encode brackets and braces in the meantime.\n\nNicolas\n\n"},{"id":"333842","messageId":"6b1b1c11-cf17-35e8-1d33-42102d9526ea@suse.de","threadId":"47347","inReplyTo":"50fb321e-bbfc-adae-992d-eea1b818171e@suse.de","subject":"Re: imap-send with gmail: curl_easy_perform() failed: URL using bad/illegal format or missing URL","fromName":"Nicolas Morey-Chaisemartin","fromEmail":"nmoreychaisemartin@suse.de","sentAt":"2017-11-30T09:47:22Z","receivedAt":"2017-11-30T09:47:30Z","isPatch":false,"sender":{"key":"nmoreychaisemartin@suse.de","avatar":"https://gravatar.com/avatar/5546322ccb9067f56b6939d9d5c758a40cab5b978b1379fd6ec8ab9b8a6a12b1?d=mp&s=160"},"body":"\n\nLe 30/11/2017 à 10:39, Nicolas Morey-Chaisemartin a écrit :\n>\n> Le 30/11/2017 à 03:04, Jonathan Nieder a écrit :\n>> (+cc: Nicolas)\n>> Hi,\n>>\n>> Doron Behar wrote:\n>>\n>>> I'm trying to send a patch with the command `git imap-send`, I used the\n>>> examples in the manual page as the main reference for my configuration:\n>>>\n>>> ```\n>>> [imap]\n>>> \tfolder = \"[Gmail]/Drafts\"\n>>> \thost = imaps://imap.gmail.com\n>>> \tuser = doron.behar@gmail.com\n>>> \tport = 993\n>>> \tsslverify = false\n>>> ```\n>>>\n>>> This is my `cat patch.out | git imap-send` output:\n>>>\n>>> ```\n>>> Password for 'imaps://doron.behar@gmail.com@imap.gmail.com':\n>>> sending 3 messages\n>>> curl_easy_perform() failed: URL using bad/illegal format or missing URL\n>>> ```\n>> Thanks for reporting this. I suspect this is related to\n>> v2.15.0-rc0~63^2 (imap-send: use curl by default when possible,\n>> 2017-09-14) --- e.g. perhaps our custom IMAP code was doing some\n>> escaping on the username that libcurl does not do.\n>>\n>> \"man git imap-send\" says this is a recommended configuration, so I\n>> don't think it's a configuration error.\n>>\n>> What platform are you on? What version of libcurl are you using?\n>>\n>> In libcurl::lib/easy.c I am also seeing\n>>\n>> if(mcode)\n>> return CURLE_URL_MALFORMAT; /* TODO: return a proper error! */\n>>\n>> which looks suspicious.\n>>\n>> Nicolas, am I on the right track?\n>>\n>> Thanks,\n>> Jonathan\n>>\n> This is due to the weird \"[Gmail]\" prefix in the folder.\n> I tried manually replacing it with:\n>         folder = %5BGmail%5D/Drafts\n> in .git/config and it works.\n>\n> curl is doing some fancy handling with brackets and braces. It make sense for multiple FTP downloads like ftp://ftp.numericals.com/file[1-100].txt, not in our case.\n> The curl command line has a --globoff argument to disable this \"regexp\" support and it seems to fix the gmail case.\n\nIn fact no, StackOverflow was wrong :)\n\n> However I couldn't find a way to change this value through the API...\n>\n> I guess we should open a bug upstream to get access to this setting through the API and add a patch that HTTP encode brackets and braces in the meantime.\n>\nThis means with have to URI encode the folder. DO we have a helper for that ?\n\nNicolas\n"},{"id":"333843","messageId":"alpine.DEB.2.20.1711301041250.30591@tvnag.unkk.fr","threadId":"47347","inReplyTo":"50fb321e-bbfc-adae-992d-eea1b818171e@suse.de","subject":"Re: imap-send with gmail: curl_easy_perform() failed: URL using bad/illegal format or missing URL","fromName":"Daniel Stenberg","fromEmail":"daniel@haxx.se","sentAt":"2017-11-30T09:46:47Z","receivedAt":"2017-11-30T09:55:16Z","isPatch":false,"sender":{"key":"daniel@haxx.se","avatar":"https://gravatar.com/avatar/69fdca87edd17cee21ca2e79fc2ff671d644603c3dc27167430f3cd3dbab7ba8?d=mp&s=160"},"body":"On Thu, 30 Nov 2017, Nicolas Morey-Chaisemartin wrote:\n\n> This is due to the weird \"[Gmail]\" prefix in the folder.\n> I tried manually replacing it with:\n>         folder = %5BGmail%5D/Drafts\n> in .git/config and it works.\n>\n> curl is doing some fancy handling with brackets and braces. It make sense \n> for multiple FTP downloads like ftp://ftp.numericals.com/file[1-100].txt, \n> not in our case. The curl command line has a --globoff argument to disable \n> this \"regexp\" support and it seems to fix the gmail case. However I couldn't \n> find a way to change this value through the API...\n\nThat's just a feature of the command line tool, \"globbing\" isn't a function \nprovided by the library. libcurl actually \"just\" expects a plain old URL.\n\nBut with the risk of falling through the cracks into the rathole that is \"what \nis a URL\" (I've blogged about the topic several times in the past and I will \nsurely do it again in the future):\n\nA \"legal\" URL (as per RFC 3986) does not contain brackets, such symbols should \nbe used URL encoded: %5B and %5D.\n\nThis said: I don't know exactly why brackets cause a problem in this case. It \ncould still be worth digging into and see if libcurl could deal with them \nbetter here...\n\n-- \n\n / daniel.haxx.se"},{"id":"333844","messageId":"alpine.DEB.2.20.1711301052350.30591@tvnag.unkk.fr","threadId":"47347","inReplyTo":"44bbb765-fb0d-5b80-c697-76a9b6ff0811@morey-chaisemartin.com","subject":"Re: imap-send with gmail: curl_easy_perform() failed: URL using bad/illegal format or missing URL","fromName":"Daniel Stenberg","fromEmail":"daniel@haxx.se","sentAt":"2017-11-30T09:55:07Z","receivedAt":"2017-11-30T09:55:24Z","isPatch":false,"sender":{"key":"daniel@haxx.se","avatar":"https://gravatar.com/avatar/69fdca87edd17cee21ca2e79fc2ff671d644603c3dc27167430f3cd3dbab7ba8?d=mp&s=160"},"body":"On Thu, 30 Nov 2017, Nicolas Morey-Chaisemartin wrote:\n\n> It would make sense to have a way to ask libcurl to URI encode for us. I'm \n> guessing there's already the code for that somewhere in curl and we would be \n> wise to use it. But to work wqith older version we'll have to do it \n> ourselves anyway.\n\nlibcurl only offers curl_easy_escape() which URL encodes a string.\n\nBut that's not really usably on an entire existing URL or path since it would \nthen also encode the slashes etc. You want to encode the relevant pieces and \nthen put them together appropriately into the final URL...\n\n-- \n\n / daniel.haxx.se\n"},{"id":"333845","messageId":"44bbb765-fb0d-5b80-c697-76a9b6ff0811@morey-chaisemartin.com","threadId":"47347","inReplyTo":"alpine.DEB.2.20.1711301041250.30591@tvnag.unkk.fr","subject":"Re: imap-send with gmail: curl_easy_perform() failed: URL using bad/illegal format or missing URL","fromName":"Nicolas Morey-Chaisemartin","fromEmail":"nicolas@morey-chaisemartin.com","sentAt":"2017-11-30T09:51:01Z","receivedAt":"2017-11-30T10:00:44Z","isPatch":false,"sender":{"key":"devel-git@morey-chaisemartin.com","avatar":"https://avatars.githubusercontent.com/u/108326?v=4"},"body":"\n\nLe 30/11/2017 à 10:46, Daniel Stenberg a écrit :\n> On Thu, 30 Nov 2017, Nicolas Morey-Chaisemartin wrote:\n>\n>> This is due to the weird \"[Gmail]\" prefix in the folder.\n>> I tried manually replacing it with:\n>>         folder = %5BGmail%5D/Drafts\n>> in .git/config and it works.\n>>\n>> curl is doing some fancy handling with brackets and braces. It make sense for multiple FTP downloads like ftp://ftp.numericals.com/file[1-100].txt, not in our case. The curl command line has a --globoff argument to disable this \"regexp\" support and it seems to fix the gmail case. However I couldn't find a way to change this value through the API...\n>\n> That's just a feature of the command line tool, \"globbing\" isn't a function provided by the library. libcurl actually \"just\" expects a plain old URL.\n>\nYep that what I figured looking a bit further in the code.\n\n> But with the risk of falling through the cracks into the rathole that is \"what is a URL\" (I've blogged about the topic several times in the past and I will surely do it again in the future):\n>\n> A \"legal\" URL (as per RFC 3986) does not contain brackets, such symbols should be used URL encoded: %5B and %5D.\n>\n> This said: I don't know exactly why brackets cause a problem in this case. It could still be worth digging into and see if libcurl could deal with them better here...\n>\n\nIt would make sense to have a way to ask libcurl to URI encode for us. I'm guessing there's already the code for that somewhere in curl and we would be wise to use it.\nBut to work wqith older version we'll have to do it ourselves anyway.\n\nNicolas\n"},{"id":"333846","messageId":"18c9478b-19fc-69f2-229f-67c05a42d4f5@suse.com","threadId":"47347","inReplyTo":"20171129171301.l3coiflkfyy533yz@NUC.localdomain","subject":"[PATCH] imap-send: URI encode server folder","fromName":"Nicolas Morey-Chaisemartin","fromEmail":"nmoreychaisemartin@suse.com","sentAt":"2017-11-30T10:07:44Z","receivedAt":"2017-11-30T10:07:54Z","isPatch":true,"sender":{"key":"devel-git@morey-chaisemartin.com","avatar":"https://avatars.githubusercontent.com/u/108326?v=4"},"body":"URI encode the server folder string before passing it to libcurl.\nThis fixes the access to the draft folder on Gmail accounts (named [Gmail]/Drafts)\n\nReported-by: Doron Behar \nSigned-off-by: Nicolas Morey-Chaisemartin \n---\n imap-send.c | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex 54e6a80fd..36c7c1b4f 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -1412,6 +1412,7 @@ static CURL *setup_curl(struct imap_server_conf *srvc, struct credential *cred)\n {\n \tCURL *curl;\n \tstruct strbuf path = STRBUF_INIT;\n+\tchar *uri_encoded_folder;\n \n \tif (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)\n \t\tdie(\"curl_global_init failed\");\n@@ -1429,7 +1430,12 @@ static CURL *setup_curl(struct imap_server_conf *srvc, struct credential *cred)\n \tstrbuf_addstr(&path, server.host);\n \tif (!path.len || path.buf[path.len - 1] != '/')\n \t\tstrbuf_addch(&path, '/');\n-\tstrbuf_addstr(&path, server.folder);\n+\n+\turi_encoded_folder = curl_easy_escape(curl, server.folder, 0);\n+\tif (!uri_encoded_folder)\n+\t\tdie(\"failed to encode server folder\");\n+\tstrbuf_addstr(&path, uri_encoded_folder);\n+\tcurl_free(uri_encoded_folder);\n \n \tcurl_easy_setopt(curl, CURLOPT_URL, path.buf);\n \tstrbuf_release(&path);\n-- \n2.15.1.272.g8e603414b\n\n"},{"id":"333861","messageId":"CAPig+cSEYHfupRt+-0sZK6H3_WcT_=wdNB2FfdYuB6geGQYZ3A@mail.gmail.com","threadId":"47347","inReplyTo":"18c9478b-19fc-69f2-229f-67c05a42d4f5@suse.com","subject":"Re: [PATCH] imap-send: URI encode server folder","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2017-11-30T17:53:16Z","receivedAt":"2017-11-30T17:53:22Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Nov 30, 2017 at 5:07 AM, Nicolas Morey-Chaisemartin\n wrote:\n> URI encode the server folder string before passing it to libcurl.\n> This fixes the access to the draft folder on Gmail accounts (named [Gmail]/Drafts)\n\nFor someone reading this commit message in the future -- someone who\ndidn't follow the email thread which led to this patch -- \"this fixes\"\ndoesn't say much about the actual problem being addressed. Can you\nexpand the commit message a bit to make it more self-contained? At\nminimum, perhaps show the error message you were experiencing, and\ncite (as Daniel pointed out) RFC 3986 and the bit about a \"legal\" URL\nnot containing brackets.\n\nAlso, a natural question which pops into the head of someone reading\nthis patch is whether other parts of the URL (host, user, etc.) also\nneed to be handled similarly. It's possible that you audited the code\nand determined that they are handled fine already, but the reader of\nthe commit message is unable to infer that. Consequently, it might be\nnice to have a sentence about that, as well (\"other parts of the URL\nare already encoded, thus are fine\" or \"other parts of the URL are not\nsubject to this problem because ...\").\n\nThe patch itself looks okay (from a cursory read).\n\nThanks.\n\n> Reported-by: Doron Behar \n> Signed-off-by: Nicolas Morey-Chaisemartin \n> ---\n> imap-send.c | 8 +++++++-\n> 1 file changed, 7 insertions(+), 1 deletion(-)\n>\n> diff --git a/imap-send.c b/imap-send.c\n> index 54e6a80fd..36c7c1b4f 100644\n> --- a/imap-send.c\n> +++ b/imap-send.c\n> @@ -1412,6 +1412,7 @@ static CURL *setup_curl(struct imap_server_conf *srvc, struct credential *cred)\n> {\n> CURL *curl;\n> struct strbuf path = STRBUF_INIT;\n> + char *uri_encoded_folder;\n>\n> if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)\n> die(\"curl_global_init failed\");\n> @@ -1429,7 +1430,12 @@ static CURL *setup_curl(struct imap_server_conf *srvc, struct credential *cred)\n> strbuf_addstr(&path, server.host);\n> if (!path.len || path.buf[path.len - 1] != '/')\n> strbuf_addch(&path, '/');\n> - strbuf_addstr(&path, server.folder);\n> +\n> + uri_encoded_folder = curl_easy_escape(curl, server.folder, 0);\n> + if (!uri_encoded_folder)\n> + die(\"failed to encode server folder\");\n> + strbuf_addstr(&path, uri_encoded_folder);\n> + curl_free(uri_encoded_folder);\n>\n> curl_easy_setopt(curl, CURLOPT_URL, path.buf);\n> strbuf_release(&path);\n> --\n> 2.15.1.272.g8e603414b\n"},{"id":"334156","messageId":"xmqqd13tjioz.fsf@gitster.mtv.corp.google.com","threadId":"47347","inReplyTo":"CAPig+cSEYHfupRt+-0sZK6H3_WcT_=wdNB2FfdYuB6geGQYZ3A@mail.gmail.com","subject":"Re: [PATCH] imap-send: URI encode server folder","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-05T15:30:36Z","receivedAt":"2017-12-05T15:30:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> ... Can you\n> expand the commit message a bit to make it more self-contained? At\n> minimum, perhaps show the error message you were experiencing, and\n> cite (as Daniel pointed out) RFC 3986 and the bit about a \"legal\" URL\n> not containing brackets.\n\nThanks for a good suggestion.\n\n\n>\n> Also, a natural question which pops into the head of someone reading\n> this patch is whether other parts of the URL (host, user, etc.) also\n> need to be handled similarly. It's possible that you audited the code\n> and determined that they are handled fine already, but the reader of\n> the commit message is unable to infer that. Consequently, it might be\n> nice to have a sentence about that, as well (\"other parts of the URL\n> are already encoded, thus are fine\" or \"other parts of the URL are not\n> subject to this problem because ...\").\n>\n> The patch itself looks okay (from a cursory read).\n>\n> Thanks.\n>\n>> Reported-by: Doron Behar \n>> Signed-off-by: Nicolas Morey-Chaisemartin \n>> ---\n>> imap-send.c | 8 +++++++-\n>> 1 file changed, 7 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/imap-send.c b/imap-send.c\n>> index 54e6a80fd..36c7c1b4f 100644\n>> --- a/imap-send.c\n>> +++ b/imap-send.c\n>> @@ -1412,6 +1412,7 @@ static CURL *setup_curl(struct imap_server_conf *srvc, struct credential *cred)\n>> {\n>> CURL *curl;\n>> struct strbuf path = STRBUF_INIT;\n>> + char *uri_encoded_folder;\n>>\n>> if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)\n>> die(\"curl_global_init failed\");\n>> @@ -1429,7 +1430,12 @@ static CURL *setup_curl(struct imap_server_conf *srvc, struct credential *cred)\n>> strbuf_addstr(&path, server.host);\n>> if (!path.len || path.buf[path.len - 1] != '/')\n>> strbuf_addch(&path, '/');\n>> - strbuf_addstr(&path, server.folder);\n>> +\n>> + uri_encoded_folder = curl_easy_escape(curl, server.folder, 0);\n>> + if (!uri_encoded_folder)\n>> + die(\"failed to encode server folder\");\n>> + strbuf_addstr(&path, uri_encoded_folder);\n>> + curl_free(uri_encoded_folder);\n>>\n>> curl_easy_setopt(curl, CURLOPT_URL, path.buf);\n>> strbuf_release(&path);\n>> --\n>> 2.15.1.272.g8e603414b\n"},{"id":"334979","messageId":"20171218182546.9962-1-kaartic.sivaraam@gmail.com","threadId":"47347","inReplyTo":"CAPig+cSEYHfupRt+-0sZK6H3_WcT_=wdNB2FfdYuB6geGQYZ3A@mail.gmail.com","subject":"[PATCH v2] imap-send: URI encode server folder","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2017-12-18T18:25:46Z","receivedAt":"2017-12-18T18:26:25Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"From: Nicolas Morey-Chaisemartin \n\nWhen trying to send a patch using 'imap-send' with 'curl' and the\nfollowing configuration:\n\n[imap]\n\tfolder = \"[Gmail]/Drafts\"\n\thost = imaps://imap.gmail.com\n\tport = 993\n\tsslverify = false\n\nresulted in the following error,\n\n curl_easy_perform() failed: URL using bad/illegal format or missing URL\n\nThat was a consequence of the not URI encoding the folder portion of\nthe URL which contained characters such as '[' which are not\nallowed in a URI. According to RFC3986, these characters should be\n\"URI encoded\".\n\nSo, URI encode the folder portion of the URL to ensure it doesn't\ncontain characters that aren't allowed in a URI.\n\nReported-by: Doron Behar \nSigned-off-by: Nicolas Morey-Chaisemartin \n---\n\nI came across the same issue that lead to this patch recently and found\nthat this patch didn't make it in. So, I thought I could help out and\nhence this v2.\n\nEric Sunshine writes:\n> For someone reading this commit message in the future -- someone who\n> didn't follow the email thread which led to this patch -- \"this fixes\"\n> doesn't say much about the actual problem being addressed. Can you\n> expand the commit message a bit to make it more self-contained? At\n> minimum, perhaps show the error message you were experiencing, and\n> cite (as Daniel pointed out) RFC 3986 and the bit about a \"legal\" URL\n> not containing brackets.\n\nI guess I covered this part.\n\n\n> Also, a natural question which pops into the head of someone reading\n> this patch is whether other parts of the URL (host, user, etc.) also\n> need to be handled similarly. It's possible that you audited the code\n> and determined that they are handled fine already, but the reader of\n> the commit message is unable to infer that. Consequently, it might be\n> nice to have a sentence about that, as well (\"other parts of the URL\n> are already encoded, thus are fine\" or \"other parts of the URL are not\n> subject to this problem because ...\").\n\nI'm not sure about this one. I guess the host and user don't need encoding\nas I suspect they wouldn't contain characters that aren't allowed. I might\nbe wrong, though. Let me know if I'm missing something.\n\n\nThanks,\nKaartic\n\n imap-send.c | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex 54e6a80fd..36c7c1b4f 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -1412,6 +1412,7 @@ static CURL *setup_curl(struct imap_server_conf *srvc, struct credential *cred)\n {\n \tCURL *curl;\n \tstruct strbuf path = STRBUF_INIT;\n+\tchar *uri_encoded_folder;\n \n \tif (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)\n \t\tdie(\"curl_global_init failed\");\n@@ -1429,7 +1430,12 @@ static CURL *setup_curl(struct imap_server_conf *srvc, struct credential *cred)\n \tstrbuf_addstr(&path, server.host);\n \tif (!path.len || path.buf[path.len - 1] != '/')\n \t\tstrbuf_addch(&path, '/');\n-\tstrbuf_addstr(&path, server.folder);\n+\n+\turi_encoded_folder = curl_easy_escape(curl, server.folder, 0);\n+\tif (!uri_encoded_folder)\n+\t\tdie(\"failed to encode server folder\");\n+\tstrbuf_addstr(&path, uri_encoded_folder);\n+\tcurl_free(uri_encoded_folder);\n \n \tcurl_easy_setopt(curl, CURLOPT_URL, path.buf);\n \tstrbuf_release(&path);\n-- \n2.15.1.620.gb9897f467\n\n"},{"id":"334984","messageId":"CAPig+cSOn8PV52aL0Ky5uUwFfNMWye93UQGioRiXjj0ze9HX+g@mail.gmail.com","threadId":"47347","inReplyTo":"20171218182546.9962-1-kaartic.sivaraam@gmail.com","subject":"Re: [PATCH v2] imap-send: URI encode server folder","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2017-12-18T18:48:35Z","receivedAt":"2017-12-18T18:48:42Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Dec 18, 2017 at 1:25 PM, Kaartic Sivaraam\n wrote:\n> From: Nicolas Morey-Chaisemartin \n> [...]\n> resulted in the following error,\n\ns/resulted/results/\n\n> curl_easy_perform() failed: URL using bad/illegal format or missing URL\n>\n> That was a consequence of the not URI encoding the folder portion of\n\ns/That was/This is/\ns/the not/not/\ns/URI encoding/URI-encoding/\n\n> the URL which contained characters such as '[' which are not\n\ns/contained/contains/\n\n> allowed in a URI. According to RFC3986, these characters should be\n> \"URI encoded\".\n>\n> So, URI encode the folder portion of the URL to ensure it doesn't\n> contain characters that aren't allowed in a URI.\n>\n> Reported-by: Doron Behar \n> Signed-off-by: Nicolas Morey-Chaisemartin \n> ---\n>\n> I came across the same issue that lead to this patch recently and found\n> that this patch didn't make it in. So, I thought I could help out and\n> hence this v2.\n\nThanks for taking up the slack.\n"},{"id":"334986","messageId":"20171218191113.18600-1-kaartic.sivaraam@gmail.com","threadId":"47347","inReplyTo":"CAPig+cSOn8PV52aL0Ky5uUwFfNMWye93UQGioRiXjj0ze9HX+g@mail.gmail.com","subject":"[PATCH v3] imap-send: URI encode server folder","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2017-12-18T19:11:13Z","receivedAt":"2017-12-18T19:11:40Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"From: Nicolas Morey-Chaisemartin \n\nWhen trying to send a patch using 'imap-send' with 'curl' and the\nfollowing configuration:\n\n[imap]\n\tfolder = \"[Gmail]/Drafts\"\n\thost = imaps://imap.gmail.com\n\tport = 993\n\tsslverify = false\n\nresults in the following error,\n\n curl_easy_perform() failed: URL using bad/illegal format or missing URL\n\nThis is a consequence of not URI-encoding the folder portion of\nthe URL which contains characters such as '[' which are not\nallowed in a URI. According to RFC3986, these characters should be\nURI-encoded.\n\nSo, URI-encode the folder before adding it to the URI to ensure it doesn't\ncontain characters that aren't allowed in a URI.\n\nReported-by: Doron Behar \nSigned-off-by: Nicolas Morey-Chaisemartin \nSigned-off-by: Kaartic Sivaraam \n---\nChanges in v3:\n - updated commit message as suggested by Eric (convert past tense\n to present tense) and added a few tweaks to it that striked me\n\nEric Sunshine writes:\n> Thanks for taking up the slack.\n\nYou're welcome. It was easier than waiting for this patch to be\nupdated so it could get into 'pu' ;-)\n\n\n imap-send.c | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex 54e6a80fd..36c7c1b4f 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -1412,6 +1412,7 @@ static CURL *setup_curl(struct imap_server_conf *srvc, struct credential *cred)\n {\n \tCURL *curl;\n \tstruct strbuf path = STRBUF_INIT;\n+\tchar *uri_encoded_folder;\n \n \tif (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)\n \t\tdie(\"curl_global_init failed\");\n@@ -1429,7 +1430,12 @@ static CURL *setup_curl(struct imap_server_conf *srvc, struct credential *cred)\n \tstrbuf_addstr(&path, server.host);\n \tif (!path.len || path.buf[path.len - 1] != '/')\n \t\tstrbuf_addch(&path, '/');\n-\tstrbuf_addstr(&path, server.folder);\n+\n+\turi_encoded_folder = curl_easy_escape(curl, server.folder, 0);\n+\tif (!uri_encoded_folder)\n+\t\tdie(\"failed to encode server folder\");\n+\tstrbuf_addstr(&path, uri_encoded_folder);\n+\tcurl_free(uri_encoded_folder);\n \n \tcurl_easy_setopt(curl, CURLOPT_URL, path.buf);\n \tstrbuf_release(&path);\n-- \n2.15.1.620.gb9897f467\n\n"},{"id":"334999","messageId":"20171218221945.GC7854@aiede.mtv.corp.google.com","threadId":"47347","inReplyTo":"20171218191113.18600-1-kaartic.sivaraam@gmail.com","subject":"Re: [PATCH v3] imap-send: URI encode server folder","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-12-18T22:19:45Z","receivedAt":"2017-12-18T22:19:53Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Kaartic Sivaraam wrote:\n\n> From: Nicolas Morey-Chaisemartin \n>\n> When trying to send a patch using 'imap-send' with 'curl' and the\n> following configuration:\n>\n> [imap]\n> \tfolder = \"[Gmail]/Drafts\"\n> \thost = imaps://imap.gmail.com\n> \tport = 993\n> \tsslverify = false\n>\n> results in the following error,\n>\n> curl_easy_perform() failed: URL using bad/illegal format or missing URL\n>\n> This is a consequence of not URI-encoding the folder portion of\n> the URL which contains characters such as '[' which are not\n> allowed in a URI. According to RFC3986, these characters should be\n> URI-encoded.\n>\n> So, URI-encode the folder before adding it to the URI to ensure it doesn't\n> contain characters that aren't allowed in a URI.\n>\n> Reported-by: Doron Behar \n> Signed-off-by: Nicolas Morey-Chaisemartin \n> Signed-off-by: Kaartic Sivaraam \n> ---\n> imap-send.c | 8 +++++++-\n> 1 file changed, 7 insertions(+), 1 deletion(-)\n\nThanks!\nReviewed-by: Jonathan Nieder \n\nIs there a straightforward way to check that this behavior gets\npreserved in tests?\n\nSincerely,\nJonathan\n"},{"id":"335000","messageId":"xmqq7etjy8ej.fsf@gitster.mtv.corp.google.com","threadId":"47347","inReplyTo":"20171218191113.18600-1-kaartic.sivaraam@gmail.com","subject":"Re: [PATCH v3] imap-send: URI encode server folder","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-18T22:32:20Z","receivedAt":"2017-12-18T22:32:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kaartic Sivaraam writes:\n\n> From: Nicolas Morey-Chaisemartin \n>\n> When trying to send a patch using 'imap-send' with 'curl' and the\n> following configuration:\n>\n> [imap]\n> \tfolder = \"[Gmail]/Drafts\"\n> \thost = imaps://imap.gmail.com\n> \tport = 993\n> \tsslverify = false\n>\n> results in the following error,\n>\n> curl_easy_perform() failed: URL using bad/illegal format or missing URL\n>\n> This is a consequence of not URI-encoding the folder portion of\n> the URL which contains characters such as '[' which are not\n> allowed in a URI. According to RFC3986, these characters should be\n> URI-encoded.\n>\n> So, URI-encode the folder before adding it to the URI to ensure it doesn't\n> contain characters that aren't allowed in a URI.\n>\n> Reported-by: Doron Behar \n> Signed-off-by: Nicolas Morey-Chaisemartin \n> Signed-off-by: Kaartic Sivaraam \n> ---\n> Changes in v3:\n> - updated commit message as suggested by Eric (convert past tense\n> to present tense) and added a few tweaks to it that striked me\n>\n> Eric Sunshine writes:\n>> Thanks for taking up the slack.\n>\n> You're welcome. It was easier than waiting for this patch to be\n> updated so it could get into 'pu' ;-)\n\nLooks good. Thanks.\n\n\n> imap-send.c | 8 +++++++-\n> 1 file changed, 7 insertions(+), 1 deletion(-)\n>\n> diff --git a/imap-send.c b/imap-send.c\n> index 54e6a80fd..36c7c1b4f 100644\n> --- a/imap-send.c\n> +++ b/imap-send.c\n> @@ -1412,6 +1412,7 @@ static CURL *setup_curl(struct imap_server_conf *srvc, struct credential *cred)\n> {\n> \tCURL *curl;\n> \tstruct strbuf path = STRBUF_INIT;\n> +\tchar *uri_encoded_folder;\n> \n> \tif (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)\n> \t\tdie(\"curl_global_init failed\");\n> @@ -1429,7 +1430,12 @@ static CURL *setup_curl(struct imap_server_conf *srvc, struct credential *cred)\n> \tstrbuf_addstr(&path, server.host);\n> \tif (!path.len || path.buf[path.len - 1] != '/')\n> \t\tstrbuf_addch(&path, '/');\n> -\tstrbuf_addstr(&path, server.folder);\n> +\n> +\turi_encoded_folder = curl_easy_escape(curl, server.folder, 0);\n> +\tif (!uri_encoded_folder)\n> +\t\tdie(\"failed to encode server folder\");\n> +\tstrbuf_addstr(&path, uri_encoded_folder);\n> +\tcurl_free(uri_encoded_folder);\n> \n> \tcurl_easy_setopt(curl, CURLOPT_URL, path.buf);\n> \tstrbuf_release(&path);\n"}]}