{"thread":{"id":"46513","subject":"[RFC] imap-send: escape backslash in password","startedAt":"2017-08-04T18:46:36Z","lastAt":"2017-08-09T12:04:41Z","messageCount":15,"participants":["Nicolas Morey-Chaisemartin","Junio C Hamano","Andreas Schwab","brian m. carlson","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"325617","messageId":"58b783d6-c024-4491-2f88-edfb9c43c55c@morey-chaisemartin.com","threadId":"46513","inReplyTo":null,"subject":"[RFC] imap-send: escape backslash in password","fromName":"Nicolas Morey-Chaisemartin","fromEmail":"nicolas@morey-chaisemartin.com","sentAt":"2017-08-04T16:16:53Z","receivedAt":"2017-08-04T18:46:36Z","isPatch":false,"sender":{"key":"devel-git@morey-chaisemartin.com","avatar":"https://avatars.githubusercontent.com/u/108326?v=4"},"body":"Password containing backslashes need to have them doubled to have them properly interpreted by the imap server.\n\nA password terminating with a blackslash used to trigger this error:\nIMAP command 'LOGIN ' returned response (BAD) - Missing '\"'\n\nSigned-off-by: Nicolas Morey-Chaisemartin \n---\n imap-send.c | 28 +++++++++++++++++++++++++++-\n 1 file changed, 27 insertions(+), 1 deletion(-)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex b2d0b849b..7fde6ec96 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -926,6 +926,32 @@ static int auth_cram_md5(struct imap_store *ctx, struct imap_cmd *cmd, const cha\n \treturn 0;\n }\n \n+static char* imap_escape_password(const char *passwd)\n+{\n+\tconst unsigned passwd_len = strlen(passwd);\n+\tchar *escaped = xmalloc(2 * passwd_len + 1);\n+\tconst char *passwd_cur = passwd;\n+\tchar *escaped_cur = escaped;\n+\n+\tdo {\n+\t\tchar *next = strchr(passwd_cur, '\\\\');\n+\n+\t\tif (!next) {\n+\t\t\tstrcpy(escaped_cur, passwd_cur);\n+\t\t} else {\n+\t\t\tint len = next - passwd_cur + 1;\n+\n+\t\t\tmemcpy(escaped_cur, passwd_cur, len);\n+\t\t\tescaped_cur += len;\n+\t\t\tnext++;\n+\t\t\t*(escaped_cur++) = '\\\\';\n+\t\t}\n+\t\tpasswd_cur = next;\n+\t} while(passwd_cur);\n+\n+\treturn escaped;\n+}\n+\n static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *folder)\n {\n \tstruct credential cred = CREDENTIAL_INIT;\n@@ -1090,7 +1116,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *f\n \t\t\tif (!srvc->user)\n \t\t\t\tsrvc->user = xstrdup(cred.username);\n \t\t\tif (!srvc->pass)\n-\t\t\t\tsrvc->pass = xstrdup(cred.password);\n+\t\t\t\tsrvc->pass = imap_escape_password(cred.password);\n \t\t}\n \n \t\tif (srvc->auth_method) {\n"},{"id":"325618","messageId":"xmqqbmnvtain.fsf@gitster.mtv.corp.google.com","threadId":"46513","inReplyTo":"58b783d6-c024-4491-2f88-edfb9c43c55c@morey-chaisemartin.com","subject":"Re: [RFC] imap-send: escape backslash in password","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-04T19:09:52Z","receivedAt":"2017-08-04T19:10:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Morey-Chaisemartin writes:\n\n> Password containing backslashes need to have them doubled to have them properly interpreted by the imap server.\n\nPlease wrap this into lines with reasonable lengths like 72 cols.\n\nIs the quoting rules documented somewhere? If so, please also give\na reference to it here. RFC3501 \"6.2.3 LOGIN Command\" does not say\nmuch (other parts of the RFC may specify the rules that apply to\narguments in general, but I didn't look for them). Without such\nreference, it is hard to judge if this change is sufficient or even\ncorrect (in an extreme case, the IMAP server you are talking with\nthat prompted you to make this change might be in violation).\n\nFor example, FRC3501 \"9. Formal Syntax\" says that both \"password\"\nand \"userid\" are \"astring\"; it looks strange that the code with this\npatch only touches cred.password while sending cred.username as-is.\n\n> +static char* imap_escape_password(const char *passwd)\n\nIn our codebase, asterisk sticks to identifier, not typename. I.e.\n\n\tstatic char *imap_escape(...)\n\n> +{\n> +\tconst unsigned passwd_len = strlen(passwd);\n> +\tchar *escaped = xmalloc(2 * passwd_len + 1);\n> +\tconst char *passwd_cur = passwd;\n> +\tchar *escaped_cur = escaped;\n> +\n> +\tdo {\n> +\t\tchar *next = strchr(passwd_cur, '\\\\');\n> +\n> +\t\tif (!next) {\n> +\t\t\tstrcpy(escaped_cur, passwd_cur);\n> +\t\t} else {\n> +\t\t\tint len = next - passwd_cur + 1;\n> +\n> +\t\t\tmemcpy(escaped_cur, passwd_cur, len);\n> +\t\t\tescaped_cur += len;\n> +\t\t\tnext++;\n> +\t\t\t*(escaped_cur++) = '\\\\';\n> +\t\t}\n> +\t\tpasswd_cur = next;\n> +\t} while(passwd_cur);\n> +\n> +\treturn escaped;\n> +}\n\nI wonder if we should use strbuf here perhaps like so:\n\n\tstruct strbuf encoded = STRBUF_INIT;\n\tconst char *p;\n\n\tfor (p = passwd; *p; p++) {\n\t\tif (need_bs_quote(*p))\n\t\t\tstrbuf_addch(&encoded, '\\\\');\n\t\tstrbuf_addch(&encoded, *p);\n\t}\n\treturn strbuf_detach(&encoded, NULL);\n\n> static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *folder)\n> {\n> \tstruct credential cred = CREDENTIAL_INIT;\n> @@ -1090,7 +1116,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *f\n> \t\t\tif (!srvc->user)\n> \t\t\t\tsrvc->user = xstrdup(cred.username);\n> \t\t\tif (!srvc->pass)\n> -\t\t\t\tsrvc->pass = xstrdup(cred.password);\n> +\t\t\t\tsrvc->pass = imap_escape_password(cred.password);\n> \t\t}\n> \n> \t\tif (srvc->auth_method) {\n\nThanks.\n"},{"id":"325620","messageId":"90b60e41-f59f-a141-1c83-3605444333b4@morey-chaisemartin.com","threadId":"46513","inReplyTo":"xmqqbmnvtain.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC] imap-send: escape backslash in password","fromName":"Nicolas Morey-Chaisemartin","fromEmail":"nicolas@morey-chaisemartin.com","sentAt":"2017-08-04T19:32:32Z","receivedAt":"2017-08-04T19:32:56Z","isPatch":false,"sender":{"key":"devel-git@morey-chaisemartin.com","avatar":"https://avatars.githubusercontent.com/u/108326?v=4"},"body":"\n\nLe 04/08/2017 à 21:09, Junio C Hamano a écrit :\n> Nicolas Morey-Chaisemartin writes:\n>\n>> Password containing backslashes need to have them doubled to have them properly interpreted by the imap server.\n> Please wrap this into lines with reasonable lengths like 72 cols.\n\nI haven't checked the coding style yet. This was a very quick try that I submitted to get some feedback on the approach.\nWIll fix for next time though.\n\n>\n> Is the quoting rules documented somewhere? If so, please also give\n> a reference to it here. RFC3501 \"6.2.3 LOGIN Command\" does not say\n> much (other parts of the RFC may specify the rules that apply to\n> arguments in general, but I didn't look for them). Without such\n> reference, it is hard to judge if this change is sufficient or even\n> correct (in an extreme case, the IMAP server you are talking with\n> that prompted you to make this change might be in violation).\n>\n> For example, FRC3501 \"9. Formal Syntax\" says that both \"password\"\n> and \"userid\" are \"astring\"; it looks strange that the code with this\n> patch only touches cred.password while sending cred.username as-is.\n\nDidn't found a RFC doc on this. I hit the bug today and looking at the error message, found a few people who add the issue with different client and required escaping backslashes\nIt probably applies to the username (would be logicial that both string in the line are parsed the same way) not sure if backslashes are allowed in username though.\nWith password generator, they are more likely to be there.\nBut it wouldn't hurt to use the escape function for both.\n\n>> +static char* imap_escape_password(const char *passwd)\n> In our codebase, asterisk sticks to identifier, not typename. I.e.\n>\n> \tstatic char *imap_escape(...)\nWill do. BTW, is there a checkpatch or similar for git ?\nScrolled quickly through the doc and did not see any reference.\n\n>\n>> +{\n>> +\tconst unsigned passwd_len = strlen(passwd);\n>> +\tchar *escaped = xmalloc(2 * passwd_len + 1);\n>> +\tconst char *passwd_cur = passwd;\n>> +\tchar *escaped_cur = escaped;\n>> +\n>> +\tdo {\n>> +\t\tchar *next = strchr(passwd_cur, '\\\\');\n>> +\n>> +\t\tif (!next) {\n>> +\t\t\tstrcpy(escaped_cur, passwd_cur);\n>> +\t\t} else {\n>> +\t\t\tint len = next - passwd_cur + 1;\n>> +\n>> +\t\t\tmemcpy(escaped_cur, passwd_cur, len);\n>> +\t\t\tescaped_cur += len;\n>> +\t\t\tnext++;\n>> +\t\t\t*(escaped_cur++) = '\\\\';\n>> +\t\t}\n>> +\t\tpasswd_cur = next;\n>> +\t} while(passwd_cur);\n>> +\n>> +\treturn escaped;\n>> +}\n> I wonder if we should use strbuf here perhaps like so:\n>\n> \tstruct strbuf encoded = STRBUF_INIT;\n> \tconst char *p;\n>\n> \tfor (p = passwd; *p; p++) {\n> \t\tif (need_bs_quote(*p))\n> \t\t\tstrbuf_addch(&encoded, '\\\\');\n> \t\tstrbuf_addch(&encoded, *p);\n> \t}\n> \treturn strbuf_detach(&encoded, NULL);\n\nI looked at the wrappers and wasn't sure if they were to be used for this (one of the main reason this is an RFC).\nI guess it would make sense. I'm not familiar with git code, but is there other escape function of this kind that could be factor ?\nOr the function is simple enough not to be worth it ?\n\nThanks\n\nNicolas\n\n"},{"id":"325621","messageId":"87bmnvktee.fsf@linux-m68k.org","threadId":"46513","inReplyTo":"xmqqbmnvtain.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC] imap-send: escape backslash in password","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2017-08-04T19:46:49Z","receivedAt":"2017-08-04T19:46:58Z","isPatch":false,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"On Aug 04 2017, Junio C Hamano wrote:\n\n> Is the quoting rules documented somewhere? If so, please also give\n> a reference to it here. RFC3501 \"6.2.3 LOGIN Command\" does not say\n> much (other parts of the RFC may specify the rules that apply to\n> arguments in general, but I didn't look for them). Without such\n> reference, it is hard to judge if this change is sufficient or even\n> correct (in an extreme case, the IMAP server you are talking with\n> that prompted you to make this change might be in violation).\n>\n> For example, FRC3501 \"9. Formal Syntax\" says that both \"password\"\n> and \"userid\" are \"astring\"; it looks strange that the code with this\n> patch only touches cred.password while sending cred.username as-is.\n\nastring = ... / string\nstring = quoted / ...\nquoted = DQUOTE *QUOTED-CHAR DQUOTE\nQUOTED-CHAR = /\n \"\\\" quoted-specials\nquoted-specials = DQUOTE / \"\\\"\n\nThus the quoting applies to any element that is a string (and a double\nquote needs to be quoted as well).\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"325622","messageId":"20170804200643.gq5pqeoeje3xii57@genre.crustytoothpaste.net","threadId":"46513","inReplyTo":"58b783d6-c024-4491-2f88-edfb9c43c55c@morey-chaisemartin.com","subject":"Re: [RFC] imap-send: escape backslash in password","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2017-08-04T20:06:43Z","receivedAt":"2017-08-04T20:06:55Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Fri, Aug 04, 2017 at 06:16:53PM +0200, Nicolas Morey-Chaisemartin wrote:\n> static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *folder)\n> {\n> \tstruct credential cred = CREDENTIAL_INIT;\n> @@ -1090,7 +1116,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *f\n> \t\t\tif (!srvc->user)\n> \t\t\t\tsrvc->user = xstrdup(cred.username);\n> \t\t\tif (!srvc->pass)\n> -\t\t\t\tsrvc->pass = xstrdup(cred.password);\n> +\t\t\t\tsrvc->pass = imap_escape_password(cred.password);\n> \t\t}\n> \n> \t\tif (srvc->auth_method) {\n\nI'm not sure if this is correct. It looks like this username and\npassword are used by whatever authentication method we use, whether\nthat's LOGIN or CRAM-MD5. I don't think we'd want to encode the\npassword here before sending it through the CRAM-MD5 authenticator.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\nhttps://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"325623","messageId":"20170804201836.3t5fp7v5xfwz64h4@sigill.intra.peff.net","threadId":"46513","inReplyTo":"20170804200643.gq5pqeoeje3xii57@genre.crustytoothpaste.net","subject":"Re: [RFC] imap-send: escape backslash in password","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-04T20:18:36Z","receivedAt":"2017-08-04T20:18:43Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 04, 2017 at 08:06:43PM +0000, brian m. carlson wrote:\n\n> On Fri, Aug 04, 2017 at 06:16:53PM +0200, Nicolas Morey-Chaisemartin wrote:\n> > static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *folder)\n> > {\n> > \tstruct credential cred = CREDENTIAL_INIT;\n> > @@ -1090,7 +1116,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *f\n> > \t\t\tif (!srvc->user)\n> > \t\t\t\tsrvc->user = xstrdup(cred.username);\n> > \t\t\tif (!srvc->pass)\n> > -\t\t\t\tsrvc->pass = xstrdup(cred.password);\n> > +\t\t\t\tsrvc->pass = imap_escape_password(cred.password);\n> > \t\t}\n> > \n> > \t\tif (srvc->auth_method) {\n> \n> I'm not sure if this is correct. It looks like this username and\n> password are used by whatever authentication method we use, whether\n> that's LOGIN or CRAM-MD5. I don't think we'd want to encode the\n> password here before sending it through the CRAM-MD5 authenticator.\n\nYeah. This is an on-the-wire encoding issue, and should happen as part\nof forming the protocol string to send. So:\n\n imap_exec(ctx, NULL, \"LOGIN \\\"%s\\\" \\\"%s\\\"\", srvc->user, srvc->pass)\n\nis probably where it needs to happen.\n\nIt looks like this issue is present in a lot of other places, too. Just\na few lines below I see:\n\n imap_exec(ctx, NULL, \"CREATE \\\"%s\\\"\", ctx->name)\n\nAs an aside, these are all potential injection vulnerabilities, too.\nE.g., if I specify my folder as\n\n foo\"\\n. DELETE \"bar\n\nthen we'd issue an accidental deletion. I doubt it's a big deal in\npractice, as it's not common to feed attacker-controlled strings to\nimap-send. But we should probably fix it anyway.\n\nThe right interface is probably to teach imap_exec() to take a\nNULL-terminated list of items (rather than a format string) and then\nquote each one appropriately.\n\n-Peff\n"},{"id":"325624","messageId":"20170804202255.3oia7ivsoa6vu4me@sigill.intra.peff.net","threadId":"46513","inReplyTo":"87bmnvktee.fsf@linux-m68k.org","subject":"Re: [RFC] imap-send: escape backslash in password","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-04T20:22:55Z","receivedAt":"2017-08-04T20:23:02Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 04, 2017 at 09:46:49PM +0200, Andreas Schwab wrote:\n\n> > For example, FRC3501 \"9. Formal Syntax\" says that both \"password\"\n> > and \"userid\" are \"astring\"; it looks strange that the code with this\n> > patch only touches cred.password while sending cred.username as-is.\n> \n> astring = ... / string\n> string = quoted / ...\n> quoted = DQUOTE *QUOTED-CHAR DQUOTE\n> QUOTED-CHAR = /\n> \"\\\" quoted-specials\n> quoted-specials = DQUOTE / \"\\\"\n> \n> Thus the quoting applies to any element that is a string (and a double\n> quote needs to be quoted as well).\n\nIt's been a long time since I've done anything with IMAP, but I think\nanother alternative would be to send it as a \"literal\", like:\n\n {6}\n foobar\n\nThat's relatively easy to format correctly using the current printf\nspecifiers that imap_exec() takes. Though as I said elsewhere in the\nthread, perhaps imap_exec() should provide a different interface.\n\nI also think it might be reasonable to scrap all of this ad-hoc imap\ncode in favor of curl, which already gets these cases right. We already\nhave a curl-backed implementation. I think we just left the old code out\nof conservatism. But it seems somewhat buggy and unmaintained, and I\nwonder if we aren't better off to simply encourage people to install\ncurl.\n\n-Peff\n"},{"id":"325627","messageId":"xmqq3797t4kq.fsf@gitster.mtv.corp.google.com","threadId":"46513","inReplyTo":"20170804202255.3oia7ivsoa6vu4me@sigill.intra.peff.net","subject":"Re: [RFC] imap-send: escape backslash in password","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-04T21:18:13Z","receivedAt":"2017-08-04T21:18:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> It's been a long time since I've done anything with IMAP, but I think\n> another alternative would be to send it as a \"literal\", like:\n>\n> {6}\n> foobar\n>\n> That's relatively easy to format correctly using the current printf\n> specifiers that imap_exec() takes. Though as I said elsewhere in the\n> thread, perhaps imap_exec() should provide a different interface.\n\nYes, I was scanning the RFC and came to the same conclusion ;-)\n\n> I also think it might be reasonable to scrap all of this ad-hoc imap\n> code in favor of curl, which already gets these cases right. We already\n> have a curl-backed implementation. I think we just left the old code out\n> of conservatism. But it seems somewhat buggy and unmaintained, and I\n> wonder if we aren't better off to simply encourage people to install\n> curl.\n\nThat is a very attractive direction to go in, especially in the mid\nto longer term. Perhaps we declare that the ad-hoc hardcoded imap\nis deprecated in the next cycle and drop the support by the end of\nthis year?\n\n"},{"id":"325629","messageId":"20170804212231.pl3uipcsujflcuha@sigill.intra.peff.net","threadId":"46513","inReplyTo":"xmqq3797t4kq.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC] imap-send: escape backslash in password","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-04T21:22:31Z","receivedAt":"2017-08-04T21:22:37Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 04, 2017 at 02:18:13PM -0700, Junio C Hamano wrote:\n\n> > I also think it might be reasonable to scrap all of this ad-hoc imap\n> > code in favor of curl, which already gets these cases right. We already\n> > have a curl-backed implementation. I think we just left the old code out\n> > of conservatism. But it seems somewhat buggy and unmaintained, and I\n> > wonder if we aren't better off to simply encourage people to install\n> > curl.\n> \n> That is a very attractive direction to go in, especially in the mid\n> to longer term. Perhaps we declare that the ad-hoc hardcoded imap\n> is deprecated in the next cycle and drop the support by the end of\n> this year?\n\nThat is fine by me. AFAIK, we already build the curl support by default\nwhen a sufficiently-advanced version of curl is available. So if there\nwere feature-parity problems hopefully somebody would have reported it.\n\nI think the deprecation here can be relatively fast because we're not\nactually dropping support for any feature. We're just requiring that\nthey install curl to get the same functionality (which might be\ninconvenient, but it's a heck of a lot less inconvenient than \"there's\nno way to do what you want anymore\").\n\n-Peff\n"},{"id":"325660","messageId":"feae203b-222c-4430-8af2-aae76c7e0bb0@morey-chaisemartin.com","threadId":"46513","inReplyTo":"20170804212231.pl3uipcsujflcuha@sigill.intra.peff.net","subject":"Re: [RFC] imap-send: escape backslash in password","fromName":"Nicolas Morey-Chaisemartin","fromEmail":"nicolas@morey-chaisemartin.com","sentAt":"2017-08-06T19:12:16Z","receivedAt":"2017-08-06T20:28:46Z","isPatch":false,"sender":{"key":"devel-git@morey-chaisemartin.com","avatar":"https://avatars.githubusercontent.com/u/108326?v=4"},"body":"\nLe 04/08/2017 à 23:22, Jeff King a écrit :\n> On Fri, Aug 04, 2017 at 02:18:13PM -0700, Junio C Hamano wrote:\n>\n>>> I also think it might be reasonable to scrap all of this ad-hoc imap\n>>> code in favor of curl, which already gets these cases right. We already\n>>> have a curl-backed implementation. I think we just left the old code out\n>>> of conservatism. But it seems somewhat buggy and unmaintained, and I\n>>> wonder if we aren't better off to simply encourage people to install\n>>> curl.\n>> That is a very attractive direction to go in, especially in the mid\n>> to longer term. Perhaps we declare that the ad-hoc hardcoded imap\n>> is deprecated in the next cycle and drop the support by the end of\n>> this year?\n> That is fine by me. AFAIK, we already build the curl support by default\n> when a sufficiently-advanced version of curl is available. So if there\n> were feature-parity problems hopefully somebody would have reported it.\n>\n> I think the deprecation here can be relatively fast because we're not\n> actually dropping support for any feature. We're just requiring that\n> they install curl to get the same functionality (which might be\n> inconvenient, but it's a heck of a lot less inconvenient than \"there's\n> no way to do what you want anymore\").\n>\n> -Peff\n\nThere is at least one difference right now:\nWhen using --curl, the username/password are loaded from the gitconfig file only.\nWhen using the legacy imap interface, it goes through credential_fill which prompts for a password.\n\nI don't think everyone is ready to store his email account password in a gitconfig file (I know I'm not).\nI don't see why it couldn't be fixed. I'll give it a try tomorrow.\n\nAlso it probably make sense to have at least one release where --curl is the default. Until your mail I had no idea this option existed so I never tried it out.\nMaking it the default will make sure almost everyone is using it and that there is feature-parity.\n\nBut I agree it's probably safer and cleaner to let curl handle everything and drop the legacy stuff once it fully works.\n\nNicolas\n\n"},{"id":"325666","messageId":"xmqqzibcqhy9.fsf@gitster.mtv.corp.google.com","threadId":"46513","inReplyTo":"20170804212231.pl3uipcsujflcuha@sigill.intra.peff.net","subject":"Re: [RFC] imap-send: escape backslash in password","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-07T01:34:22Z","receivedAt":"2017-08-07T01:34:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> That is fine by me. AFAIK, we already build the curl support by default\n> when a sufficiently-advanced version of curl is available. So if there\n> were feature-parity problems hopefully somebody would have reported it.\n>\n> I think the deprecation here can be relatively fast because we're not\n> actually dropping support for any feature. We're just requiring that\n> they install curl to get the same functionality (which might be\n> inconvenient, but it's a heck of a lot less inconvenient than \"there's\n> no way to do what you want anymore\").\n\nYeah, as long as imap-supporting libcurl is not too recent and are\navailable everywhere, we should be OK.\n\nThanks.\n"},{"id":"325741","messageId":"20170807205830.yge6kcpmg5gkogjj@sigill.intra.peff.net","threadId":"46513","inReplyTo":"feae203b-222c-4430-8af2-aae76c7e0bb0@morey-chaisemartin.com","subject":"Re: [RFC] imap-send: escape backslash in password","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-07T20:58:30Z","receivedAt":"2017-08-07T20:58:45Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 06, 2017 at 09:12:16PM +0200, Nicolas Morey-Chaisemartin wrote:\n\n> Also it probably make sense to have at least one release where --curl\n> is the default. Until your mail I had no idea this option existed so I\n> never tried it out.\n> Making it the default will make sure almost everyone is using it and\n> that there is feature-parity.\n\nYeah, I had thought that the curl implementation _was_ the default if\nyou have curl. But we just build it by default, and you have to manually\nenable it. So I agree it has not gotten nearly as much testing as I had\nthought, and as you found, it diverges from the earlier implementation\nin quite a few areas.\n\nSo I think we would need to take any deprecation much more slowly than I\nhad first thought (and your patches in the nearby thread are moving in a\ngood direction).\n\n-Peff\n"},{"id":"325784","messageId":"20170808072510.leb525df4hmbwcvo@sigill.intra.peff.net","threadId":"46513","inReplyTo":"xmqqzibcqhy9.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC] imap-send: escape backslash in password","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-08T07:25:10Z","receivedAt":"2017-08-08T07:25:17Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 06, 2017 at 06:34:22PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > That is fine by me. AFAIK, we already build the curl support by default\n> > when a sufficiently-advanced version of curl is available. So if there\n> > were feature-parity problems hopefully somebody would have reported it.\n> >\n> > I think the deprecation here can be relatively fast because we're not\n> > actually dropping support for any feature. We're just requiring that\n> > they install curl to get the same functionality (which might be\n> > inconvenient, but it's a heck of a lot less inconvenient than \"there's\n> > no way to do what you want anymore\").\n> \n> Yeah, as long as imap-supporting libcurl is not too recent and are\n> available everywhere, we should be OK.\n\nI think we're not quite ready to switch to curl based on comments in the\nnearby thread. But just for reference, since I started looking into\nthis...\n\nThe defines in the Makefile turn on USE_CURL_FOR_IMAP_SEND want curl\n7.34.0. That's only from 2013, which is probably recent enough that it\nmay cause a problem (I had originally thought it was a few years older,\nbut I forgot the curl version hex encoding; 072200 is 7.34.0).\n\nFor comparison, nothing older than curl 7.19.4 will work for building\nGit since v2.12.0, as we added some unconditional uses of CURLPROTO_*\nthere. Nobody seems to have noticed or complained. I pointed this out a\nfew months ago[1] and suggested we clean up some of the more antiquated\n#if blocks in http.c that don't even build. There was some complaint\nthat we should keep even these ancient versions working, but the\ncompile error is still in \"master\".\n\nSo it's not clear to me that anybody cares about going that far back\n(which is mid-2009), but I'd guess that 2013 might cause some problems.\n\n[1] https://public-inbox.org/git/20170404025438.bgxz5sfmrawqswcj@sigill.intra.peff.net/\n if you're curious (you were offline for a while at that time, I\n think).\n"},{"id":"325813","messageId":"xmqqk22ec84i.fsf@gitster.mtv.corp.google.com","threadId":"46513","inReplyTo":"20170808072510.leb525df4hmbwcvo@sigill.intra.peff.net","subject":"Re: [RFC] imap-send: escape backslash in password","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-08T16:54:53Z","receivedAt":"2017-08-08T16:55:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I think we're not quite ready to switch to curl based on comments in the\n> nearby thread. But just for reference, since I started looking into\n> this...\n>\n> The defines in the Makefile turn on USE_CURL_FOR_IMAP_SEND want curl\n> 7.34.0. That's only from 2013, which is probably recent enough that it\n> may cause a problem (I had originally thought it was a few years older,\n> but I forgot the curl version hex encoding; 072200 is 7.34.0).\n>\n> For comparison, nothing older than curl 7.19.4 will work for building\n> Git since v2.12.0, as we added some unconditional uses of CURLPROTO_*\n> there. Nobody seems to have noticed or complained. I pointed this out a\n> few months ago[1] and suggested we clean up some of the more antiquated\n> #if blocks in http.c that don't even build. There was some complaint\n> that we should keep even these ancient versions working, but the\n> compile error is still in \"master\".\n>\n> So it's not clear to me that anybody cares about going that far back\n> (which is mid-2009), but I'd guess that 2013 might cause some problems.\n>\n> [1] https://public-inbox.org/git/20170404025438.bgxz5sfmrawqswcj@sigill.intra.peff.net/\n> if you're curious (you were offline for a while at that time, I\n> think).\n\nThanks for digging. It would not help the issue on this thread at\nall. While I agree with your conclusion in the quoted thread:\n\n I think it might be nice to declare a \"too old\" version, though,\n just so we can stop adding _new_ ifdefs. Maybe 7.11.1 is that\n version now, and in another few years we can bump to 7.16.0. :)\n\nit appears that we silently declared it to 7.19.4 and found out that\nnobody complained, without us having to wait for a few years?\n\n"},{"id":"325941","messageId":"20170809120432.am77utnrnkh6mpgt@sigill.intra.peff.net","threadId":"46513","inReplyTo":"xmqqk22ec84i.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC] imap-send: escape backslash in password","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-09T12:04:32Z","receivedAt":"2017-08-09T12:04:41Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 08, 2017 at 09:54:53AM -0700, Junio C Hamano wrote:\n\n> > For comparison, nothing older than curl 7.19.4 will work for building\n> > Git since v2.12.0, as we added some unconditional uses of CURLPROTO_*\n> > there. Nobody seems to have noticed or complained. I pointed this out a\n> > few months ago[1] and suggested we clean up some of the more antiquated\n> > #if blocks in http.c that don't even build. There was some complaint\n> > that we should keep even these ancient versions working, but the\n> > compile error is still in \"master\".\n> >\n> > So it's not clear to me that anybody cares about going that far back\n> > (which is mid-2009), but I'd guess that 2013 might cause some problems.\n> >\n> > [1] https://public-inbox.org/git/20170404025438.bgxz5sfmrawqswcj@sigill.intra.peff.net/\n> > if you're curious (you were offline for a while at that time, I\n> > think).\n> \n> Thanks for digging. It would not help the issue on this thread at\n> all. While I agree with your conclusion in the quoted thread:\n> \n> I think it might be nice to declare a \"too old\" version, though,\n> just so we can stop adding _new_ ifdefs. Maybe 7.11.1 is that\n> version now, and in another few years we can bump to 7.16.0. :)\n> \n> it appears that we silently declared it to 7.19.4 and found out that\n> nobody complained, without us having to wait for a few years?\n\nYeah, I think the 7.19.4 breakage was discussed after I wrote that (and\nafter investigating, I think the 7.16.0 cutoff is probably pretty\nreasonable even without that). I do think it's worth addressing. I just\nsent a series:\n\n https://public-inbox.org/git/20170809120024.7phdjzjv54uv5dpz@sigill.intra.peff.net/\n\n-Peff\n"}]}