{"thread":{"id":"37894","subject":"Re: [PATCH] imap-send: use libcurl for implementation","startedAt":"2014-11-07T12:46:35Z","lastAt":"2014-11-07T12:46:35Z","messageCount":1,"participants":["Bernhard Reiter"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"251453","messageId":"545CBF2B.3050607@raz.or.at","threadId":"37894","inReplyTo":null,"subject":"Re: [PATCH] imap-send: use libcurl for implementation","fromName":"Bernhard Reiter","fromEmail":"ockham@raz.or.at","sentAt":"2014-11-07T12:46:35Z","receivedAt":"2014-11-07T12:46:35Z","isPatch":true,"sender":{"key":"ockham@raz.or.at","avatar":"https://avatars.githubusercontent.com/u/96308?v=4"},"body":"Zitat von Junio C Hamano <gitster@pobox.com>:\n\n> Bernhard Reiter <ockham@raz.or.at> writes:\n>\n>> @@ -25,7 +25,6 @@ Typical usage is something like:\n>>\n>>  git format-patch --signoff --stdout --attach origin | git imap-send\n>>\n>> -\n>>  OPTIONS\n>\n> Why?\n\nBy mistake when rebasing. Sorry, fixed. (Same for previous -1360,12 hunk.)\n\n>> @@ -37,6 +36,17 @@ OPTIONS\n>>  --quiet::\n>>  \tBe quiet.\n>>\n>> +--curl::\n>> +\tUse libcurl to communicate with the IMAP server, unless tunneling\n>> +\tinto it.  Only available if git was built with the\n>> +\tUSE_CURL_FOR_IMAP_SEND option set, in which case this is the\n>> +\tdefault behavior.\n>> +\n>> +--no-curl::\n>> +\tTalk to the IMAP server using git's own IMAP routines instead of\n>> +\tusing libcurl.  Only available git was built with the\n>> +\tUSE_CURL_FOR_IMAP_SEND option set; implicitly assumed otherwise.\n>> +\n>\n> I think we tend to spell \"Git\" not \"git\" when we refer to the\n> software suite as a whole.\n>\n> More importantly, the description on these two items are no longer\n> in line with the implementation, aren't they?  We accept these\n> options but warn and a build without libcurl ignores --curl with a\n> warning, and --curl is not default in any build.\n\nFixed.\n\n>> @@ -87,7 +97,9 @@ imap.preformattedHTML::\n>>\n>>  imap.authMethod::\n>>  \tSpecify authenticate method for authentication with IMAP server.\n>> -\tCurrent supported method is 'CRAM-MD5' only. If this is not set\n>> +\tIf git was built with the NO_CURL option, or if your curl version is\n>> +\t< 7.34.0, or if you're running git-imap-send with the --no-curl\n>\n> s/< /older than /;\n>\n> Also quote --no-curl inside bq-pair, i.e. `--no-curl`, as that is\n> something the user will type as-is.\n\nFixed.\n\n> [...]\n>\n>> diff --git a/imap-send.c b/imap-send.c\n>> index 7f9d30e..01ce175 100644\n>> --- a/imap-send.c\n>> +++ b/imap-send.c\n>> @@ -30,13 +30,18 @@\n>>  #ifdef NO_OPENSSL\n>>  typedef void *SSL;\n>>  #endif\n>> +#ifdef USE_CURL_FOR_IMAP_SEND\n>> +#include \"http.h\"\n>> +#endif\n>>\n>>  static int verbosity;\n>> +static int use_curl; /* strictly opt in */\n>>\n>> -static const char * const imap_send_usage[] = { \"git imap-send [-v] \n>> [-q] < <mbox>\", NULL };\n>> +static const char * const imap_send_usage[] = { \"git imap-send [-v] \n>> [-q] [--[no-]curl] < <mbox>\", NULL };\n>>\n>>  static struct option imap_send_options[] = {\n>>  \tOPT__VERBOSITY(&verbosity),\n>> +\tOPT_BOOL(0, \"curl\", &use_curl, \"use libcurl to communicate with \n>> the IMAP server\"),\n>>  \tOPT_END()\n>>  };\n>>\n>> @@ -1344,14 +1349,138 @@ static void git_imap_config(void)\n>>  \tgit_config_get_string(\"imap.authmethod\", &server.auth_method);\n>>  }\n>>\n>> -int main(int argc, char **argv)\n>> -{\n>> -\tstruct strbuf all_msgs = STRBUF_INIT;\n>> +static int append_msgs_to_imap(struct imap_server_conf *server, \n>> struct strbuf* all_msgs, int total) {\n>\n> The opening brace sits on its own line by itself, so\n\nFixed (I hope there wasn't more...)\n\n>> +#ifdef USE_CURL_FOR_IMAP_SEND\n>> +static CURL *setup_curl(struct imap_server_conf *srvc)\n>> +{\n>> +\tCURL *curl;\n>> +\tstruct strbuf path = STRBUF_INIT;\n>> +\tstruct strbuf auth = STRBUF_INIT;\n>> +\n>> +\tif (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)\n>> +\t\tdie(\"curl_global_init failed\");\n>> +\n>> +\tcurl = curl_easy_init();\n>> +\n>> +\tif (!curl)\n>> +\t\tdie(\"curl_easy_init failed\");\n>> +\n>> +\tcurl_easy_setopt(curl, CURLOPT_USERNAME, server.user);\n>> +\tcurl_easy_setopt(curl, CURLOPT_PASSWORD, server.pass);\n>> +\n>> +\tstrbuf_addstr(&path, server.host);\n>> +\tif (!path.len || path.buf[path.len - 1] != '/')\n>> +\t\tstrbuf_addch(&path, '/');\n>> +\tstrbuf_addstr(&path, server.folder);\n>> +\n>> +\tcurl_easy_setopt(curl, CURLOPT_URL, path.buf);\n>> +\tcurl_easy_setopt(curl, CURLOPT_PORT, server.port);\n>> +\n>> +\tif (server.auth_method) {\n>> +\t\tstrbuf_addstr(&auth, \"AUTH=\");\n>> +\t\tstrbuf_addstr(&auth, server.auth_method);\n>> +\t\tcurl_easy_setopt(curl, CURLOPT_LOGIN_OPTIONS, auth.buf);\n>> +\t}\n>\n> Are path.buf and auth.buf leaked here, or does CURL *curl take\n> possession of them by curl_easy_setopt() and not freeing them\n> ourselves is the right thing to do?  Assuming that is the case,\n> perhaps we would want to use strbuf_detach() on &path and &auth\n> to make it clear that is what is going on?\n\nI looked it up at http://curl.haxx.se/libcurl/c/curl_easy_setopt.html --\nchar* parameters are actually string copied by curl_easy_setopt(),\nso I'm now strbuf_release()ing path and auth after use.\n\nBernhard\n\n-- >8 --\n\nUse libcurl's high-level API functions to implement git-imap-send\ninstead of the previous low-level OpenSSL-based functions.\n\nSince version 7.30.0, libcurl's API has been able to communicate with\nIMAP servers. Using those high-level functions instead of the current\nones would reduce imap-send.c by some 1200 lines of code. For now,\nthe old ones are wrapped in #ifdefs, and the new functions are enabled\nby make if curl's version is >= 7.34.0, from which version on curl's\nCURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been\navailable. The low-level functions will still be used for tunneling\ninto the server for now.\n\nAs I don't have access to that many IMAP servers, I haven't been able to\ntest the new code with a wide variety of parameter combinations. I did\ntest both secure and insecure (imaps:// and imap://) connections and\nvalues of \"PLAIN\" and \"LOGIN\" for the authMethod.\n\nSigned-off-by: Bernhard Reiter <ockham@raz.or.at>\n---\n Documentation/git-imap-send.txt |  15 +++-\n INSTALL                         |  15 ++--\n Makefile                        |  16 +++-\n imap-send.c                     | 174 ++++++++++++++++++++++++++++++++++------\n 4 files changed, 184 insertions(+), 36 deletions(-)\n\ndiff --git a/Documentation/git-imap-send.txt b/Documentation/git-imap-send.txt\nindex 0897131..77aacf1 100644\n--- a/Documentation/git-imap-send.txt\n+++ b/Documentation/git-imap-send.txt\n@@ -9,7 +9,7 @@ git-imap-send - Send a collection of patches from stdin to an IMAP folder\n SYNOPSIS\n --------\n [verse]\n-'git imap-send' [-v] [-q]\n+'git imap-send' [-v] [-q] [--[no-]curl]\n \n \n DESCRIPTION\n@@ -37,6 +37,15 @@ OPTIONS\n --quiet::\n \tBe quiet.\n \n+--curl::\n+\tUse libcurl to communicate with the IMAP server, unless tunneling\n+\tinto it.  Ignored if Git was built without the USE_CURL_FOR_IMAP_SEND\n+\toption set.\n+\n+--no-curl::\n+\tTalk to the IMAP server using git's own IMAP routines instead of\n+\tusing libcurl.\n+\n \n CONFIGURATION\n -------------\n@@ -87,7 +96,9 @@ imap.preformattedHTML::\n \n imap.authMethod::\n \tSpecify authenticate method for authentication with IMAP server.\n-\tCurrent supported method is 'CRAM-MD5' only. If this is not set\n+\tIf Git was built with the NO_CURL option, or if your curl version is older\n+\tthan 7.34.0, or if you're running git-imap-send with the `--no-curl`\n+\toption, the only supported method is 'CRAM-MD5'. If this is not set\n \tthen 'git imap-send' uses the basic IMAP plaintext LOGIN command.\n \n Examples\ndiff --git a/INSTALL b/INSTALL\nindex 6ec7a24..ffb071e 100644\n--- a/INSTALL\n+++ b/INSTALL\n@@ -108,18 +108,21 @@ Issues of note:\n \t  so you might need to install additional packages other than Perl\n \t  itself, e.g. Time::HiRes.\n \n-\t- \"openssl\" library is used by git-imap-send to use IMAP over SSL.\n-\t  If you don't need it, use NO_OPENSSL.\n+\t- git-imap-send needs the OpenSSL library to talk IMAP over SSL if\n+\t  you are using libcurl older than 7.34.0.  Otherwise you can use\n+\t  NO_OPENSSL without losing git-imap-send.\n \n \t  By default, git uses OpenSSL for SHA1 but it will use its own\n \t  library (inspired by Mozilla's) with either NO_OPENSSL or\n \t  BLK_SHA1.  Also included is a version optimized for PowerPC\n \t  (PPC_SHA1).\n \n-\t- \"libcurl\" library is used by git-http-fetch and git-fetch.  You\n-\t  might also want the \"curl\" executable for debugging purposes.\n-\t  If you do not use http:// or https:// repositories, you do not\n-\t  have to have them (use NO_CURL).\n+\t- \"libcurl\" library is used by git-http-fetch, git-fetch, and, if\n+\t  the curl version >= 7.34.0, for git-imap-send.  You might also\n+\t  want the \"curl\" executable for debugging purposes. If you do not\n+\t  use http:// or https:// repositories, and do not want to put\n+\t  patches into an IMAP mailbox, you do not have to have them\n+\t  (use NO_CURL).\n \n \t- \"expat\" library; git-http-push uses it for remote lock\n \t  management over DAV.  Similar to \"curl\" above, this is optional\ndiff --git a/Makefile b/Makefile\nindex 827006b..fb954a9 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -995,6 +995,9 @@ ifdef HAVE_ALLOCA_H\n \tBASIC_CFLAGS += -DHAVE_ALLOCA_H\n endif\n \n+IMAP_SEND_BUILDDEPS =\n+IMAP_SEND_LDFLAGS = $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)\n+\n ifdef NO_CURL\n \tBASIC_CFLAGS += -DNO_CURL\n \tREMOTE_CURL_PRIMARY =\n@@ -1029,6 +1032,15 @@ else\n \t\t\tPROGRAM_OBJS += http-push.o\n \t\tendif\n \tendif\n+\tcurl_check := $(shell (echo 072200; curl-config --vernum) 2>/dev/null | sort -r | sed -ne 2p)\n+\tifeq \"$(curl_check)\" \"072200\"\n+\t\tUSE_CURL_FOR_IMAP_SEND = YesPlease\n+\tendif\n+\tifdef USE_CURL_FOR_IMAP_SEND\n+\t\tBASIC_CFLAGS += -DUSE_CURL_FOR_IMAP_SEND\n+\t\tIMAP_SEND_BUILDDEPS = http.o\n+\t\tIMAP_SEND_LDFLAGS += $(CURL_LIBCURL)\n+\tendif\n \tifndef NO_EXPAT\n \t\tifdef EXPATDIR\n \t\t\tBASIC_CFLAGS += -I$(EXPATDIR)/include\n@@ -1895,9 +1907,9 @@ endif\n git-%$X: %.o GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)\n \n-git-imap-send$X: imap-send.o GIT-LDFLAGS $(GITLIBS)\n+git-imap-send$X: imap-send.o $(IMAP_SEND_BUILDDEPS) GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\n-\t\t$(LIBS) $(OPENSSL_LINK) $(OPENSSL_LIBSSL) $(LIB_4_CRYPTO)\n+\t\t$(LIBS) $(IMAP_SEND_LDFLAGS)\n \n git-http-fetch$X: http.o http-walker.o http-fetch.o GIT-LDFLAGS $(GITLIBS)\n \t$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \\\ndiff --git a/imap-send.c b/imap-send.c\nindex 7f9d30e..78fba00 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -30,13 +30,18 @@\n #ifdef NO_OPENSSL\n typedef void *SSL;\n #endif\n+#ifdef USE_CURL_FOR_IMAP_SEND\n+#include \"http.h\"\n+#endif\n \n static int verbosity;\n+static int use_curl; /* strictly opt in */\n \n-static const char * const imap_send_usage[] = { \"git imap-send [-v] [-q] < <mbox>\", NULL };\n+static const char * const imap_send_usage[] = { \"git imap-send [-v] [-q] [--[no-]curl] < <mbox>\", NULL };\n \n static struct option imap_send_options[] = {\n \tOPT__VERBOSITY(&verbosity),\n+\tOPT_BOOL(0, \"curl\", &use_curl, \"use libcurl to communicate with the IMAP server\"),\n \tOPT_END()\n };\n \n@@ -1344,14 +1349,141 @@ static void git_imap_config(void)\n \tgit_config_get_string(\"imap.authmethod\", &server.auth_method);\n }\n \n-int main(int argc, char **argv)\n+static int append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total)\n {\n-\tstruct strbuf all_msgs = STRBUF_INIT;\n \tstruct strbuf msg = STRBUF_INIT;\n \tstruct imap_store *ctx = NULL;\n \tint ofs = 0;\n \tint r;\n-\tint total, n = 0;\n+\tint n = 0;\n+\n+\tctx = imap_open_store(server, server->folder);\n+\tif (!ctx) {\n+\t\tfprintf(stderr, \"failed to open store\\n\");\n+\t\treturn 1;\n+\t}\n+\tctx->name = server->folder;\n+\n+\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n+\twhile (1) {\n+\t\tunsigned percent = n * 100 / total;\n+\n+\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n+\n+\t\tif (!split_msg(all_msgs, &msg, &ofs))\n+\t\t\tbreak;\n+\t\tif (server->use_html)\n+\t\t\twrap_in_html(&msg);\n+\t\tr = imap_store_msg(ctx, &msg);\n+\t\tif (r != DRV_OK)\n+\t\t\tbreak;\n+\t\tn++;\n+\t}\n+\tfprintf(stderr, \"\\n\");\n+\n+\timap_close_store(ctx);\n+\n+\treturn 0;\n+}\n+\n+#ifdef USE_CURL_FOR_IMAP_SEND\n+static CURL *setup_curl(struct imap_server_conf *srvc)\n+{\n+\tCURL *curl;\n+\tstruct strbuf path = STRBUF_INIT;\n+\tstruct strbuf auth = STRBUF_INIT;\n+\n+\tif (curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK)\n+\t\tdie(\"curl_global_init failed\");\n+\n+\tcurl = curl_easy_init();\n+\n+\tif (!curl)\n+\t\tdie(\"curl_easy_init failed\");\n+\n+\tcurl_easy_setopt(curl, CURLOPT_USERNAME, server.user);\n+\tcurl_easy_setopt(curl, CURLOPT_PASSWORD, server.pass);\n+\n+\tstrbuf_addstr(&path, server.host);\n+\tif (!path.len || path.buf[path.len - 1] != '/')\n+\t\tstrbuf_addch(&path, '/');\n+\tstrbuf_addstr(&path, server.folder);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_URL, path.buf);\n+\tstrbuf_release(&path);\n+\tcurl_easy_setopt(curl, CURLOPT_PORT, server.port);\n+\n+\tif (server.auth_method) {\n+\t\tstrbuf_addstr(&auth, \"AUTH=\");\n+\t\tstrbuf_addstr(&auth, server.auth_method);\n+\t\tcurl_easy_setopt(curl, CURLOPT_LOGIN_OPTIONS, auth.buf);\n+\t}\n+\tstrbuf_release(&auth);\n+\n+\tif (server.use_ssl)\n+\t\tcurl_easy_setopt(curl, CURLOPT_USE_SSL, (long)CURLUSESSL_ALL);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, server.ssl_verify);\n+\tcurl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, server.ssl_verify);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_READFUNCTION, fread_buffer);\n+\n+\tcurl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);\n+\n+\tif (0 < verbosity)\n+\t\tcurl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);\n+\n+\treturn curl;\n+}\n+\n+static int curl_append_msgs_to_imap(struct imap_server_conf *server, struct strbuf* all_msgs, int total) {\n+\tint ofs = 0;\n+\tint n = 0;\n+\tstruct buffer msgbuf = { STRBUF_INIT, 0 };\n+\tCURL *curl;\n+\tCURLcode res = CURLE_OK;\n+\n+\tcurl = setup_curl(server);\n+\tcurl_easy_setopt(curl, CURLOPT_READDATA, &msgbuf);\n+\n+\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n+\twhile (1) {\n+\t\tunsigned percent = n * 100 / total;\n+\n+\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n+\n+\t\tint prev_len = msgbuf.buf.len;\n+\t\tif (!split_msg(all_msgs, &msgbuf.buf, &ofs))\n+\t\t\tbreak;\n+\t\tif (server->use_html)\n+\t\t\twrap_in_html(&msgbuf.buf);\n+\t\tlf_to_crlf(&msgbuf.buf);\n+\n+\t\tcurl_easy_setopt(curl, CURLOPT_INFILESIZE_LARGE, (curl_off_t)(msgbuf.buf.len-prev_len));\n+\n+\t\tres = curl_easy_perform(curl);\n+\n+\t\tif(res != CURLE_OK) {\n+\t\t\tfprintf(stderr, \"curl_easy_perform() failed: %s\\n\",\n+\t\t\t\t\tcurl_easy_strerror(res));\n+\t\t\tbreak;\n+\t\t}\n+\n+\t\tn++;\n+\t}\n+\tfprintf(stderr, \"\\n\");\n+\n+\tcurl_easy_cleanup(curl);\n+\tcurl_global_cleanup();\n+\n+\treturn 0;\n+}\n+#endif\n+\n+int main(int argc, char **argv)\n+{\n+\tstruct strbuf all_msgs = STRBUF_INIT;\n+\tint total;\n \tint nongit_ok;\n \n \tgit_extract_argv0_path(argv[0]);\n@@ -1366,6 +1498,13 @@ int main(int argc, char **argv)\n \tif (argc)\n \t\tusage_with_options(imap_send_usage, imap_send_options);\n \n+#ifndef USE_CURL_FOR_IMAP_SEND\n+\tif (use_curl) {\n+\t\twarning(\"--use-curl not supported in this build\");\n+\t\tuse_curl = 0;\n+\t}\n+#endif\n+\n \tif (!server.port)\n \t\tserver.port = server.use_ssl ? 993 : 143;\n \n@@ -1399,29 +1538,12 @@ int main(int argc, char **argv)\n \t}\n \n \t/* write it to the imap server */\n-\tctx = imap_open_store(&server, server.folder);\n-\tif (!ctx) {\n-\t\tfprintf(stderr, \"failed to open store\\n\");\n-\t\treturn 1;\n-\t}\n-\n-\tfprintf(stderr, \"sending %d message%s\\n\", total, (total != 1) ? \"s\" : \"\");\n-\twhile (1) {\n-\t\tunsigned percent = n * 100 / total;\n \n-\t\tfprintf(stderr, \"%4u%% (%d/%d) done\\r\", percent, n, total);\n-\t\tif (!split_msg(&all_msgs, &msg, &ofs))\n-\t\t\tbreak;\n-\t\tif (server.use_html)\n-\t\t\twrap_in_html(&msg);\n-\t\tr = imap_store_msg(ctx, &msg);\n-\t\tif (r != DRV_OK)\n-\t\t\tbreak;\n-\t\tn++;\n-\t}\n-\tfprintf(stderr, \"\\n\");\n+\tif (server.tunnel)\n+\t\treturn append_msgs_to_imap(&server, &all_msgs, total);\n \n-\timap_close_store(ctx);\n+\tif (use_curl)\n+\t\treturn curl_append_msgs_to_imap(&server, &all_msgs, total);\n \n-\treturn 0;\n+\treturn append_msgs_to_imap(&server, &all_msgs, total);\n }\n-- \n2.2.0.rc0.4.gb4b9e19.dirty\n"}]}