git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Nicolas Morey-Chaisemartin <nicolas@morey-chaisemartin.com>
Cc: git@vger.kernel.org
Subject: Re: [RFC 0/3] imap-send curl tunnelling support
Date: Thu, 24 Aug 2017 06:53:32 -0700	[thread overview]
Message-ID: <20170824135331.27wtwicjuoiyremx@sigill.intra.peff.net> (raw)
In-Reply-To: <e11d4449-8377-dbd7-3ad5-441baf7446b6@morey-chaisemartin.com>

On Thu, Aug 24, 2017 at 10:00:47AM +0200, Nicolas Morey-Chaisemartin wrote:

> > Yes, I agree. I was hoping when we started this discussion that we were
> > more ready to switch to curl-by-default. But sadly, that isn't close to
> > being the case. But hopefully we can at least end up with logic that
> > lets us use it in the easy cases (no tunneling) and falls back in the
> > harder ones.
>
> I opened a bug upstream and they already fixed this.
> https://github.com/curl/curl/pull/1820

Cool! That's much faster than I had expected. :)

> At least bleeding edge curl user should be able to use this.
> I'm not sure where to go with these patches now.
> 
> 1) There does not seem to be an easy/clean workaround for the lack of socketpair on windows.
> Fidling with a loopback AF_UNIX?AF_LOCAL socket should work but it
> means creating a socket file somewhere which pulls a lot of potential
> issues (where to put it ? Post-mortem cleanup ? Parallel imap-send ?)

Even if you create a non-anonymous socket and connect to both ends, I'm
not sure how it works to pass that to the spawned child. IIRC, our
run_command emulation cannot pass arbitrary descriptors to the child
processes (but I don't know the details of why that is the case, or if
there are windows-specific calls we could be making to work around it).

> 2) The PREAUTH support won't largely be available  for a while (curl,
> release, distro, etc.)
> - If this is the main use case, it does not make much sense to puch
> curl; tunneling support without this. I could push the code and only
> enable the curl tunneling for the next curl release ?
>   Meaning no one (or close to no one) would use this until some later
>   This also means very little testing (apart from mine) until the next
> curl version gets widely available
> - If this is not the main case (or at least the non PREAUTH is
> important enough), it would make sense to get this changes in.
>   But it would probably need some more to code to either fallback to
> legacy mode when curl failed (due to PREAUTH) or detect PREAUTH and
> directly use the legacy mode.

It seems like we should be able to hit the cases that we know work out
of the box, and just hold back the default for the others. Like:

  static int use_curl_auto(void)
  {
  #ifndef USE_CURL_FOR_IMAP_SEND
	/* not built; we cannot use it */
	return 0;
  #else
	if (srvc->tunnel) {
  #if LIBCURL_VERSION < ...
		/* no preauth support */
		return 0;
  #else
		return 1;
  #endif /* LIBCURL_VERSION < ... */
	}
	... other checks go here ...
  #endif /* USE_CURL */
  }

  ...
  int use_curl = -1; /* auto */
  ... set use_curl to 0/1 from --curl/--no-curl command line */
  if (use_curl < 0)
      use_curl = use_curl_auto();

I'm not sure what other cases are left. But over time we'd hope that
use_curl_auto() would shrink to just "return 1", at which point
everybody is using it (and we can drop the fallback code).

-Peff

  reply	other threads:[~2017-08-24 13:53 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-09 14:43 [RFC 0/3] imap-send curl tunnelling support Nicolas Morey-Chaisemartin
2017-08-09 14:46 ` [RFC 1/3] imap-send: move tunnel setup to its own function Nicolas Morey-Chaisemartin
2017-08-09 14:46 ` [RFC 2/3] imap-send: use a socketpair instead of pipe to communicate with the tunnel Nicolas Morey-Chaisemartin
2017-08-09 14:46 ` [RFC 3/3] imap_send: add support for curl over tunnel Nicolas Morey-Chaisemartin
2017-08-15 17:49 ` [RFC 0/3] imap-send curl tunnelling support Nicolas Morey-Chaisemartin
2017-08-15 18:18   ` Stefan Beller
2017-08-16 12:30     ` Johannes Schindelin
2017-08-21  7:27       ` Nicolas Morey-Chaisemartin
2017-08-22 17:10         ` Johannes Sixt
2017-08-22 18:22           ` Nicolas Morey-Chaisemartin
2017-08-23 22:35           ` Johannes Schindelin
2017-08-16  8:34 ` Jeff King
2017-08-21  7:34   ` Nicolas Morey-Chaisemartin
2017-08-23 21:43     ` Jeff King
2017-08-24  8:00       ` Nicolas Morey-Chaisemartin
2017-08-24 13:53         ` Jeff King [this message]
2017-08-24 14:02           ` Daniel Stenberg
2017-08-24 14:30             ` Jeff King
2017-08-24 21:22               ` Daniel Stenberg
2017-08-24 14:15           ` Nicolas Morey-Chaisemartin
2017-08-24 14:28             ` Jeff King
     [not found] ` <7ee8331d-e154-7539-e000-4087406f39fa@suse.de>
2017-08-16  8:39   ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170824135331.27wtwicjuoiyremx@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=nicolas@morey-chaisemartin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).