All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH/RFC] git-imap-send: use libcurl for implementation
@ 2014-08-14 21:46 Bernhard Reiter
  2014-08-19 17:51 ` Junio C Hamano
  0 siblings, 1 reply; 11+ messages in thread
From: Bernhard Reiter @ 2014-08-14 21:46 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, 434599

[-- Attachment #1: Type: text/plain, Size: 5030 bytes --]


Use libcurl's high-level API functions to implement git-imap-send
instead of the previous low-level OpenSSL-based functions.

Since version 7.30.0, libcurl's API has been able to communicate with
IMAP servers. Using those high-level functions instead of the current
ones would reduce imap-send.c by some 1200 lines of code. For now,
the old ones are wrapped in #ifdefs, and the new functions are enabled
by make if curl's version is >= 7.35.0, from which version on curl's
CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been
available.

As I don't have access to that many IMAP servers, I haven't been able to
test the new code with a wide variety of parameter combinations. I did
test both secure and insecure (imaps:// and imap://) connections and
values of "PLAIN" and "LOGIN" for the authMethod.

Signed-off-by: Bernhard Reiter <ockham@raz.or.at>
---
Am 2014-08-13 um 03:59 schrieb Jonathan Nieder:
> Bernhard Reiter wrote:
>> [...]
>
> Wow!  This sounds lovely.  Thanks for working on this.

Well thanks for the friendly welcome and the helpful comments!

I'm attaching a patch where I've applied the fixes you suggested, plus:

* I added the lf_to_crlf conversion to the curl codepath as
communication with another IMAP server I tried was broken without it.

* I added STARTTLS. (That's just the
curl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_ALL);
line)

* I tested (and fixed) authentication, i.e. the auth_method stuff. As
the corresponding CURLOPT_LOGIN_OPTIONS flag has only been available
starting with curl 7.35.0, I've bumped the required version to that.
(Apparently it was possible to achieve the same effect with a different
option in between versions 7.31.0 and 7.34.0 [1], but I haven't found
yet how. Is it worth the effort?)

* I made that file scope imap_folder a member of struct imap_server_conf
(named folder), which makes some things easier.

>> @@ -1417,31 +269,89 @@ int main(int argc, char **argv)
>>  		return 1;
>>  	}
>>
>> +	curl_global_init(CURL_GLOBAL_ALL);
>
> http.c seems to make the same mistake,

Patch at http://permalink.gmane.org/gmane.comp.version-control.git/255221

> [...]
>> +	if (server.tunnel) {
>> +		const char *argv[] = { server.tunnel, NULL };
>> +		struct child_process tunnel = {NULL};
>
> (not about this patch) Could use the child_proccess's internal
> argv_array:
>
> 		struct child_process tunnel = {NULL};
> 		argv_array_push(&tunnel.args, server.tunnel);

Patch at
http://permalink.gmane.org/gmane.comp.version-control.git/255220 (The
patch attached to this mail depends on that one.)

No comments on those patches yet, though.

> (about this patch) Would there be a way to make this part reuse the
> existing code?  The only difference I see is that *srvc has been
> renamed to server, which doesn't seem to be related to the change of
> transport API from OpenSSL to libcurl.
>
> [...]
>> +		curl_socket_t sockfd = tunnel.out; // what about tunnel.in ?
>
> Hmm.  curl expects to get a socket it can send(), recv(), setsockopt(),
> etc on instead of a pair of fds to read() and write().
>
> I wonder why someone would want to use SSL through a tunnel, though.
> Currently it's impossible to get to the SSL codepath when a tunnel
> is active (it's in the 'else' block an 'if (srvc->tunnel)').  If that
> property is preserved, then we should be safe.

Now this turns out to be the one major annoyance left, because we only
have those two fds (actually pipes, right?), and not a socket that we
could pass to curl, so we can't use it to talk to the IMAP server. So if
the tunnel parameter is set, we're stuck with the old hand-written IMAP
handling routines, even with USE_CURL_FOR_IMAP set, meaning I can't wrap
as much in #ifdef...#endif blocks as I'd like. :-( BTW, due to two of
the blocks that I do add I get a compiler warning about the curl handle
remaining possibly unitialized :-/
I've removed the curl specific socket handling routines, as we can't use
them anyway for now.

I've asked about passing two pipes instead of a socket to curl on their
ML [1] as this has even been discussed before [2], but unfortunately,
there doesn't seem to be a solution as of yet. I've also asked on SO
[3], but no answers yet.

> To summarize:
> [...]
>
>  * As soon as you're ready to roll this out to a wider audience of
>    testers, let me know, and we can try to get it into shape for
>    Junio's "next" branch (and hence Debian experimental).

Is this one good enough already?

Bernhard

[1] http://sourceforge.net/p/curl/bugs/1372/
[2] http://curl.haxx.se/mail/lib-2014-08/0102.html
[3] http://curl.haxx.se/mail/lib-2011-05/0102.html
[4]
http://stackoverflow.com/questions/25306264/connect-in-and-out-pipes-to-network-socket

 Documentation/git-imap-send.txt |   3 +-
 INSTALL                         |  15 +++---
 Makefile                        |  16 +++++-
 git.spec.in                     |   5 +-
 imap-send.c                     | 109
+++++++++++++++++++++++++++++++++++++---
 5 files changed, 132 insertions(+), 16 deletions(-)



[-- Attachment #2: 0003-git-imap-send-use-libcurl-for-implementation.patch --]
[-- Type: text/x-patch, Size: 9590 bytes --]

diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
index 875d283..abce2a6 100644
--- a/Documentation/git-imap-send.txt
+++ b/Documentation/git-imap-send.txt
@@ -76,7 +76,8 @@ imap.preformattedHTML::
 
 imap.authMethod::
 	Specify authenticate method for authentication with IMAP server.
-	Current supported method is 'CRAM-MD5' only.
+	If you compiled git with the NO_CURL option or if your curl version is
+	< 7.35.0, the only supported method is 'CRAM-MD5'.
 
 Examples
 ~~~~~~~~
diff --git a/INSTALL b/INSTALL
index 6ec7a24..e2770a0 100644
--- a/INSTALL
+++ b/INSTALL
@@ -108,18 +108,21 @@ Issues of note:
 	  so you might need to install additional packages other than Perl
 	  itself, e.g. Time::HiRes.
 
-	- "openssl" library is used by git-imap-send to use IMAP over SSL.
-	  If you don't need it, use NO_OPENSSL.
+	- "openssl" library is used by git-imap-send to use IMAP over SSL,
+	  unless you're using curl >= 7.35.0, in which case that will be
+	  used. If you don't need git-imap-send, you can use NO_OPENSSL.
 
 	  By default, git uses OpenSSL for SHA1 but it will use its own
 	  library (inspired by Mozilla's) with either NO_OPENSSL or
 	  BLK_SHA1.  Also included is a version optimized for PowerPC
 	  (PPC_SHA1).
 
-	- "libcurl" library is used by git-http-fetch and git-fetch.  You
-	  might also want the "curl" executable for debugging purposes.
-	  If you do not use http:// or https:// repositories, you do not
-	  have to have them (use NO_CURL).
+	- "libcurl" library is used by git-http-fetch, git-fetch, and, if
+	  the curl version >= 7.35.0, for git-imap-send.  You might also
+	  want the "curl" executable for debugging purposes. If you do not
+	  use http:// or https:// repositories, and do not want to put
+	  patches into an IMAP mailbox, you do not have to have them
+	  (use NO_CURL).
 
 	- "expat" library; git-http-push uses it for remote lock
 	  management over DAV.  Similar to "curl" above, this is optional
diff --git a/Makefile b/Makefile
index 2320de5..a5e89ec 100644
--- a/Makefile
+++ b/Makefile
@@ -1120,6 +1120,9 @@ ifdef HAVE_ALLOCA_H
 	BASIC_CFLAGS += -DHAVE_ALLOCA_H
 endif
 
+IMAP_SEND_BUILDDEPS =
+IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
+
 ifdef NO_CURL
 	BASIC_CFLAGS += -DNO_CURL
 	REMOTE_CURL_PRIMARY =
@@ -1154,6 +1157,15 @@ else
 			PROGRAM_OBJS += http-push.o
 		endif
 	endif
+	curl_check := $(shell (echo 072300; curl-config --vernum) 2>/dev/null | sort -r | sed -ne 2p)
+	ifeq "$(curl_check)" "072300"
+		USE_CURL_FOR_IMAP_SEND = YesPlease
+	endif
+	ifdef USE_CURL_FOR_IMAP_SEND
+		BASIC_CFLAGS += -DUSE_CURL_FOR_IMAP_SEND
+		IMAP_SEND_BUILDDEPS = http.o
+		IMAP_SEND_LDFLAGS = $(CURL_LIBCURL)
+	endif
 	ifndef NO_EXPAT
 		ifdef EXPATDIR
 			BASIC_CFLAGS += -I$(EXPATDIR)/include
@@ -2067,9 +2079,9 @@ endif
 git-%$X: %.o GIT-LDFLAGS $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
 
-git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS)
+git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
-		$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
+		$(LIBS) $(IMAP_SEND_LDFLAGS)
 
 git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
diff --git a/git.spec.in b/git.spec.in
index d61d537..9535cc3 100644
--- a/git.spec.in
+++ b/git.spec.in
@@ -8,7 +8,7 @@ License: 	GPL
 Group: 		Development/Tools
 URL: 		http://kernel.org/pub/software/scm/git/
 Source: 	http://kernel.org/pub/software/scm/git/%{name}-%{version}.tar.gz
-BuildRequires:	zlib-devel >= 1.2, openssl-devel, curl-devel, expat-devel, gettext  %{!?_without_docs:, xmlto, asciidoc > 6.0.3}
+BuildRequires:	zlib-devel >= 1.2, openssl-devel, curl-devel >= 7.35.0, expat-devel, gettext  %{!?_without_docs:, xmlto, asciidoc > 6.0.3}
 BuildRoot:	%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 
 Requires:	perl-Git = %{version}-%{release}
@@ -214,6 +214,9 @@ rm -rf $RPM_BUILD_ROOT
 # No files for you!
 
 %changelog
+* Mon Aug 11 2014 Bernhard Reiter <ockham@raz.or.at>
+- Require version >= 7.35.0 of curl-devel for IMAP functions.
+
 * Sun Sep 18 2011 Jakub Narebski <jnareb@gmail.com>
 - Add gitweb manpages to 'gitweb' subpackage
 
diff --git a/imap-send.c b/imap-send.c
index fb01a9c..a45570d 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -22,6 +22,10 @@
  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
+#ifdef USE_CURL_FOR_IMAP_SEND
+#define NO_OPENSSL
+#endif
+
 #include "cache.h"
 #include "credential.h"
 #include "exec_cmd.h"
@@ -29,6 +33,9 @@
 #ifdef NO_OPENSSL
 typedef void *SSL;
 #endif
+#ifdef USE_CURL_FOR_IMAP_SEND
+#include "http.h"
+#endif
 
 static const char imap_send_usage[] = "git imap-send < <mbox>";
 
@@ -44,9 +51,7 @@ __attribute__((format (printf, 1, 2)))
 static void imap_info(const char *, ...);
 __attribute__((format (printf, 1, 2)))
 static void imap_warn(const char *, ...);
-
 static char *next_arg(char **);
-
 __attribute__((format (printf, 3, 4)))
 static int nfsnprintf(char *buf, int blen, const char *fmt, ...);
 
@@ -69,6 +74,7 @@ struct imap_server_conf {
 	char *tunnel;
 	char *host;
 	int port;
+	char *folder;
 	char *user;
 	char *pass;
 	int use_ssl;
@@ -82,6 +88,7 @@ static struct imap_server_conf server = {
 	NULL,	/* tunnel */
 	NULL,	/* host */
 	0,	/* port */
+	NULL,	/* folder */
 	NULL,	/* user */
 	NULL,	/* pass */
 	0,   	/* use_ssl */
@@ -1323,7 +1330,54 @@ static int split_msg(struct strbuf *all_msgs, struct strbuf *msg, int *ofs)
 	return 1;
 }
 
-static char *imap_folder;
+#ifdef USE_CURL_FOR_IMAP_SEND
+static CURL *setup_curl(struct imap_server_conf *srvc)
+{
+	CURL *curl;
+	struct strbuf path = STRBUF_INIT;
+	struct strbuf auth = STRBUF_INIT;
+
+	if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)
+		die("curl_global_init failed");
+
+	curl = curl_easy_init();
+
+	if (!curl)
+		die("curl_easy_init failed");
+
+	curl_easy_setopt(curl, CURLOPT_USERNAME, server.user);
+	curl_easy_setopt(curl, CURLOPT_PASSWORD, server.pass);
+
+	strbuf_addstr(&path, server.host);
+	if (!path.len || path.buf[path.len - 1] != '/')
+		strbuf_addch(&path, '/');
+	strbuf_addstr(&path, server.folder);
+
+	curl_easy_setopt(curl, CURLOPT_URL, path.buf);
+	curl_easy_setopt(curl, CURLOPT_PORT, server.port);
+
+	if (server.auth_method) {
+		strbuf_addstr(&auth, "AUTH=");
+		strbuf_addstr(&auth, server.auth_method);
+		curl_easy_setopt(curl, CURLOPT_LOGIN_OPTIONS, auth.buf);
+	}
+
+	if (server.use_ssl)
+		curl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_ALL);
+
+	curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, server.ssl_verify);
+	curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, server.ssl_verify);
+
+	curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
+
+	curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
+
+	if (Verbose)
+		curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
+
+	return curl;
+}
+#endif
 
 static int git_imap_config(const char *key, const char *val, void *cb)
 {
@@ -1339,7 +1393,7 @@ static int git_imap_config(const char *key, const char *val, void *cb)
 		return config_error_nonbool(key);
 
 	if (!strcmp("folder", key)) {
-		imap_folder = xstrdup(val);
+		server.folder = xstrdup(val);
 	} else if (!strcmp("host", key)) {
 		if (starts_with(val, "imap:"))
 			val += 5;
@@ -1373,6 +1427,11 @@ int main(int argc, char **argv)
 	int r;
 	int total, n = 0;
 	int nongit_ok;
+#ifdef USE_CURL_FOR_IMAP_SEND
+	struct buffer msgbuf = { STRBUF_INIT, 0 };
+	CURL *curl;
+	CURLcode res = CURLE_OK;
+#endif /* #ifndef USE_CURL_FOR_IMAP_SEND */
 
 	git_extract_argv0_path(argv[0]);
 
@@ -1387,7 +1446,7 @@ int main(int argc, char **argv)
 	if (!server.port)
 		server.port = server.use_ssl ? 993 : 143;
 
-	if (!imap_folder) {
+	if (!server.folder) {
 		fprintf(stderr, "no imap store specified\n");
 		return 1;
 	}
@@ -1417,18 +1476,48 @@ int main(int argc, char **argv)
 	}
 
 	/* write it to the imap server */
+
+#ifdef USE_CURL_FOR_IMAP_SEND
+	if (!server.tunnel) {
+		curl = setup_curl(&server);
+		curl_easy_setopt(curl, CURLOPT_READDATA, &msgbuf);
+	} else {
+#endif
 	ctx = imap_open_store(&server);
 	if (!ctx) {
 		fprintf(stderr, "failed to open store\n");
 		return 1;
 	}
+	ctx->name = server.folder;
+#ifdef USE_CURL_FOR_IMAP_SEND
+	}
+#endif
 
 	fprintf(stderr, "sending %d message%s\n", total, (total != 1) ? "s" : "");
-	ctx->name = imap_folder;
 	while (1) {
 		unsigned percent = n * 100 / total;
 
 		fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
+#ifdef USE_CURL_FOR_IMAP_SEND
+		if (!server.tunnel) {
+			int prev_len = msgbuf.buf.len;
+			if (!split_msg(&all_msgs, &msgbuf.buf, &ofs))
+				break;
+			if (server.use_html)
+				wrap_in_html(&msgbuf.buf);
+			lf_to_crlf(&msgbuf.buf);
+
+			curl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE, (curl_off_t)(msgbuf.buf.len-prev_len));
+
+			res = curl_easy_perform(curl);
+
+			if(res != CURLE_OK) {
+				fprintf(stderr, "curl_easy_perform() failed: %s\n",
+						curl_easy_strerror(res));
+				break;
+			}
+		} else {
+#endif
 		if (!split_msg(&all_msgs, &msg, &ofs))
 			break;
 		if (server.use_html)
@@ -1436,11 +1525,19 @@ int main(int argc, char **argv)
 		r = imap_store_msg(ctx, &msg);
 		if (r != DRV_OK)
 			break;
+#ifdef USE_CURL_FOR_IMAP_SEND
+		}
+#endif
 		n++;
 	}
 	fprintf(stderr, "\n");
 
+#ifdef USE_CURL_FOR_IMAP_SEND
+	curl_easy_cleanup(curl);
+	curl_global_cleanup();
+#else
 	imap_close_store(ctx);
+#endif
 
 	return 0;
 }


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH/RFC] git-imap-send: use libcurl for implementation
  2014-08-14 21:46 [PATCH/RFC] git-imap-send: use libcurl for implementation Bernhard Reiter
@ 2014-08-19 17:51 ` Junio C Hamano
  2014-08-25 20:11   ` Bernhard Reiter
  0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2014-08-19 17:51 UTC (permalink / raw)
  To: Bernhard Reiter; +Cc: Jonathan Nieder, git

Bernhard Reiter <ockham@raz.or.at> writes:

> Use libcurl's high-level API functions to implement git-imap-send
> instead of the previous low-level OpenSSL-based functions.
>
> Since version 7.30.0, libcurl's API has been able to communicate with
> IMAP servers. Using those high-level functions instead of the current
> ones would reduce imap-send.c by some 1200 lines of code. For now,
> the old ones are wrapped in #ifdefs, and the new functions are enabled
> by make if curl's version is >= 7.35.0, from which version on curl's
> CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been
> available.
>
> As I don't have access to that many IMAP servers, I haven't been able to
> test the new code with a wide variety of parameter combinations. I did
> test both secure and insecure (imaps:// and imap://) connections and
> values of "PLAIN" and "LOGIN" for the authMethod.
>
> Signed-off-by: Bernhard Reiter <ockham@raz.or.at>
> ---

I think people have missed this one, partly because it was hidden as
an attachment.

>  Documentation/git-imap-send.txt |   3 +-
>  INSTALL                         |  15 +++---
>  Makefile                        |  16 +++++-
>  git.spec.in                     |   5 +-
>  imap-send.c                     | 109
> +++++++++++++++++++++++++++++++++++++---
>  5 files changed, 132 insertions(+), 16 deletions(-)

I smell a line-wrapped patch but it probably is at my receiving end,
forcing the attachment into a flattened form inside my MUA.

Nice to see git.spec.in updated; I like it when I see that the
author paid attention to details.

> diff --git a/imap-send.c b/imap-send.c
> index fb01a9c..a45570d 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -22,6 +22,10 @@
>   *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>   */
>  
> +#ifdef USE_CURL_FOR_IMAP_SEND
> +#define NO_OPENSSL
> +#endif
> +

This looks strange and stands out like a sore thumb.  Do any of our
other sources do this kind of macro tweaking inside C source before
including git-compat-util.h (or its equivalent like cache.h)?

I think what you are trying to do is to change the meaning of
NO_OPENSSL, which merely means "we do not have OpenSSL library and
do not want to link with it", locally to "we may or may not have and
use OpenSSL library elsewhere in Git, but in the code below we do
not want to use the code written to work on top of OpenSSL and
instead use libcurl".  Because of that, you define NO_OPENSSL before
including any of our headers to cause us not to include the headers,
and typedef away SSL, for example.

>  #include "cache.h"
>  #include "credential.h"
>  #include "exec_cmd.h"
> @@ -29,6 +33,9 @@
>  #ifdef NO_OPENSSL
>  typedef void *SSL;
>  #endif
> +#ifdef USE_CURL_FOR_IMAP_SEND
> +#include "http.h"
> +#endif

But does it have to be that way?  For one thing, doing it this way,
the user has to make a compilation-time choice, but if you separate
these compilation time macro into two, one for "can we even link
with and use OpenSSL?" (which is what NO_OPENSSL is about) and
another for "do we want an ability to talk to imap via libcurl?",
wouldn't it make it possible for you to switch between them at
runtime (e.g. you might want to go over the direct connection when
tunneling, while letting libcurl do the heavy lifting in
non-tunneled operation)?

> @@ -44,9 +51,7 @@ __attribute__((format (printf, 1, 2)))
>  static void imap_info(const char *, ...);
>  __attribute__((format (printf, 1, 2)))
>  static void imap_warn(const char *, ...);
> -
>  static char *next_arg(char **);
> -
>  __attribute__((format (printf, 3, 4)))
>  static int nfsnprintf(char *buf, int blen, const char *fmt, ...);
>  
> @@ -69,6 +74,7 @@ struct imap_server_conf {
>  	char *tunnel;
>  	char *host;
>  	int port;
> +	char *folder;
>  	char *user;
>  	char *pass;
>  	int use_ssl;
> @@ -82,6 +88,7 @@ static struct imap_server_conf server = {
>  	NULL,	/* tunnel */
>  	NULL,	/* host */
>  	0,	/* port */
> +	NULL,	/* folder */
>  	NULL,	/* user */
>  	NULL,	/* pass */
>  	0,   	/* use_ssl */
> @@ -1323,7 +1330,54 @@ static int split_msg(struct strbuf *all_msgs, struct strbuf *msg, int *ofs)
>  	return 1;
>  }
>  
> -static char *imap_folder;

All of the above seem to have meant well, but these changes are not
about talking with IMAP servers via libcurl.  We'd prefer to see
changes like these as preliminary clean-up before the main change as
separate patch(es).

> @@ -1417,18 +1476,48 @@ int main(int argc, char **argv)
>  	}
>  
>  	/* write it to the imap server */
> +
> +#ifdef USE_CURL_FOR_IMAP_SEND
> +	if (!server.tunnel) {
> +		curl = setup_curl(&server);
> +		curl_easy_setopt(curl, CURLOPT_READDATA, &msgbuf);
> +	} else {
> +#endif
>  	ctx = imap_open_store(&server);
>  	if (!ctx) {
>  		fprintf(stderr, "failed to open store\n");
>  		return 1;
>  	}
> +	ctx->name = server.folder;
> +#ifdef USE_CURL_FOR_IMAP_SEND
> +	}
> +#endif
>  
>  	fprintf(stderr, "sending %d message%s\n", total, (total != 1) ? "s" : "");
> -	ctx->name = imap_folder;
>  	while (1) {
>  		unsigned percent = n * 100 / total;
>  
>  		fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
> +#ifdef USE_CURL_FOR_IMAP_SEND
> +		if (!server.tunnel) {
> ...
> +			}
> +		} else {
> +#endif
>  		if (!split_msg(&all_msgs, &msg, &ofs))
>  			break;
>  		if (server.use_html)
> @@ -1436,11 +1525,19 @@ int main(int argc, char **argv)
>  		r = imap_store_msg(ctx, &msg);
>  		if (r != DRV_OK)
>  			break;
> +#ifdef USE_CURL_FOR_IMAP_SEND
> +		}
> +#endif
>  		n++;
>  	}
>  	fprintf(stderr, "\n");
>  
> +#ifdef USE_CURL_FOR_IMAP_SEND
> +	curl_easy_cleanup(curl);
> +	curl_global_cleanup();
> +#else
>  	imap_close_store(ctx);
> +#endif
>  
>  	return 0;
>  }

Ugly.  Can we do this with less #ifdef/#else/#endif in the primary
code path?

If we were to keep these two modes as a choice the users have to
make at the compilation time, that is.

Thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH/RFC] git-imap-send: use libcurl for implementation
  2014-08-19 17:51 ` Junio C Hamano
@ 2014-08-25 20:11   ` Bernhard Reiter
  2014-08-25 21:08     ` Junio C Hamano
  0 siblings, 1 reply; 11+ messages in thread
From: Bernhard Reiter @ 2014-08-25 20:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jonathan Nieder, git

Am 2014-08-19 um 19:51 schrieb Junio C Hamano:
> This looks strange and stands out like a sore thumb.  Do any of our
> other sources do this kind of macro tweaking inside C source before
> including git-compat-util.h (or its equivalent like cache.h)?

I haven't checked, but I agree that it's desirable to avoid.

> I think what you are trying to do is to change the meaning of
> NO_OPENSSL, which merely means "we do not have OpenSSL library and
> do not want to link with it", locally to "we may or may not have and
> use OpenSSL library elsewhere in Git, but in the code below we do
> not want to use the code written to work on top of OpenSSL and
> instead use libcurl".  

"...and we don't want to link to OpenSSL in that case." Yeah.

> Because of that, you define NO_OPENSSL before
> including any of our headers to cause us not to include the headers,
> and typedef away SSL, for example.

The SSL un-typedef'ing was there before, but it's true that I'm defining
NO_OPENSSL on the very top so the included headers don't require OpenSSL
(and so we don't have to link to it later).

>>  #include "cache.h"
>>  #include "credential.h"
>>  #include "exec_cmd.h"
>> @@ -29,6 +33,9 @@
>>  #ifdef NO_OPENSSL
>>  typedef void *SSL;
>>  #endif
>> +#ifdef USE_CURL_FOR_IMAP_SEND
>> +#include "http.h"
>> +#endif
> 
> But does it have to be that way?  For one thing, doing it this way,
> the user has to make a compilation-time choice, but if you separate
> these compilation time macro into two, one for "can we even link
> with and use OpenSSL?" (which is what NO_OPENSSL is about) and
> another for "do we want an ability to talk to imap via libcurl?",
> wouldn't it make it possible for you to switch between them at
> runtime (e.g. you might want to go over the direct connection when
> tunneling, while letting libcurl do the heavy lifting in
> non-tunneled operation)?

Yeah, but I still need to wrap the non-tunneled operation in #ifdefs in
case we don't USE_CURL_FOR_IMAP_SEND in which case we fall back to the
handwritten IMAP code, don't I?

Maybe I'm not getting you entirely right, but if I don't typedef
NO_OPENSSL if USE_CURL_FOR_IMAP_SEND is defined, I don't see any way to
not link to OpenSSL, even if it's not required. I'm including a slightly
modified patch which does that (hoping that I've finally managed to send
a usable patch). Sorry it's nothing breathtakingly better than before,
even after giving this some thought I didn't arrive at a very elegant
new solution... (see below for more on that)

>> @@ -1417,18 +1476,48 @@ int main(int argc, char **argv)
>>  	}
>>  
>>  	/* write it to the imap server */
>> +
>> +#ifdef USE_CURL_FOR_IMAP_SEND
>> +	if (!server.tunnel) {
>> +		curl = setup_curl(&server);
>> +		curl_easy_setopt(curl, CURLOPT_READDATA, &msgbuf);
>> +	} else {
>> +#endif
>>  	ctx = imap_open_store(&server);
>>  	if (!ctx) {
>>  		fprintf(stderr, "failed to open store\n");
>>  		return 1;
>>  	}
>> +	ctx->name = server.folder;
>> +#ifdef USE_CURL_FOR_IMAP_SEND
>> +	}
>> +#endif
>>  
>>  	fprintf(stderr, "sending %d message%s\n", total, (total != 1) ? "s" : "");
>> -	ctx->name = imap_folder;
>>  	while (1) {
>>  		unsigned percent = n * 100 / total;
>>  
>>  		fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
>> +#ifdef USE_CURL_FOR_IMAP_SEND
>> +		if (!server.tunnel) {
>> ...
>> +			}
>> +		} else {
>> +#endif
>>  		if (!split_msg(&all_msgs, &msg, &ofs))
>>  			break;
>>  		if (server.use_html)
>> @@ -1436,11 +1525,19 @@ int main(int argc, char **argv)
>>  		r = imap_store_msg(ctx, &msg);
>>  		if (r != DRV_OK)
>>  			break;
>> +#ifdef USE_CURL_FOR_IMAP_SEND
>> +		}
>> +#endif
>>  		n++;
>>  	}
>>  	fprintf(stderr, "\n");
>>  
>> +#ifdef USE_CURL_FOR_IMAP_SEND
>> +	curl_easy_cleanup(curl);
>> +	curl_global_cleanup();
>> +#else
>>  	imap_close_store(ctx);
>> +#endif
>>  
>>  	return 0;
>>  }
> 
> Ugly.  Can we do this with less #ifdef/#else/#endif in the primary
> code path?

It is ugly, but as much as I'd love to put e.g.

+#ifdef USE_CURL_FOR_IMAP_SEND
+	if (!server.tunnel) {
+		curl = setup_curl(&server);

etc into imap_open_store (and similarly for imap_store_msg etc), I don't
see any easy way to do it; imap_open_store's return type would still be
struct imap_store* in the non-tunneling case, and CURL* otherwise.

So the best I can come up with here is merging some of the #ifdef
blocks, but that means duplicating the code that applies in both cases.
But that isn't any better, is it?

> If we were to keep these two modes as a choice the users have to
> make at the compilation time, that is.

As stated above, I'm not sure how to do entirely without at least those
two compile time switches (NO_OPENSSL and USE_CURL_FOR_IMAP_SEND).

Sorry, perhaps I missed something obvious; grateful for any hints on how
to do it better.

Bernhard

 Documentation/git-imap-send.txt |  3 +-
 INSTALL                         | 15 ++++---
 Makefile                        | 16 ++++++-
 git.spec.in                     |  5 ++-
 imap-send.c                     | 96 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 125 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt
index 7d991d9..9d244c4 100644
--- a/Documentation/git-imap-send.txt
+++ b/Documentation/git-imap-send.txt
@@ -75,7 +75,8 @@ imap.preformattedHTML::
 
 imap.authMethod::
 	Specify authenticate method for authentication with IMAP server.
-	Current supported method is 'CRAM-MD5' only. If this is not set
+	If you compiled git with the NO_CURL option or if your curl version is
+	< 7.35.0, the only supported method is 'CRAM-MD5'. If this is not set
 	then 'git imap-send' uses the basic IMAP plaintext LOGIN command.
 
 Examples
diff --git a/INSTALL b/INSTALL
index 6ec7a24..e2770a0 100644
--- a/INSTALL
+++ b/INSTALL
@@ -108,18 +108,21 @@ Issues of note:
 	  so you might need to install additional packages other than Perl
 	  itself, e.g. Time::HiRes.
 
-	- "openssl" library is used by git-imap-send to use IMAP over SSL.
-	  If you don't need it, use NO_OPENSSL.
+	- "openssl" library is used by git-imap-send to use IMAP over SSL,
+	  unless you're using curl >= 7.35.0, in which case that will be
+	  used. If you don't need git-imap-send, you can use NO_OPENSSL.
 
 	  By default, git uses OpenSSL for SHA1 but it will use its own
 	  library (inspired by Mozilla's) with either NO_OPENSSL or
 	  BLK_SHA1.  Also included is a version optimized for PowerPC
 	  (PPC_SHA1).
 
-	- "libcurl" library is used by git-http-fetch and git-fetch.  You
-	  might also want the "curl" executable for debugging purposes.
-	  If you do not use http:// or https:// repositories, you do not
-	  have to have them (use NO_CURL).
+	- "libcurl" library is used by git-http-fetch, git-fetch, and, if
+	  the curl version >= 7.35.0, for git-imap-send.  You might also
+	  want the "curl" executable for debugging purposes. If you do not
+	  use http:// or https:// repositories, and do not want to put
+	  patches into an IMAP mailbox, you do not have to have them
+	  (use NO_CURL).
 
 	- "expat" library; git-http-push uses it for remote lock
 	  management over DAV.  Similar to "curl" above, this is optional
diff --git a/Makefile b/Makefile
index 237bc05..c08963c 100644
--- a/Makefile
+++ b/Makefile
@@ -1133,6 +1133,9 @@ ifdef HAVE_ALLOCA_H
 	BASIC_CFLAGS += -DHAVE_ALLOCA_H
 endif
 
+IMAP_SEND_BUILDDEPS =
+IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
+
 ifdef NO_CURL
 	BASIC_CFLAGS += -DNO_CURL
 	REMOTE_CURL_PRIMARY =
@@ -1167,6 +1170,15 @@ else
 			PROGRAM_OBJS += http-push.o
 		endif
 	endif
+	curl_check := $(shell (echo 072300; curl-config --vernum) 2>/dev/null | sort -r | sed -ne 2p)
+	ifeq "$(curl_check)" "072300"
+		USE_CURL_FOR_IMAP_SEND = YesPlease
+	endif
+	ifdef USE_CURL_FOR_IMAP_SEND
+		BASIC_CFLAGS += -DUSE_CURL_FOR_IMAP_SEND
+		IMAP_SEND_BUILDDEPS = http.o
+		IMAP_SEND_LDFLAGS += $(CURL_LIBCURL)
+	endif
 	ifndef NO_EXPAT
 		ifdef EXPATDIR
 			BASIC_CFLAGS += -I$(EXPATDIR)/include
@@ -2084,9 +2096,9 @@ endif
 git-%$X: %.o GIT-LDFLAGS $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
 
-git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS)
+git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
-		$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
+		$(LIBS) $(IMAP_SEND_LDFLAGS)
 
 git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
diff --git a/git.spec.in b/git.spec.in
index d61d537..9535cc3 100644
--- a/git.spec.in
+++ b/git.spec.in
@@ -8,7 +8,7 @@ License: 	GPL
 Group: 		Development/Tools
 URL: 		http://kernel.org/pub/software/scm/git/
 Source: 	http://kernel.org/pub/software/scm/git/%{name}-%{version}.tar.gz
-BuildRequires:	zlib-devel >= 1.2, openssl-devel, curl-devel, expat-devel, gettext  %{!?_without_docs:, xmlto, asciidoc > 6.0.3}
+BuildRequires:	zlib-devel >= 1.2, openssl-devel, curl-devel >= 7.35.0, expat-devel, gettext  %{!?_without_docs:, xmlto, asciidoc > 6.0.3}
 BuildRoot:	%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 
 Requires:	perl-Git = %{version}-%{release}
@@ -214,6 +214,9 @@ rm -rf $RPM_BUILD_ROOT
 # No files for you!
 
 %changelog
+* Mon Aug 11 2014 Bernhard Reiter <ockham@raz.or.at>
+- Require version >= 7.35.0 of curl-devel for IMAP functions.
+
 * Sun Sep 18 2011 Jakub Narebski <jnareb@gmail.com>
 - Add gitweb manpages to 'gitweb' subpackage
 
diff --git a/imap-send.c b/imap-send.c
index ad330a6..3343429 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -29,6 +29,9 @@
 #ifdef NO_OPENSSL
 typedef void *SSL;
 #endif
+#ifdef USE_CURL_FOR_IMAP_SEND
+#include "http.h"
+#endif
 
 static const char imap_send_usage[] = "git imap-send < <mbox>";
 
@@ -1307,6 +1310,55 @@ static int split_msg(struct strbuf *all_msgs, struct strbuf *msg, int *ofs)
 	return 1;
 }
 
+#ifdef USE_CURL_FOR_IMAP_SEND
+static CURL *setup_curl(struct imap_server_conf *srvc)
+{
+	CURL *curl;
+	struct strbuf path = STRBUF_INIT;
+	struct strbuf auth = STRBUF_INIT;
+
+	if (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)
+		die("curl_global_init failed");
+
+	curl = curl_easy_init();
+
+	if (!curl)
+		die("curl_easy_init failed");
+
+	curl_easy_setopt(curl, CURLOPT_USERNAME, server.user);
+	curl_easy_setopt(curl, CURLOPT_PASSWORD, server.pass);
+
+	strbuf_addstr(&path, server.host);
+	if (!path.len || path.buf[path.len - 1] != '/')
+		strbuf_addch(&path, '/');
+	strbuf_addstr(&path, server.folder);
+
+	curl_easy_setopt(curl, CURLOPT_URL, path.buf);
+	curl_easy_setopt(curl, CURLOPT_PORT, server.port);
+
+	if (server.auth_method) {
+		strbuf_addstr(&auth, "AUTH=");
+		strbuf_addstr(&auth, server.auth_method);
+		curl_easy_setopt(curl, CURLOPT_LOGIN_OPTIONS, auth.buf);
+	}
+
+	if (server.use_ssl)
+		curl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_ALL);
+
+	curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, server.ssl_verify);
+	curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, server.ssl_verify);
+
+	curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
+
+	curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
+
+	if (Verbose)
+		curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
+
+	return curl;
+}
+#endif
+
 static void git_imap_config(void)
 {
 	const char *val = NULL;
@@ -1347,6 +1399,11 @@ int main(int argc, char **argv)
 	int r;
 	int total, n = 0;
 	int nongit_ok;
+#ifdef USE_CURL_FOR_IMAP_SEND
+	struct buffer msgbuf = { STRBUF_INIT, 0 };
+	CURL *curl;
+	CURLcode res = CURLE_OK;
+#endif /* #ifndef USE_CURL_FOR_IMAP_SEND */
 
 	git_extract_argv0_path(argv[0]);
 
@@ -1391,17 +1448,48 @@ int main(int argc, char **argv)
 	}
 
 	/* write it to the imap server */
+
+#ifdef USE_CURL_FOR_IMAP_SEND
+	if (!server.tunnel) {
+		curl = setup_curl(&server);
+		curl_easy_setopt(curl, CURLOPT_READDATA, &msgbuf);
+	} else {
+#endif
 	ctx = imap_open_store(&server, server.folder);
 	if (!ctx) {
 		fprintf(stderr, "failed to open store\n");
 		return 1;
 	}
+	ctx->name = server.folder;
+#ifdef USE_CURL_FOR_IMAP_SEND
+	}
+#endif
 
 	fprintf(stderr, "sending %d message%s\n", total, (total != 1) ? "s" : "");
 	while (1) {
 		unsigned percent = n * 100 / total;
 
 		fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
+#ifdef USE_CURL_FOR_IMAP_SEND
+		if (!server.tunnel) {
+			int prev_len = msgbuf.buf.len;
+			if (!split_msg(&all_msgs, &msgbuf.buf, &ofs))
+				break;
+			if (server.use_html)
+				wrap_in_html(&msgbuf.buf);
+			lf_to_crlf(&msgbuf.buf);
+
+			curl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE, (curl_off_t)(msgbuf.buf.len-prev_len));
+
+			res = curl_easy_perform(curl);
+
+			if(res != CURLE_OK) {
+				fprintf(stderr, "curl_easy_perform() failed: %s\n",
+						curl_easy_strerror(res));
+				break;
+			}
+		} else {
+#endif
 		if (!split_msg(&all_msgs, &msg, &ofs))
 			break;
 		if (server.use_html)
@@ -1409,11 +1497,19 @@ int main(int argc, char **argv)
 		r = imap_store_msg(ctx, &msg);
 		if (r != DRV_OK)
 			break;
+#ifdef USE_CURL_FOR_IMAP_SEND
+		}
+#endif
 		n++;
 	}
 	fprintf(stderr, "\n");
 
+#ifdef USE_CURL_FOR_IMAP_SEND
+	curl_easy_cleanup(curl);
+	curl_global_cleanup();
+#else
 	imap_close_store(ctx);
+#endif
 
 	return 0;
 }
-- 
2.1.0.372.g89e5a25

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH/RFC] git-imap-send: use libcurl for implementation
  2014-08-25 20:11   ` Bernhard Reiter
@ 2014-08-25 21:08     ` Junio C Hamano
  0 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2014-08-25 21:08 UTC (permalink / raw)
  To: Bernhard Reiter; +Cc: Jonathan Nieder, git

Bernhard Reiter <ockham@raz.or.at> writes:

> Yeah, but I still need to wrap the non-tunneled operation in #ifdefs in
> case we don't USE_CURL_FOR_IMAP_SEND in which case we fall back to the
> handwritten IMAP code, don't I?

We do not mind multiple implementations of the same helper function
that are guarded with #ifdef/#endif, and we do use that style quite
a lot.  Would it help?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH/RFC] git-imap-send: use libcurl for implementation
  2014-08-19 11:14         ` Bernhard Reiter
@ 2014-08-19 17:13           ` Junio C Hamano
  0 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2014-08-19 17:13 UTC (permalink / raw)
  To: Bernhard Reiter; +Cc: Jeff King, Jonathan Nieder, git, 434599

Bernhard Reiter <ockham@raz.or.at> writes:

> Am 2014-08-17 um 20:42 schrieb Jeff King:
>> [...]
>> 
>>>> I'm not sure I understand this comment. Even if SSL is not in use,
>>>> wouldn't we be passing a regular pipe to curl, which would break?
>>>
>>> Yeah, we can't do that, and thus would have to keep the handwritten IMAP
>>> implementation just for the tunnel case (allowing to drop only the
>>> OpenSSL specific stuff), see my other email:
>>> http://www.mail-archive.com/git@vger.kernel.org/msg56791.html (the
>>> relevant part is pretty far down at the bottom).
>> 
>> I'd really love it if we could make this work with tunnels and
>> eventually get rid of the hand-written imap code entirely. I agree with
>> Jonathan that we probably need to keep it around a bit for people on
>> older curl, but dropping it is a good goal in the long run. That code
>> was forked from the isync project, but mangled enough that we could not
>> take bug fixes from upstream. As not many people use imap-send, I
>> suspect it is largely unmaintained and the source of many lurking
>> bugs[1]. Replacing it with curl's maintained implementation is probably
>> a good step.

I would agree with s/a good step/a good goal/ ;-)

> I'll work on this as soon as I find some time, but as that will include
> changes to run-command.c (and possibly other files?), I'd like to cover
> that in a commit of its own. Do you guys think the current patch [1] is
> good enough for "official" submission already?

My impression from reading the discussion in this thread has been
that the patch that started this thread would break the tunneling
code, i.e. is not there yet.  Or did you mean some other patch?

The other patch $gmane/255403 from you looked good and I think I
already have a copy queued on 'pu' as f9dc5d65 (git-imap-send:
simplify tunnel construction, 2014-08-13).

Thanks.


[References]

*$gmane/255403*
    http://thread.gmane.org/gmane.comp.version-control.git/255220/focus=255403

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH/RFC] git-imap-send: use libcurl for implementation
  2014-08-17 18:42       ` Jeff King
@ 2014-08-19 11:14         ` Bernhard Reiter
  2014-08-19 17:13           ` Junio C Hamano
  0 siblings, 1 reply; 11+ messages in thread
From: Bernhard Reiter @ 2014-08-19 11:14 UTC (permalink / raw)
  To: Jeff King; +Cc: Jonathan Nieder, git, 434599

Am 2014-08-17 um 20:42 schrieb Jeff King:
> [...]
> 
>>> I'm not sure I understand this comment. Even if SSL is not in use,
>>> wouldn't we be passing a regular pipe to curl, which would break?
>>
>> Yeah, we can't do that, and thus would have to keep the handwritten IMAP
>> implementation just for the tunnel case (allowing to drop only the
>> OpenSSL specific stuff), see my other email:
>> http://www.mail-archive.com/git@vger.kernel.org/msg56791.html (the
>> relevant part is pretty far down at the bottom).
> 
> I'd really love it if we could make this work with tunnels and
> eventually get rid of the hand-written imap code entirely. I agree with
> Jonathan that we probably need to keep it around a bit for people on
> older curl, but dropping it is a good goal in the long run. That code
> was forked from the isync project, but mangled enough that we could not
> take bug fixes from upstream. As not many people use imap-send, I
> suspect it is largely unmaintained and the source of many lurking
> bugs[1]. Replacing it with curl's maintained implementation is probably
> a good step.

I'll work on this as soon as I find some time, but as that will include
changes to run-command.c (and possibly other files?), I'd like to cover
that in a commit of its own. Do you guys think the current patch [1] is
good enough for "official" submission already? If so, do I need some
sort of official review? Documentation/SubmittingPatches says I'm only
supposed to direct it to Junio after the list "reaches consensus", so
I'm wondering how to get there... :-)

Bernhard

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH/RFC] git-imap-send: use libcurl for implementation
  2014-08-17 12:56     ` Bernhard Reiter
@ 2014-08-17 18:42       ` Jeff King
  2014-08-19 11:14         ` Bernhard Reiter
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff King @ 2014-08-17 18:42 UTC (permalink / raw)
  To: Bernhard Reiter; +Cc: Jonathan Nieder, git, 434599

On Sun, Aug 17, 2014 at 02:56:10PM +0200, Bernhard Reiter wrote:

> > I'm not sure if that would cause problems on Windows,
> > though.
> 
> Apparently socketpair is not available there. Googling "socketpair
> windows" yields, among a lot of other useful resources, the following
> relatively actively maintained ~150 LOC, BSD-3-clause-licensed,
> implementation:
> 
> https://github.com/ncm/selectable-socketpair
> 
> That license is GPL compatible, so should we consider including that
> implementation with git? That's the kind of stuff that goes to
> compat/win32, right?

Thanks for researching. Sticking that in compat/ would be our usual
strategy, yes.

> > I'm not sure I understand this comment. Even if SSL is not in use,
> > wouldn't we be passing a regular pipe to curl, which would break?
> 
> Yeah, we can't do that, and thus would have to keep the handwritten IMAP
> implementation just for the tunnel case (allowing to drop only the
> OpenSSL specific stuff), see my other email:
> http://www.mail-archive.com/git@vger.kernel.org/msg56791.html (the
> relevant part is pretty far down at the bottom).

I'd really love it if we could make this work with tunnels and
eventually get rid of the hand-written imap code entirely. I agree with
Jonathan that we probably need to keep it around a bit for people on
older curl, but dropping it is a good goal in the long run. That code
was forked from the isync project, but mangled enough that we could not
take bug fixes from upstream. As not many people use imap-send, I
suspect it is largely unmaintained and the source of many lurking
bugs[1]. Replacing it with curl's maintained implementation is probably
a good step.

-Peff

[1] That's my somewhat subjective opinion, but I feel like I have seen
    several bugs reported in imap-send that had literally been there for
    years. And having worked on IMAP implementations in a past life, I
    know there are many dark corners of the protocol that vary from
    server to server.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH/RFC] git-imap-send: use libcurl for implementation
  2014-08-17  8:30   ` Jeff King
@ 2014-08-17 12:56     ` Bernhard Reiter
  2014-08-17 18:42       ` Jeff King
  0 siblings, 1 reply; 11+ messages in thread
From: Bernhard Reiter @ 2014-08-17 12:56 UTC (permalink / raw)
  To: Jeff King, Jonathan Nieder; +Cc: git, 434599

Am 2014-08-17 um 10:30 schrieb Jeff King:
> On Tue, Aug 12, 2014 at 06:59:17PM -0700, Jonathan Nieder wrote:
> 
>>> +		curl_socket_t sockfd = tunnel.out; // what about tunnel.in ?
>>
>> Hmm.  curl expects to get a socket it can send(), recv(), setsockopt(),
>> etc on instead of a pair of fds to read() and write().
> 
> I wonder if we could teach run_command to optionally use socketpair()
> instead of pipe(). 

That sounds like a good idea to me.

> I'm not sure if that would cause problems on Windows,
> though.

Apparently socketpair is not available there. Googling "socketpair
windows" yields, among a lot of other useful resources, the following
relatively actively maintained ~150 LOC, BSD-3-clause-licensed,
implementation:

https://github.com/ncm/selectable-socketpair

That license is GPL compatible, so should we consider including that
implementation with git? That's the kind of stuff that goes to
compat/win32, right?

One thing to consider: seems like socketpair() gives AF_LOCAL sockets,
so I've asked [1] on the curl ML if that would work or if libcurl needs
an AF_INET one.

>> I wonder why someone would want to use SSL through a tunnel, though.
>> Currently it's impossible to get to the SSL codepath when a tunnel is
>> active (it's in the 'else' block an 'if (srvc->tunnel)').  If that
>> property is preserved, then we should be safe.
> 
> I'm not sure I understand this comment. Even if SSL is not in use,
> wouldn't we be passing a regular pipe to curl, which would break?

Yeah, we can't do that, and thus would have to keep the handwritten IMAP
implementation just for the tunnel case (allowing to drop only the
OpenSSL specific stuff), see my other email:
http://www.mail-archive.com/git@vger.kernel.org/msg56791.html (the
relevant part is pretty far down at the bottom).

Bernhard

[1] http://curl.haxx.se/mail/lib-2014-08/0131.html

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH/RFC] git-imap-send: use libcurl for implementation
  2014-08-13  1:59 ` Jonathan Nieder
@ 2014-08-17  8:30   ` Jeff King
  2014-08-17 12:56     ` Bernhard Reiter
  0 siblings, 1 reply; 11+ messages in thread
From: Jeff King @ 2014-08-17  8:30 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Bernhard Reiter, git, 434599

On Tue, Aug 12, 2014 at 06:59:17PM -0700, Jonathan Nieder wrote:

> > +		curl_socket_t sockfd = tunnel.out; // what about tunnel.in ?
> 
> Hmm.  curl expects to get a socket it can send(), recv(), setsockopt(),
> etc on instead of a pair of fds to read() and write().

I wonder if we could teach run_command to optionally use socketpair()
instead of pipe(). I'm not sure if that would cause problems on Windows,
though.

> I wonder why someone would want to use SSL through a tunnel, though.
> Currently it's impossible to get to the SSL codepath when a tunnel is
> active (it's in the 'else' block an 'if (srvc->tunnel)').  If that
> property is preserved, then we should be safe.

I'm not sure I understand this comment. Even if SSL is not in use,
wouldn't we be passing a regular pipe to curl, which would break?

-Peff

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH/RFC] git-imap-send: use libcurl for implementation
  2014-08-12 21:50 Bernhard Reiter
@ 2014-08-13  1:59 ` Jonathan Nieder
  2014-08-17  8:30   ` Jeff King
  0 siblings, 1 reply; 11+ messages in thread
From: Jonathan Nieder @ 2014-08-13  1:59 UTC (permalink / raw)
  To: Bernhard Reiter; +Cc: git, 434599

Bernhard Reiter wrote:

> Use libcurl's high-level API functions to implement git-imap-send
> instead of the previous low-level OpenSSL-based functions.

Wow!  This sounds lovely.  Thanks for working on this.

[...]
> Since version 7.30.0, libcurl's API has been able to communicate with
> IMAP servers. Using those high-level functions instead of the current
> ones reduces imap-send.c by some 1200 lines of code.
>
> As I don't have access to that many IMAP servers, I haven't been able to
> test a variety of parameter combinations. I did test both secure and
> insecure (imaps:// and imap://) connections -- this e-mail was generated
> that way -- but could e.g. neither test the authMethod nor the tunnel
> parameter.

The above two paragraphs belong in the commit message, too, since
they'll be just as useful for someone looking back through the history
as for someone reading the patch on-list today.

[...]
> --- a/INSTALL
> +++ b/INSTALL
[...]
> -	- "libcurl" library is used by git-http-fetch and git-fetch.  You
> +	- "libcurl" library is used by git-http-fetch, git-fetch, and
> +	  git-impap-send.  You might also want the "curl" executable for

Typo: s/impap-send/imap-send/

> --- a/Makefile
> +++ b/Makefile
> @@ -2067,9 +2067,9 @@ endif
>  git-%$X: %.o GIT-LDFLAGS $(GITLIBS)
>  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
>  
> -git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS)
> +git-imap-send$X: imap-send.o http.o GIT-LDFLAGS $(GITLIBS)
>  	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
> -		$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
> +		$(LIBS) $(CURL_LIBCURL)

7.30.0 is only ~1 year old.  Does this mean users would need to update
curl in order to build imap-send?

For example, Debian 7.6 ships curl 7.26.0 and CentOS 7 has curl 7.29.0.

Ideally this could be done as an optional feature:

 1. Introduce a USE_CURL_FOR_IMAP_SEND makefile variable to take
    advantage of the nice new API.

 2. (optional) Use the curl_check makefile variable to turn on
    USE_CURL_FOR_IMAP_SEND automatically when appropriate.

 3. In a few years, when everyone has upgraded, we could simplify by
    getting rid of the USE_OPENSSL_FOR_IMAP_SEND code path.

What do you think?

[...]
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -22,47 +22,13 @@
>   *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>   */
>  
> -#include "cache.h"

Usual style is to start with a #include of "cache.h" or
"git-compat-util.h".  "http.h" including cache.h for itself was an
old mistake.  (I'll reply with a patch to fix that.)

[...]
> +#include <curl/curl.h>

http.h already #includes this.  Do you use other helpers from
http.h/http.c or do you use libcurl directly?  (Just curious.)

Some style nits:

[...]
> +static curl_socket_t opensocket(void *clientp, curlsocktype purpose,
> +								struct curl_sockaddr *address)

Long line.  Do you have ts=4?  (Git uses 8-space tabs.  There's some
emacs magic in Documentation/CodingGuidelines.  It should be possible
to add similar hints there for other editors if they don't do the
right thing by default.)

> +{
> +	curl_socket_t sockfd;
> +	(void)purpose;
> +	(void)address;

Elsewhere git lets unused parameters be.  The unused parameter warning
is too noisy in callback-heavy code (e.g., for_each_ref) so we don't
turn it on.

> +	sockfd = *(curl_socket_t *)clientp;
> +	/* the actual externally set socket is passed in via the OPENSOCKETDATA
> +	   option */

(style nit) Comments in git look like this:

	/*
	 * The actual, externally set socket is passed in via the
	 * OPENSOCKETDATA option.
	 */
	return sockfd;

[...]
> +static int sockopt_callback(void *clientp, curl_socket_t curlfd,
> +							curlsocktype purpose)
> +{
> +	/* This return code was added in libcurl 7.21.5 */
> +	return CURL_SOCKOPT_ALREADY_CONNECTED;

I'd drop the comment, unless there's some subtlety involved.  (E.g.,
is there some other return code that would be more appropriate but was
introudced later or something?)

[...]
> @@ -1368,12 +218,14 @@ static int git_imap_config(const char *key, const char *val, void *cb)
>  int main(int argc, char **argv)
>  {
>  	struct strbuf all_msgs = STRBUF_INIT;
> -	struct strbuf msg = STRBUF_INIT;
> +	struct buffer msg = { STRBUF_INIT, 0 };

Ah, ok --- we do use http.c stuff.

[...]
> +	char path[8192];
> +	int pathlen;

I realize the old code only had 8192 for the IMAP command buffer,
but could this be a strbuf now, or is there some underlying limit
somewhere else?

[...]
> @@ -1417,31 +269,89 @@ int main(int argc, char **argv)
>  		return 1;
>  	}
>  
> +	curl_global_init(CURL_GLOBAL_ALL);

http.c seems to make the same mistake, but should the return value
from this be checked?

> -	/* write it to the imap server */
> -	ctx = imap_open_store(&server);
> -	if (!ctx) {
> -		fprintf(stderr, "failed to open store\n");
> +	curl = curl_easy_init();
> +
> +	if (!curl) {
> +		fprintf(stderr, "failed to init curl\n");
>  		return 1;

Could do

		die("failed to init curl");

for more consistent message format and exit codes (128 for internal
errors, with an error message starting with 'fatal: ').

[...]
> +	if (ends_with(server.host, "/"))
> +		pathlen = snprintf (path, sizeof(path), "%s%s", server.host, imap_folder);
> +	else
> +		pathlen = snprintf (path, sizeof(path), "%s/%s", server.host, imap_folder);
> +
> +	if (pathlen < 0)
> +		die("Fatal: Out of memory");
> +	if (pathlen >= sizeof(path))
> +		die("imap URL overflow!");

With a strbuf, this could be something like

	strbuf_addstr(&path, server.host);
	if (!path.len || path.buf[path.len - 1] != '/')
		strbuf_addch(&path, '/');
	strbuf_addstr(&path, imap_folder);

or

	if (ends_with(...))
		strbuf_addf(&path, "%s%s", ...);
	else
		strbuf_addf(...);

Killing the unused ctx->prefix handling is nice. :)

[...]
> +	if (server.tunnel) {
> +		const char *argv[] = { server.tunnel, NULL };
> +		struct child_process tunnel = {NULL};

(not about this patch) Could use the child_proccess's internal
argv_array:

		struct child_process tunnel = {NULL};
		argv_array_push(&tunnel.args, server.tunnel);

(about this patch) Would there be a way to make this part reuse the
existing code?  The only difference I see is that *srvc has been
renamed to server, which doesn't seem to be related to the change of
transport API from OpenSSL to libcurl.

[...]
> +		curl_socket_t sockfd = tunnel.out; // what about tunnel.in ?

Hmm.  curl expects to get a socket it can send(), recv(), setsockopt(),
etc on instead of a pair of fds to read() and write().

I wonder why someone would want to use SSL through a tunnel, though.
Currently it's impossible to get to the SSL codepath when a tunnel
is active (it's in the 'else' block an 'if (srvc->tunnel)').  If that
property is preserved, then we should be safe.

To summarize:

 * I like this idea a lot!  Using libcurl's imaps:// support directly
   means one less dependency to worry about, and using alternate SSL
   libraries like gnutls or nss becomes much easier (e.g., see
   http://fedoraproject.org/wiki/FedoraCryptoConsolidation for how
   that makes configuring certificate trust simpler).

 * This would be easier to take if guarded by an #ifdef, so people
   stuck on ancient libcurl would still be able to use git (and
   ideally still use imap over SSL).

 * This shouldn't have to touch the imap.tunnel support.  imap-send's
   imap.tunnel configuration expects the tunnel to take care of
   securing the channel (e.g. by using 'openssl s_client').

 * Any potential cleanups noticed along the way are very welcome,
   as separate patches.

 * As soon as you're ready to roll this out to a wider audience of
   testers, let me know, and we can try to get it into shape for
   Junio's "next" branch (and hence Debian experimental).

Thanks and hope that helps,
Jonathan

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH/RFC] git-imap-send: use libcurl for implementation
@ 2014-08-12 21:50 Bernhard Reiter
  2014-08-13  1:59 ` Jonathan Nieder
  0 siblings, 1 reply; 11+ messages in thread
From: Bernhard Reiter @ 2014-08-12 21:50 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 1162 bytes --]


Use libcurl's high-level API functions to implement git-imap-send
instead of the previous low-level OpenSSL-based functions.

Signed-off-by: Bernhard Reiter <ockham@raz.or.at>
---
Since version 7.30.0, libcurl's API has been able to communicate with
IMAP servers. Using those high-level functions instead of the current
ones reduces imap-send.c by some 1200 lines of code.

As I don't have access to that many IMAP servers, I haven't been able to
test a variety of parameter combinations. I did test both secure and
insecure (imaps:// and imap://) connections -- this e-mail was generated
that way -- but could e.g. neither test the authMethod nor the tunnel
parameter.

As git-imap-send is one of the two instances OpenSSL is currently used
by git -- the other one being SHA1 -- it might be worthwhile considering
dropping it altogether as there's already a SHA1 library built into git
available as an alternative.

Kind regards
Bernhard
PS: Please CC!

 INSTALL     |   14 +-
 Makefile    |    4 +-
 git.spec.in |    5 +-
 imap-send.c | 1288
+++++------------------------------------------------------
 4 files changed, 111 insertions(+), 1200 deletions(-)



[-- Attachment #2: 0001-git-imap-send-use-libcurl-for-implementation.patch --]
[-- Type: text/x-patch, Size: 36413 bytes --]

diff --git a/INSTALL b/INSTALL
index 6ec7a24..2cd3a42 100644
--- a/INSTALL
+++ b/INSTALL
@@ -108,18 +108,16 @@ Issues of note:
 	  so you might need to install additional packages other than Perl
 	  itself, e.g. Time::HiRes.
 
-	- "openssl" library is used by git-imap-send to use IMAP over SSL.
-	  If you don't need it, use NO_OPENSSL.
-
-	  By default, git uses OpenSSL for SHA1 but it will use its own
+	- By default, git uses OpenSSL for SHA1 but it will use its own
 	  library (inspired by Mozilla's) with either NO_OPENSSL or
 	  BLK_SHA1.  Also included is a version optimized for PowerPC
 	  (PPC_SHA1).
 
-	- "libcurl" library is used by git-http-fetch and git-fetch.  You
-	  might also want the "curl" executable for debugging purposes.
-	  If you do not use http:// or https:// repositories, you do not
-	  have to have them (use NO_CURL).
+	- "libcurl" library is used by git-http-fetch, git-fetch, and
+	  git-impap-send.  You might also want the "curl" executable for
+	  debugging purposes. If you do not use http:// or https://
+	  repositories, and do not want to put patches into an IMAP
+	  mailbox, you do not have to have them (use NO_CURL).
 
 	- "expat" library; git-http-push uses it for remote lock
 	  management over DAV.  Similar to "curl" above, this is optional
diff --git a/Makefile b/Makefile
index 2320de5..7805603 100644
--- a/Makefile
+++ b/Makefile
@@ -2067,9 +2067,9 @@ endif
 git-%$X: %.o GIT-LDFLAGS $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
 
-git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS)
+git-imap-send$X: imap-send.o http.o GIT-LDFLAGS $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
-		$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)
+		$(LIBS) $(CURL_LIBCURL)
 
 git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
diff --git a/git.spec.in b/git.spec.in
index d61d537..7d9230f 100644
--- a/git.spec.in
+++ b/git.spec.in
@@ -8,7 +8,7 @@ License: 	GPL
 Group: 		Development/Tools
 URL: 		http://kernel.org/pub/software/scm/git/
 Source: 	http://kernel.org/pub/software/scm/git/%{name}-%{version}.tar.gz
-BuildRequires:	zlib-devel >= 1.2, openssl-devel, curl-devel, expat-devel, gettext  %{!?_without_docs:, xmlto, asciidoc > 6.0.3}
+BuildRequires:	zlib-devel >= 1.2, openssl-devel, curl-devel >= 7.30.0, expat-devel, gettext  %{!?_without_docs:, xmlto, asciidoc > 6.0.3}
 BuildRoot:	%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 
 Requires:	perl-Git = %{version}-%{release}
@@ -214,6 +214,9 @@ rm -rf $RPM_BUILD_ROOT
 # No files for you!
 
 %changelog
+* Mon Aug 11 2014 Bernhard Reiter <ockham@raz.or.at>
+- Require version >= 7.30.0 of curl-devel for IMAP functions.
+
 * Sun Sep 18 2011 Jakub Narebski <jnareb@gmail.com>
 - Add gitweb manpages to 'gitweb' subpackage
 
diff --git a/imap-send.c b/imap-send.c
index 524fbab..0c4583f 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -22,47 +22,13 @@
  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 
-#include "cache.h"
-#include "credential.h"
+#include "http.h"
 #include "exec_cmd.h"
 #include "run-command.h"
-#ifdef NO_OPENSSL
-typedef void *SSL;
-#endif
 
-static const char imap_send_usage[] = "git imap-send < <mbox>";
-
-#undef DRV_OK
-#define DRV_OK          0
-#define DRV_MSG_BAD     -1
-#define DRV_BOX_BAD     -2
-#define DRV_STORE_BAD   -3
-
-static int Verbose, Quiet;
-
-__attribute__((format (printf, 1, 2)))
-static void imap_info(const char *, ...);
-__attribute__((format (printf, 1, 2)))
-static void imap_warn(const char *, ...);
-
-static char *next_arg(char **);
-
-__attribute__((format (printf, 3, 4)))
-static int nfsnprintf(char *buf, int blen, const char *fmt, ...);
+#include <curl/curl.h>
 
-static int nfvasprintf(char **strp, const char *fmt, va_list ap)
-{
-	int len;
-	char tmp[8192];
-
-	len = vsnprintf(tmp, sizeof(tmp), fmt, ap);
-	if (len < 0)
-		die("Fatal: Out of memory");
-	if (len >= sizeof(tmp))
-		die("imap command overflow!");
-	*strp = xmemdupz(tmp, len);
-	return len;
-}
+static const char imap_send_usage[] = "git imap-send < <mbox>";
 
 struct imap_server_conf {
 	char *name;
@@ -90,1144 +56,6 @@ static struct imap_server_conf server = {
 	NULL,	/* auth_method */
 };
 
-struct imap_socket {
-	int fd[2];
-	SSL *ssl;
-};
-
-struct imap_buffer {
-	struct imap_socket sock;
-	int bytes;
-	int offset;
-	char buf[1024];
-};
-
-struct imap_cmd;
-
-struct imap {
-	int uidnext; /* from SELECT responses */
-	unsigned caps, rcaps; /* CAPABILITY results */
-	/* command queue */
-	int nexttag, num_in_progress, literal_pending;
-	struct imap_cmd *in_progress, **in_progress_append;
-	struct imap_buffer buf; /* this is BIG, so put it last */
-};
-
-struct imap_store {
-	/* currently open mailbox */
-	const char *name; /* foreign! maybe preset? */
-	int uidvalidity;
-	struct imap *imap;
-	const char *prefix;
-};
-
-struct imap_cmd_cb {
-	int (*cont)(struct imap_store *ctx, struct imap_cmd *cmd, const char *prompt);
-	void (*done)(struct imap_store *ctx, struct imap_cmd *cmd, int response);
-	void *ctx;
-	char *data;
-	int dlen;
-	int uid;
-	unsigned create:1, trycreate:1;
-};
-
-struct imap_cmd {
-	struct imap_cmd *next;
-	struct imap_cmd_cb cb;
-	char *cmd;
-	int tag;
-};
-
-#define CAP(cap) (imap->caps & (1 << (cap)))
-
-enum CAPABILITY {
-	NOLOGIN = 0,
-	UIDPLUS,
-	LITERALPLUS,
-	NAMESPACE,
-	STARTTLS,
-	AUTH_CRAM_MD5
-};
-
-static const char *cap_list[] = {
-	"LOGINDISABLED",
-	"UIDPLUS",
-	"LITERAL+",
-	"NAMESPACE",
-	"STARTTLS",
-	"AUTH=CRAM-MD5",
-};
-
-#define RESP_OK    0
-#define RESP_NO    1
-#define RESP_BAD   2
-
-static int get_cmd_result(struct imap_store *ctx, struct imap_cmd *tcmd);
-
-
-#ifndef NO_OPENSSL
-static void ssl_socket_perror(const char *func)
-{
-	fprintf(stderr, "%s: %s\n", func, ERR_error_string(ERR_get_error(), NULL));
-}
-#endif
-
-static void socket_perror(const char *func, struct imap_socket *sock, int ret)
-{
-#ifndef NO_OPENSSL
-	if (sock->ssl) {
-		int sslerr = SSL_get_error(sock->ssl, ret);
-		switch (sslerr) {
-		case SSL_ERROR_NONE:
-			break;
-		case SSL_ERROR_SYSCALL:
-			perror("SSL_connect");
-			break;
-		default:
-			ssl_socket_perror("SSL_connect");
-			break;
-		}
-	} else
-#endif
-	{
-		if (ret < 0)
-			perror(func);
-		else
-			fprintf(stderr, "%s: unexpected EOF\n", func);
-	}
-}
-
-#ifdef NO_OPENSSL
-static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int verify)
-{
-	fprintf(stderr, "SSL requested but SSL support not compiled in\n");
-	return -1;
-}
-
-#else
-
-static int host_matches(const char *host, const char *pattern)
-{
-	if (pattern[0] == '*' && pattern[1] == '.') {
-		pattern += 2;
-		if (!(host = strchr(host, '.')))
-			return 0;
-		host++;
-	}
-
-	return *host && *pattern && !strcasecmp(host, pattern);
-}
-
-static int verify_hostname(X509 *cert, const char *hostname)
-{
-	int len;
-	X509_NAME *subj;
-	char cname[1000];
-	int i, found;
-	STACK_OF(GENERAL_NAME) *subj_alt_names;
-
-	/* try the DNS subjectAltNames */
-	found = 0;
-	if ((subj_alt_names = X509_get_ext_d2i(cert, NID_subject_alt_name, NULL, NULL))) {
-		int num_subj_alt_names = sk_GENERAL_NAME_num(subj_alt_names);
-		for (i = 0; !found && i < num_subj_alt_names; i++) {
-			GENERAL_NAME *subj_alt_name = sk_GENERAL_NAME_value(subj_alt_names, i);
-			if (subj_alt_name->type == GEN_DNS &&
-			    strlen((const char *)subj_alt_name->d.ia5->data) == (size_t)subj_alt_name->d.ia5->length &&
-			    host_matches(hostname, (const char *)(subj_alt_name->d.ia5->data)))
-				found = 1;
-		}
-		sk_GENERAL_NAME_pop_free(subj_alt_names, GENERAL_NAME_free);
-	}
-	if (found)
-		return 0;
-
-	/* try the common name */
-	if (!(subj = X509_get_subject_name(cert)))
-		return error("cannot get certificate subject");
-	if ((len = X509_NAME_get_text_by_NID(subj, NID_commonName, cname, sizeof(cname))) < 0)
-		return error("cannot get certificate common name");
-	if (strlen(cname) == (size_t)len && host_matches(hostname, cname))
-		return 0;
-	return error("certificate owner '%s' does not match hostname '%s'",
-		     cname, hostname);
-}
-
-static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int verify)
-{
-#if (OPENSSL_VERSION_NUMBER >= 0x10000000L)
-	const SSL_METHOD *meth;
-#else
-	SSL_METHOD *meth;
-#endif
-	SSL_CTX *ctx;
-	int ret;
-	X509 *cert;
-
-	SSL_library_init();
-	SSL_load_error_strings();
-
-	if (use_tls_only)
-		meth = TLSv1_method();
-	else
-		meth = SSLv23_method();
-
-	if (!meth) {
-		ssl_socket_perror("SSLv23_method");
-		return -1;
-	}
-
-	ctx = SSL_CTX_new(meth);
-
-	if (verify)
-		SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER, NULL);
-
-	if (!SSL_CTX_set_default_verify_paths(ctx)) {
-		ssl_socket_perror("SSL_CTX_set_default_verify_paths");
-		return -1;
-	}
-	sock->ssl = SSL_new(ctx);
-	if (!sock->ssl) {
-		ssl_socket_perror("SSL_new");
-		return -1;
-	}
-	if (!SSL_set_rfd(sock->ssl, sock->fd[0])) {
-		ssl_socket_perror("SSL_set_rfd");
-		return -1;
-	}
-	if (!SSL_set_wfd(sock->ssl, sock->fd[1])) {
-		ssl_socket_perror("SSL_set_wfd");
-		return -1;
-	}
-
-#ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
-	/*
-	 * SNI (RFC4366)
-	 * OpenSSL does not document this function, but the implementation
-	 * returns 1 on success, 0 on failure after calling SSLerr().
-	 */
-	ret = SSL_set_tlsext_host_name(sock->ssl, server.host);
-	if (ret != 1)
-		warning("SSL_set_tlsext_host_name(%s) failed.", server.host);
-#endif
-
-	ret = SSL_connect(sock->ssl);
-	if (ret <= 0) {
-		socket_perror("SSL_connect", sock, ret);
-		return -1;
-	}
-
-	if (verify) {
-		/* make sure the hostname matches that of the certificate */
-		cert = SSL_get_peer_certificate(sock->ssl);
-		if (!cert)
-			return error("unable to get peer certificate.");
-		if (verify_hostname(cert, server.host) < 0)
-			return -1;
-	}
-
-	return 0;
-}
-#endif
-
-static int socket_read(struct imap_socket *sock, char *buf, int len)
-{
-	ssize_t n;
-#ifndef NO_OPENSSL
-	if (sock->ssl)
-		n = SSL_read(sock->ssl, buf, len);
-	else
-#endif
-		n = xread(sock->fd[0], buf, len);
-	if (n <= 0) {
-		socket_perror("read", sock, n);
-		close(sock->fd[0]);
-		close(sock->fd[1]);
-		sock->fd[0] = sock->fd[1] = -1;
-	}
-	return n;
-}
-
-static int socket_write(struct imap_socket *sock, const char *buf, int len)
-{
-	int n;
-#ifndef NO_OPENSSL
-	if (sock->ssl)
-		n = SSL_write(sock->ssl, buf, len);
-	else
-#endif
-		n = write_in_full(sock->fd[1], buf, len);
-	if (n != len) {
-		socket_perror("write", sock, n);
-		close(sock->fd[0]);
-		close(sock->fd[1]);
-		sock->fd[0] = sock->fd[1] = -1;
-	}
-	return n;
-}
-
-static void socket_shutdown(struct imap_socket *sock)
-{
-#ifndef NO_OPENSSL
-	if (sock->ssl) {
-		SSL_shutdown(sock->ssl);
-		SSL_free(sock->ssl);
-	}
-#endif
-	close(sock->fd[0]);
-	close(sock->fd[1]);
-}
-
-/* simple line buffering */
-static int buffer_gets(struct imap_buffer *b, char **s)
-{
-	int n;
-	int start = b->offset;
-
-	*s = b->buf + start;
-
-	for (;;) {
-		/* make sure we have enough data to read the \r\n sequence */
-		if (b->offset + 1 >= b->bytes) {
-			if (start) {
-				/* shift down used bytes */
-				*s = b->buf;
-
-				assert(start <= b->bytes);
-				n = b->bytes - start;
-
-				if (n)
-					memmove(b->buf, b->buf + start, n);
-				b->offset -= start;
-				b->bytes = n;
-				start = 0;
-			}
-
-			n = socket_read(&b->sock, b->buf + b->bytes,
-					 sizeof(b->buf) - b->bytes);
-
-			if (n <= 0)
-				return -1;
-
-			b->bytes += n;
-		}
-
-		if (b->buf[b->offset] == '\r') {
-			assert(b->offset + 1 < b->bytes);
-			if (b->buf[b->offset + 1] == '\n') {
-				b->buf[b->offset] = 0;  /* terminate the string */
-				b->offset += 2; /* next line */
-				if (Verbose)
-					puts(*s);
-				return 0;
-			}
-		}
-
-		b->offset++;
-	}
-	/* not reached */
-}
-
-static void imap_info(const char *msg, ...)
-{
-	va_list va;
-
-	if (!Quiet) {
-		va_start(va, msg);
-		vprintf(msg, va);
-		va_end(va);
-		fflush(stdout);
-	}
-}
-
-static void imap_warn(const char *msg, ...)
-{
-	va_list va;
-
-	if (Quiet < 2) {
-		va_start(va, msg);
-		vfprintf(stderr, msg, va);
-		va_end(va);
-	}
-}
-
-static char *next_arg(char **s)
-{
-	char *ret;
-
-	if (!s || !*s)
-		return NULL;
-	while (isspace((unsigned char) **s))
-		(*s)++;
-	if (!**s) {
-		*s = NULL;
-		return NULL;
-	}
-	if (**s == '"') {
-		++*s;
-		ret = *s;
-		*s = strchr(*s, '"');
-	} else {
-		ret = *s;
-		while (**s && !isspace((unsigned char) **s))
-			(*s)++;
-	}
-	if (*s) {
-		if (**s)
-			*(*s)++ = 0;
-		if (!**s)
-			*s = NULL;
-	}
-	return ret;
-}
-
-static int nfsnprintf(char *buf, int blen, const char *fmt, ...)
-{
-	int ret;
-	va_list va;
-
-	va_start(va, fmt);
-	if (blen <= 0 || (unsigned)(ret = vsnprintf(buf, blen, fmt, va)) >= (unsigned)blen)
-		die("Fatal: buffer too small. Please report a bug.");
-	va_end(va);
-	return ret;
-}
-
-static struct imap_cmd *v_issue_imap_cmd(struct imap_store *ctx,
-					 struct imap_cmd_cb *cb,
-					 const char *fmt, va_list ap)
-{
-	struct imap *imap = ctx->imap;
-	struct imap_cmd *cmd;
-	int n, bufl;
-	char buf[1024];
-
-	cmd = xmalloc(sizeof(struct imap_cmd));
-	nfvasprintf(&cmd->cmd, fmt, ap);
-	cmd->tag = ++imap->nexttag;
-
-	if (cb)
-		cmd->cb = *cb;
-	else
-		memset(&cmd->cb, 0, sizeof(cmd->cb));
-
-	while (imap->literal_pending)
-		get_cmd_result(ctx, NULL);
-
-	if (!cmd->cb.data)
-		bufl = nfsnprintf(buf, sizeof(buf), "%d %s\r\n", cmd->tag, cmd->cmd);
-	else
-		bufl = nfsnprintf(buf, sizeof(buf), "%d %s{%d%s}\r\n",
-				  cmd->tag, cmd->cmd, cmd->cb.dlen,
-				  CAP(LITERALPLUS) ? "+" : "");
-
-	if (Verbose) {
-		if (imap->num_in_progress)
-			printf("(%d in progress) ", imap->num_in_progress);
-		if (memcmp(cmd->cmd, "LOGIN", 5))
-			printf(">>> %s", buf);
-		else
-			printf(">>> %d LOGIN <user> <pass>\n", cmd->tag);
-	}
-	if (socket_write(&imap->buf.sock, buf, bufl) != bufl) {
-		free(cmd->cmd);
-		free(cmd);
-		if (cb)
-			free(cb->data);
-		return NULL;
-	}
-	if (cmd->cb.data) {
-		if (CAP(LITERALPLUS)) {
-			n = socket_write(&imap->buf.sock, cmd->cb.data, cmd->cb.dlen);
-			free(cmd->cb.data);
-			if (n != cmd->cb.dlen ||
-			    socket_write(&imap->buf.sock, "\r\n", 2) != 2) {
-				free(cmd->cmd);
-				free(cmd);
-				return NULL;
-			}
-			cmd->cb.data = NULL;
-		} else
-			imap->literal_pending = 1;
-	} else if (cmd->cb.cont)
-		imap->literal_pending = 1;
-	cmd->next = NULL;
-	*imap->in_progress_append = cmd;
-	imap->in_progress_append = &cmd->next;
-	imap->num_in_progress++;
-	return cmd;
-}
-
-__attribute__((format (printf, 3, 4)))
-static struct imap_cmd *issue_imap_cmd(struct imap_store *ctx,
-				       struct imap_cmd_cb *cb,
-				       const char *fmt, ...)
-{
-	struct imap_cmd *ret;
-	va_list ap;
-
-	va_start(ap, fmt);
-	ret = v_issue_imap_cmd(ctx, cb, fmt, ap);
-	va_end(ap);
-	return ret;
-}
-
-__attribute__((format (printf, 3, 4)))
-static int imap_exec(struct imap_store *ctx, struct imap_cmd_cb *cb,
-		     const char *fmt, ...)
-{
-	va_list ap;
-	struct imap_cmd *cmdp;
-
-	va_start(ap, fmt);
-	cmdp = v_issue_imap_cmd(ctx, cb, fmt, ap);
-	va_end(ap);
-	if (!cmdp)
-		return RESP_BAD;
-
-	return get_cmd_result(ctx, cmdp);
-}
-
-__attribute__((format (printf, 3, 4)))
-static int imap_exec_m(struct imap_store *ctx, struct imap_cmd_cb *cb,
-		       const char *fmt, ...)
-{
-	va_list ap;
-	struct imap_cmd *cmdp;
-
-	va_start(ap, fmt);
-	cmdp = v_issue_imap_cmd(ctx, cb, fmt, ap);
-	va_end(ap);
-	if (!cmdp)
-		return DRV_STORE_BAD;
-
-	switch (get_cmd_result(ctx, cmdp)) {
-	case RESP_BAD: return DRV_STORE_BAD;
-	case RESP_NO: return DRV_MSG_BAD;
-	default: return DRV_OK;
-	}
-}
-
-static int skip_imap_list_l(char **sp, int level)
-{
-	char *s = *sp;
-
-	for (;;) {
-		while (isspace((unsigned char)*s))
-			s++;
-		if (level && *s == ')') {
-			s++;
-			break;
-		}
-		if (*s == '(') {
-			/* sublist */
-			s++;
-			if (skip_imap_list_l(&s, level + 1))
-				goto bail;
-		} else if (*s == '"') {
-			/* quoted string */
-			s++;
-			for (; *s != '"'; s++)
-				if (!*s)
-					goto bail;
-			s++;
-		} else {
-			/* atom */
-			for (; *s && !isspace((unsigned char)*s); s++)
-				if (level && *s == ')')
-					break;
-		}
-
-		if (!level)
-			break;
-		if (!*s)
-			goto bail;
-	}
-	*sp = s;
-	return 0;
-
-bail:
-	return -1;
-}
-
-static void skip_list(char **sp)
-{
-	skip_imap_list_l(sp, 0);
-}
-
-static void parse_capability(struct imap *imap, char *cmd)
-{
-	char *arg;
-	unsigned i;
-
-	imap->caps = 0x80000000;
-	while ((arg = next_arg(&cmd)))
-		for (i = 0; i < ARRAY_SIZE(cap_list); i++)
-			if (!strcmp(cap_list[i], arg))
-				imap->caps |= 1 << i;
-	imap->rcaps = imap->caps;
-}
-
-static int parse_response_code(struct imap_store *ctx, struct imap_cmd_cb *cb,
-			       char *s)
-{
-	struct imap *imap = ctx->imap;
-	char *arg, *p;
-
-	if (*s != '[')
-		return RESP_OK;		/* no response code */
-	s++;
-	if (!(p = strchr(s, ']'))) {
-		fprintf(stderr, "IMAP error: malformed response code\n");
-		return RESP_BAD;
-	}
-	*p++ = 0;
-	arg = next_arg(&s);
-	if (!strcmp("UIDVALIDITY", arg)) {
-		if (!(arg = next_arg(&s)) || !(ctx->uidvalidity = atoi(arg))) {
-			fprintf(stderr, "IMAP error: malformed UIDVALIDITY status\n");
-			return RESP_BAD;
-		}
-	} else if (!strcmp("UIDNEXT", arg)) {
-		if (!(arg = next_arg(&s)) || !(imap->uidnext = atoi(arg))) {
-			fprintf(stderr, "IMAP error: malformed NEXTUID status\n");
-			return RESP_BAD;
-		}
-	} else if (!strcmp("CAPABILITY", arg)) {
-		parse_capability(imap, s);
-	} else if (!strcmp("ALERT", arg)) {
-		/* RFC2060 says that these messages MUST be displayed
-		 * to the user
-		 */
-		for (; isspace((unsigned char)*p); p++);
-		fprintf(stderr, "*** IMAP ALERT *** %s\n", p);
-	} else if (cb && cb->ctx && !strcmp("APPENDUID", arg)) {
-		if (!(arg = next_arg(&s)) || !(ctx->uidvalidity = atoi(arg)) ||
-		    !(arg = next_arg(&s)) || !(*(int *)cb->ctx = atoi(arg))) {
-			fprintf(stderr, "IMAP error: malformed APPENDUID status\n");
-			return RESP_BAD;
-		}
-	}
-	return RESP_OK;
-}
-
-static int get_cmd_result(struct imap_store *ctx, struct imap_cmd *tcmd)
-{
-	struct imap *imap = ctx->imap;
-	struct imap_cmd *cmdp, **pcmdp, *ncmdp;
-	char *cmd, *arg, *arg1, *p;
-	int n, resp, resp2, tag;
-
-	for (;;) {
-		if (buffer_gets(&imap->buf, &cmd))
-			return RESP_BAD;
-
-		arg = next_arg(&cmd);
-		if (*arg == '*') {
-			arg = next_arg(&cmd);
-			if (!arg) {
-				fprintf(stderr, "IMAP error: unable to parse untagged response\n");
-				return RESP_BAD;
-			}
-
-			if (!strcmp("NAMESPACE", arg)) {
-				/* rfc2342 NAMESPACE response. */
-				skip_list(&cmd); /* Personal mailboxes */
-				skip_list(&cmd); /* Others' mailboxes */
-				skip_list(&cmd); /* Shared mailboxes */
-			} else if (!strcmp("OK", arg) || !strcmp("BAD", arg) ||
-				   !strcmp("NO", arg) || !strcmp("BYE", arg)) {
-				if ((resp = parse_response_code(ctx, NULL, cmd)) != RESP_OK)
-					return resp;
-			} else if (!strcmp("CAPABILITY", arg)) {
-				parse_capability(imap, cmd);
-			} else if ((arg1 = next_arg(&cmd))) {
-				; /*
-				   * Unhandled response-data with at least two words.
-				   * Ignore it.
-				   *
-				   * NEEDSWORK: Previously this case handled '<num> EXISTS'
-				   * and '<num> RECENT' but as a probably-unintended side
-				   * effect it ignores other unrecognized two-word
-				   * responses.  imap-send doesn't ever try to read
-				   * messages or mailboxes these days, so consider
-				   * eliminating this case.
-				   */
-			} else {
-				fprintf(stderr, "IMAP error: unable to parse untagged response\n");
-				return RESP_BAD;
-			}
-		} else if (!imap->in_progress) {
-			fprintf(stderr, "IMAP error: unexpected reply: %s %s\n", arg, cmd ? cmd : "");
-			return RESP_BAD;
-		} else if (*arg == '+') {
-			/* This can happen only with the last command underway, as
-			   it enforces a round-trip. */
-			cmdp = (struct imap_cmd *)((char *)imap->in_progress_append -
-			       offsetof(struct imap_cmd, next));
-			if (cmdp->cb.data) {
-				n = socket_write(&imap->buf.sock, cmdp->cb.data, cmdp->cb.dlen);
-				free(cmdp->cb.data);
-				cmdp->cb.data = NULL;
-				if (n != (int)cmdp->cb.dlen)
-					return RESP_BAD;
-			} else if (cmdp->cb.cont) {
-				if (cmdp->cb.cont(ctx, cmdp, cmd))
-					return RESP_BAD;
-			} else {
-				fprintf(stderr, "IMAP error: unexpected command continuation request\n");
-				return RESP_BAD;
-			}
-			if (socket_write(&imap->buf.sock, "\r\n", 2) != 2)
-				return RESP_BAD;
-			if (!cmdp->cb.cont)
-				imap->literal_pending = 0;
-			if (!tcmd)
-				return DRV_OK;
-		} else {
-			tag = atoi(arg);
-			for (pcmdp = &imap->in_progress; (cmdp = *pcmdp); pcmdp = &cmdp->next)
-				if (cmdp->tag == tag)
-					goto gottag;
-			fprintf(stderr, "IMAP error: unexpected tag %s\n", arg);
-			return RESP_BAD;
-		gottag:
-			if (!(*pcmdp = cmdp->next))
-				imap->in_progress_append = pcmdp;
-			imap->num_in_progress--;
-			if (cmdp->cb.cont || cmdp->cb.data)
-				imap->literal_pending = 0;
-			arg = next_arg(&cmd);
-			if (!strcmp("OK", arg))
-				resp = DRV_OK;
-			else {
-				if (!strcmp("NO", arg)) {
-					if (cmdp->cb.create && cmd && (cmdp->cb.trycreate || !memcmp(cmd, "[TRYCREATE]", 11))) { /* SELECT, APPEND or UID COPY */
-						p = strchr(cmdp->cmd, '"');
-						if (!issue_imap_cmd(ctx, NULL, "CREATE \"%.*s\"", (int)(strchr(p + 1, '"') - p + 1), p)) {
-							resp = RESP_BAD;
-							goto normal;
-						}
-						/* not waiting here violates the spec, but a server that does not
-						   grok this nonetheless violates it too. */
-						cmdp->cb.create = 0;
-						if (!(ncmdp = issue_imap_cmd(ctx, &cmdp->cb, "%s", cmdp->cmd))) {
-							resp = RESP_BAD;
-							goto normal;
-						}
-						free(cmdp->cmd);
-						free(cmdp);
-						if (!tcmd)
-							return 0;	/* ignored */
-						if (cmdp == tcmd)
-							tcmd = ncmdp;
-						continue;
-					}
-					resp = RESP_NO;
-				} else /*if (!strcmp("BAD", arg))*/
-					resp = RESP_BAD;
-				fprintf(stderr, "IMAP command '%s' returned response (%s) - %s\n",
-					 memcmp(cmdp->cmd, "LOGIN", 5) ?
-							cmdp->cmd : "LOGIN <user> <pass>",
-							arg, cmd ? cmd : "");
-			}
-			if ((resp2 = parse_response_code(ctx, &cmdp->cb, cmd)) > resp)
-				resp = resp2;
-		normal:
-			if (cmdp->cb.done)
-				cmdp->cb.done(ctx, cmdp, resp);
-			free(cmdp->cb.data);
-			free(cmdp->cmd);
-			free(cmdp);
-			if (!tcmd || tcmd == cmdp)
-				return resp;
-		}
-	}
-	/* not reached */
-}
-
-static void imap_close_server(struct imap_store *ictx)
-{
-	struct imap *imap = ictx->imap;
-
-	if (imap->buf.sock.fd[0] != -1) {
-		imap_exec(ictx, NULL, "LOGOUT");
-		socket_shutdown(&imap->buf.sock);
-	}
-	free(imap);
-}
-
-static void imap_close_store(struct imap_store *ctx)
-{
-	imap_close_server(ctx);
-	free(ctx);
-}
-
-#ifndef NO_OPENSSL
-
-/*
- * hexchar() and cram() functions are based on the code from the isync
- * project (http://isync.sf.net/).
- */
-static char hexchar(unsigned int b)
-{
-	return b < 10 ? '0' + b : 'a' + (b - 10);
-}
-
-#define ENCODED_SIZE(n) (4*((n+2)/3))
-static char *cram(const char *challenge_64, const char *user, const char *pass)
-{
-	int i, resp_len, encoded_len, decoded_len;
-	HMAC_CTX hmac;
-	unsigned char hash[16];
-	char hex[33];
-	char *response, *response_64, *challenge;
-
-	/*
-	 * length of challenge_64 (i.e. base-64 encoded string) is a good
-	 * enough upper bound for challenge (decoded result).
-	 */
-	encoded_len = strlen(challenge_64);
-	challenge = xmalloc(encoded_len);
-	decoded_len = EVP_DecodeBlock((unsigned char *)challenge,
-				      (unsigned char *)challenge_64, encoded_len);
-	if (decoded_len < 0)
-		die("invalid challenge %s", challenge_64);
-	HMAC_Init(&hmac, (unsigned char *)pass, strlen(pass), EVP_md5());
-	HMAC_Update(&hmac, (unsigned char *)challenge, decoded_len);
-	HMAC_Final(&hmac, hash, NULL);
-	HMAC_CTX_cleanup(&hmac);
-
-	hex[32] = 0;
-	for (i = 0; i < 16; i++) {
-		hex[2 * i] = hexchar((hash[i] >> 4) & 0xf);
-		hex[2 * i + 1] = hexchar(hash[i] & 0xf);
-	}
-
-	/* response: "<user> <digest in hex>" */
-	resp_len = strlen(user) + 1 + strlen(hex) + 1;
-	response = xmalloc(resp_len);
-	sprintf(response, "%s %s", user, hex);
-
-	response_64 = xmalloc(ENCODED_SIZE(resp_len) + 1);
-	encoded_len = EVP_EncodeBlock((unsigned char *)response_64,
-				      (unsigned char *)response, resp_len);
-	if (encoded_len < 0)
-		die("EVP_EncodeBlock error");
-	response_64[encoded_len] = '\0';
-	return (char *)response_64;
-}
-
-#else
-
-static char *cram(const char *challenge_64, const char *user, const char *pass)
-{
-	die("If you want to use CRAM-MD5 authenticate method, "
-	    "you have to build git-imap-send with OpenSSL library.");
-}
-
-#endif
-
-static int auth_cram_md5(struct imap_store *ctx, struct imap_cmd *cmd, const char *prompt)
-{
-	int ret;
-	char *response;
-
-	response = cram(prompt, server.user, server.pass);
-
-	ret = socket_write(&ctx->imap->buf.sock, response, strlen(response));
-	if (ret != strlen(response))
-		return error("IMAP error: sending response failed");
-
-	free(response);
-
-	return 0;
-}
-
-static struct imap_store *imap_open_store(struct imap_server_conf *srvc)
-{
-	struct credential cred = CREDENTIAL_INIT;
-	struct imap_store *ctx;
-	struct imap *imap;
-	char *arg, *rsp;
-	int s = -1, preauth;
-
-	ctx = xcalloc(1, sizeof(*ctx));
-
-	ctx->imap = imap = xcalloc(sizeof(*imap), 1);
-	imap->buf.sock.fd[0] = imap->buf.sock.fd[1] = -1;
-	imap->in_progress_append = &imap->in_progress;
-
-	/* open connection to IMAP server */
-
-	if (srvc->tunnel) {
-		const char *argv[] = { srvc->tunnel, NULL };
-		struct child_process tunnel = {NULL};
-
-		imap_info("Starting tunnel '%s'... ", srvc->tunnel);
-
-		tunnel.argv = argv;
-		tunnel.use_shell = 1;
-		tunnel.in = -1;
-		tunnel.out = -1;
-		if (start_command(&tunnel))
-			die("cannot start proxy %s", argv[0]);
-
-		imap->buf.sock.fd[0] = tunnel.out;
-		imap->buf.sock.fd[1] = tunnel.in;
-
-		imap_info("ok\n");
-	} else {
-#ifndef NO_IPV6
-		struct addrinfo hints, *ai0, *ai;
-		int gai;
-		char portstr[6];
-
-		snprintf(portstr, sizeof(portstr), "%d", srvc->port);
-
-		memset(&hints, 0, sizeof(hints));
-		hints.ai_socktype = SOCK_STREAM;
-		hints.ai_protocol = IPPROTO_TCP;
-
-		imap_info("Resolving %s... ", srvc->host);
-		gai = getaddrinfo(srvc->host, portstr, &hints, &ai);
-		if (gai) {
-			fprintf(stderr, "getaddrinfo: %s\n", gai_strerror(gai));
-			goto bail;
-		}
-		imap_info("ok\n");
-
-		for (ai0 = ai; ai; ai = ai->ai_next) {
-			char addr[NI_MAXHOST];
-
-			s = socket(ai->ai_family, ai->ai_socktype,
-				   ai->ai_protocol);
-			if (s < 0)
-				continue;
-
-			getnameinfo(ai->ai_addr, ai->ai_addrlen, addr,
-				    sizeof(addr), NULL, 0, NI_NUMERICHOST);
-			imap_info("Connecting to [%s]:%s... ", addr, portstr);
-
-			if (connect(s, ai->ai_addr, ai->ai_addrlen) < 0) {
-				close(s);
-				s = -1;
-				perror("connect");
-				continue;
-			}
-
-			break;
-		}
-		freeaddrinfo(ai0);
-#else /* NO_IPV6 */
-		struct hostent *he;
-		struct sockaddr_in addr;
-
-		memset(&addr, 0, sizeof(addr));
-		addr.sin_port = htons(srvc->port);
-		addr.sin_family = AF_INET;
-
-		imap_info("Resolving %s... ", srvc->host);
-		he = gethostbyname(srvc->host);
-		if (!he) {
-			perror("gethostbyname");
-			goto bail;
-		}
-		imap_info("ok\n");
-
-		addr.sin_addr.s_addr = *((int *) he->h_addr_list[0]);
-
-		s = socket(PF_INET, SOCK_STREAM, 0);
-
-		imap_info("Connecting to %s:%hu... ", inet_ntoa(addr.sin_addr), ntohs(addr.sin_port));
-		if (connect(s, (struct sockaddr *)&addr, sizeof(addr))) {
-			close(s);
-			s = -1;
-			perror("connect");
-		}
-#endif
-		if (s < 0) {
-			fputs("Error: unable to connect to server.\n", stderr);
-			goto bail;
-		}
-
-		imap->buf.sock.fd[0] = s;
-		imap->buf.sock.fd[1] = dup(s);
-
-		if (srvc->use_ssl &&
-		    ssl_socket_connect(&imap->buf.sock, 0, srvc->ssl_verify)) {
-			close(s);
-			goto bail;
-		}
-		imap_info("ok\n");
-	}
-
-	/* read the greeting string */
-	if (buffer_gets(&imap->buf, &rsp)) {
-		fprintf(stderr, "IMAP error: no greeting response\n");
-		goto bail;
-	}
-	arg = next_arg(&rsp);
-	if (!arg || *arg != '*' || (arg = next_arg(&rsp)) == NULL) {
-		fprintf(stderr, "IMAP error: invalid greeting response\n");
-		goto bail;
-	}
-	preauth = 0;
-	if (!strcmp("PREAUTH", arg))
-		preauth = 1;
-	else if (strcmp("OK", arg) != 0) {
-		fprintf(stderr, "IMAP error: unknown greeting response\n");
-		goto bail;
-	}
-	parse_response_code(ctx, NULL, rsp);
-	if (!imap->caps && imap_exec(ctx, NULL, "CAPABILITY") != RESP_OK)
-		goto bail;
-
-	if (!preauth) {
-#ifndef NO_OPENSSL
-		if (!srvc->use_ssl && CAP(STARTTLS)) {
-			if (imap_exec(ctx, NULL, "STARTTLS") != RESP_OK)
-				goto bail;
-			if (ssl_socket_connect(&imap->buf.sock, 1,
-					       srvc->ssl_verify))
-				goto bail;
-			/* capabilities may have changed, so get the new capabilities */
-			if (imap_exec(ctx, NULL, "CAPABILITY") != RESP_OK)
-				goto bail;
-		}
-#endif
-		imap_info("Logging in...\n");
-		if (!srvc->user || !srvc->pass) {
-			cred.protocol = xstrdup(srvc->use_ssl ? "imaps" : "imap");
-			cred.host = xstrdup(srvc->host);
-
-			if (srvc->user)
-				cred.username = xstrdup(srvc->user);
-			if (srvc->pass)
-				cred.password = xstrdup(srvc->pass);
-
-			credential_fill(&cred);
-
-			if (!srvc->user)
-				srvc->user = xstrdup(cred.username);
-			if (!srvc->pass)
-				srvc->pass = xstrdup(cred.password);
-		}
-
-		if (CAP(NOLOGIN)) {
-			fprintf(stderr, "Skipping account %s@%s, server forbids LOGIN\n", srvc->user, srvc->host);
-			goto bail;
-		}
-
-		if (srvc->auth_method) {
-			struct imap_cmd_cb cb;
-
-			if (!strcmp(srvc->auth_method, "CRAM-MD5")) {
-				if (!CAP(AUTH_CRAM_MD5)) {
-					fprintf(stderr, "You specified"
-						"CRAM-MD5 as authentication method, "
-						"but %s doesn't support it.\n", srvc->host);
-					goto bail;
-				}
-				/* CRAM-MD5 */
-
-				memset(&cb, 0, sizeof(cb));
-				cb.cont = auth_cram_md5;
-				if (imap_exec(ctx, &cb, "AUTHENTICATE CRAM-MD5") != RESP_OK) {
-					fprintf(stderr, "IMAP error: AUTHENTICATE CRAM-MD5 failed\n");
-					goto bail;
-				}
-			} else {
-				fprintf(stderr, "Unknown authentication method:%s\n", srvc->host);
-				goto bail;
-			}
-		} else {
-			if (!imap->buf.sock.ssl)
-				imap_warn("*** IMAP Warning *** Password is being "
-					  "sent in the clear\n");
-			if (imap_exec(ctx, NULL, "LOGIN \"%s\" \"%s\"", srvc->user, srvc->pass) != RESP_OK) {
-				fprintf(stderr, "IMAP error: LOGIN failed\n");
-				goto bail;
-			}
-		}
-	} /* !preauth */
-
-	if (cred.username)
-		credential_approve(&cred);
-	credential_clear(&cred);
-
-	ctx->prefix = "";
-	return ctx;
-
-bail:
-	if (cred.username)
-		credential_reject(&cred);
-	credential_clear(&cred);
-
-	imap_close_store(ctx);
-	return NULL;
-}
-
-/*
- * Insert CR characters as necessary in *msg to ensure that every LF
- * character in *msg is preceded by a CR.
- */
-static void lf_to_crlf(struct strbuf *msg)
-{
-	char *new;
-	size_t i, j;
-	char lastc;
-
-	/* First pass: tally, in j, the size of the new string: */
-	for (i = j = 0, lastc = '\0'; i < msg->len; i++) {
-		if (msg->buf[i] == '\n' && lastc != '\r')
-			j++; /* a CR will need to be added here */
-		lastc = msg->buf[i];
-		j++;
-	}
-
-	new = xmalloc(j + 1);
-
-	/*
-	 * Second pass: write the new string.  Note that this loop is
-	 * otherwise identical to the first pass.
-	 */
-	for (i = j = 0, lastc = '\0'; i < msg->len; i++) {
-		if (msg->buf[i] == '\n' && lastc != '\r')
-			new[j++] = '\r';
-		lastc = new[j++] = msg->buf[i];
-	}
-	strbuf_attach(msg, new, j, j + 1);
-}
-
-/*
- * Store msg to IMAP.  Also detach and free the data from msg->data,
- * leaving msg->data empty.
- */
-static int imap_store_msg(struct imap_store *ctx, struct strbuf *msg)
-{
-	struct imap *imap = ctx->imap;
-	struct imap_cmd_cb cb;
-	const char *prefix, *box;
-	int ret;
-
-	lf_to_crlf(msg);
-	memset(&cb, 0, sizeof(cb));
-
-	cb.dlen = msg->len;
-	cb.data = strbuf_detach(msg, NULL);
-
-	box = ctx->name;
-	prefix = !strcmp(box, "INBOX") ? "" : ctx->prefix;
-	cb.create = 0;
-	ret = imap_exec_m(ctx, &cb, "APPEND \"%s%s\" ", prefix, box);
-	imap->caps = imap->rcaps;
-	if (ret != DRV_OK)
-		return ret;
-
-	return DRV_OK;
-}
-
 static void wrap_in_html(struct strbuf *msg)
 {
 	struct strbuf buf = STRBUF_INIT;
@@ -1324,6 +152,28 @@ static int split_msg(struct strbuf *all_msgs, struct strbuf *msg, int *ofs)
 	return 1;
 }
 
+static curl_socket_t opensocket(void *clientp, curlsocktype purpose,
+								struct curl_sockaddr *address)
+{
+	curl_socket_t sockfd;
+	(void)purpose;
+	(void)address;
+	sockfd = *(curl_socket_t *)clientp;
+	/* the actual externally set socket is passed in via the OPENSOCKETDATA
+	   option */
+	return sockfd;
+}
+
+static int sockopt_callback(void *clientp, curl_socket_t curlfd,
+							curlsocktype purpose)
+{
+	(void)clientp;
+	(void)curlfd;
+	(void)purpose;
+	/* This return code was added in libcurl 7.21.5 */
+	return CURL_SOCKOPT_ALREADY_CONNECTED;
+}
+
 static char *imap_folder;
 
 static int git_imap_config(const char *key, const char *val, void *cb)
@@ -1368,12 +218,14 @@ static int git_imap_config(const char *key, const char *val, void *cb)
 int main(int argc, char **argv)
 {
 	struct strbuf all_msgs = STRBUF_INIT;
-	struct strbuf msg = STRBUF_INIT;
-	struct imap_store *ctx = NULL;
+	struct buffer msg = { STRBUF_INIT, 0 };
 	int ofs = 0;
-	int r;
 	int total, n = 0;
 	int nongit_ok;
+	char path[8192];
+	int pathlen;
+	CURL *curl;
+	CURLcode r = CURLE_OK;
 
 	git_extract_argv0_path(argv[0]);
 
@@ -1417,31 +269,89 @@ int main(int argc, char **argv)
 		return 1;
 	}
 
-	/* write it to the imap server */
-	ctx = imap_open_store(&server);
-	if (!ctx) {
-		fprintf(stderr, "failed to open store\n");
+	curl_global_init(CURL_GLOBAL_ALL);
+	curl = curl_easy_init();
+
+	if (!curl) {
+		fprintf(stderr, "failed to init curl\n");
 		return 1;
 	}
 
+	curl_easy_setopt(curl, CURLOPT_USERNAME, server.user);
+	curl_easy_setopt(curl, CURLOPT_PASSWORD, server.pass);
+
+	if (ends_with(server.host, "/"))
+		pathlen = snprintf (path, sizeof(path), "%s%s", server.host, imap_folder);
+	else
+		pathlen = snprintf (path, sizeof(path), "%s/%s", server.host, imap_folder);
+
+	if (pathlen < 0)
+		die("Fatal: Out of memory");
+	if (pathlen >= sizeof(path))
+		die("imap URL overflow!");
+
+	curl_easy_setopt(curl, CURLOPT_URL, path);
+	curl_easy_setopt(curl, CURLOPT_PORT, server.port);
+	curl_easy_setopt(curl, CURLOPT_LOGIN_OPTIONS, server.auth_method);
+
+	if (server.tunnel) {
+		const char *argv[] = { server.tunnel, NULL };
+		struct child_process tunnel = {NULL};
+
+		fprintf(stderr, "Starting tunnel '%s'... ", server.tunnel);
+
+		tunnel.argv = argv;
+		tunnel.use_shell = 1;
+		tunnel.in = -1;
+		tunnel.out = -1;
+		if (start_command(&tunnel))
+			die("cannot start proxy %s", argv[0]);
+
+		curl_socket_t sockfd = tunnel.out; // what about tunnel.in ?
+
+		/* call this function to get a socket */
+		curl_easy_setopt(curl, CURLOPT_OPENSOCKETFUNCTION, opensocket);
+		curl_easy_setopt(curl, CURLOPT_OPENSOCKETDATA, &sockfd);
+
+		/* call this function to set options for the socket */
+		curl_easy_setopt(curl, CURLOPT_SOCKOPTFUNCTION, sockopt_callback);
+	}
+
+	curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, server.ssl_verify);
+	curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, server.ssl_verify);
+
+	curl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);
+	curl_easy_setopt(curl, CURLOPT_READDATA, &msg);
+	curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
+
+	/* curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L); */
+
 	fprintf(stderr, "sending %d message%s\n", total, (total != 1) ? "s" : "");
-	ctx->name = imap_folder;
+
 	while (1) {
 		unsigned percent = n * 100 / total;
 
 		fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
-		if (!split_msg(&all_msgs, &msg, &ofs))
+		int prev_len = msg.buf.len;
+		if (!split_msg(&all_msgs, &msg.buf, &ofs))
 			break;
 		if (server.use_html)
-			wrap_in_html(&msg);
-		r = imap_store_msg(ctx, &msg);
-		if (r != DRV_OK)
+			wrap_in_html(&msg.buf);
+
+		curl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE, (curl_off_t)(msg.buf.len-prev_len));
+
+		r = curl_easy_perform(curl);
+
+		if(r != CURLE_OK) {
+			fprintf(stderr, "curl_easy_perform() failed: %s\n",
+					curl_easy_strerror(r));
 			break;
+		}
 		n++;
 	}
 	fprintf(stderr, "\n");
 
-	imap_close_store(ctx);
-
+	curl_easy_cleanup(curl);
+	curl_global_cleanup();
 	return 0;
 }


^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2014-08-25 21:09 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-14 21:46 [PATCH/RFC] git-imap-send: use libcurl for implementation Bernhard Reiter
2014-08-19 17:51 ` Junio C Hamano
2014-08-25 20:11   ` Bernhard Reiter
2014-08-25 21:08     ` Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2014-08-12 21:50 Bernhard Reiter
2014-08-13  1:59 ` Jonathan Nieder
2014-08-17  8:30   ` Jeff King
2014-08-17 12:56     ` Bernhard Reiter
2014-08-17 18:42       ` Jeff King
2014-08-19 11:14         ` Bernhard Reiter
2014-08-19 17:13           ` Junio C Hamano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.