Re: [RFC] imap-send: escape backslash in password

Jeff King [off-list ref] writes:

It's been a long time since I've done anything with IMAP, but I think
another alternative would be to send it as a "literal", like:

  {6}
  foobar

That's relatively easy to format correctly using the current printf
specifiers that imap_exec() takes. Though as I said elsewhere in the
thread, perhaps imap_exec() should provide a different interface.

Yes, I was scanning the RFC and came to the same conclusion ;-)

I also think it might be reasonable to scrap all of this ad-hoc imap
code in favor of curl, which already gets these cases right. We already
have a curl-backed implementation. I think we just left the old code out
of conservatism. But it seems somewhat buggy and unmaintained, and I
wonder if we aren't better off to simply encourage people to install
curl.

That is a very attractive direction to go in, especially in the mid
to longer term.  Perhaps we declare that the ad-hoc hardcoded imap
is deprecated in the next cycle and drop the support by the end of
this year?

On Fri, Aug 04, 2017 at 02:18:13PM -0700, Junio C Hamano wrote:

quoted

I also think it might be reasonable to scrap all of this ad-hoc imap
code in favor of curl, which already gets these cases right. We already
have a curl-backed implementation. I think we just left the old code out
of conservatism. But it seems somewhat buggy and unmaintained, and I
wonder if we aren't better off to simply encourage people to install
curl.

That is a very attractive direction to go in, especially in the mid
to longer term.  Perhaps we declare that the ad-hoc hardcoded imap
is deprecated in the next cycle and drop the support by the end of
this year?

That is fine by me. AFAIK, we already build the curl support by default
when a sufficiently-advanced version of curl is available. So if there
were feature-parity problems hopefully somebody would have reported it.

I think the deprecation here can be relatively fast because we're not
actually dropping support for any feature. We're just requiring that
they install curl to get the same functionality (which might be
inconvenient, but it's a heck of a lot less inconvenient than "there's
no way to do what you want anymore").

-Peff

Le 04/08/2017 à 23:22, Jeff King a écrit :

On Fri, Aug 04, 2017 at 02:18:13PM -0700, Junio C Hamano wrote:

quoted

quoted

I also think it might be reasonable to scrap all of this ad-hoc imap
code in favor of curl, which already gets these cases right. We already
have a curl-backed implementation. I think we just left the old code out
of conservatism. But it seems somewhat buggy and unmaintained, and I
wonder if we aren't better off to simply encourage people to install
curl.

That is a very attractive direction to go in, especially in the mid
to longer term.  Perhaps we declare that the ad-hoc hardcoded imap
is deprecated in the next cycle and drop the support by the end of
this year?

That is fine by me. AFAIK, we already build the curl support by default
when a sufficiently-advanced version of curl is available. So if there
were feature-parity problems hopefully somebody would have reported it.

I think the deprecation here can be relatively fast because we're not
actually dropping support for any feature. We're just requiring that
they install curl to get the same functionality (which might be
inconvenient, but it's a heck of a lot less inconvenient than "there's
no way to do what you want anymore").

-Peff

There is at least one difference right now:
When using --curl, the username/password are loaded from the gitconfig file only.
When using the legacy imap interface, it goes through credential_fill which prompts for a password.

I don't think everyone is ready to store his email account password in a gitconfig file (I know I'm not).
I don't see why it couldn't be fixed. I'll give it a try tomorrow.

Also 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.
Making it the default will make sure almost everyone is using it and that there is feature-parity.

But I agree it's probably safer and cleaner to let curl handle everything and drop the legacy stuff once it fully works.

Nicolas

On Sun, Aug 06, 2017 at 09:12:16PM +0200, Nicolas Morey-Chaisemartin wrote:

Also 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.
Making it the default will make sure almost everyone is using it and
that there is feature-parity.

Yeah, I had thought that the curl implementation _was_ the default if
you have curl. But we just build it by default, and you have to manually
enable it. So I agree it has not gotten nearly as much testing as I had
thought, and as you found, it diverges from the earlier implementation
in quite a few areas.

So I think we would need to take any deprecation much more slowly than I
had first thought (and your patches in the nearby thread are moving in a
good direction).

-Peff