From 825041fc8c2b36335cd1dfb62c2ae68a9399a015 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Fri, 1 May 2015 11:45:06 +0200 Subject: [PATCH 01/12] make the bdb check actually check for a linkable library it only checked whether the header is compilable. amends e1d0ea8a1. --- configure.ac | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index d7d3404..d9a0800 100644 --- a/configure.ac +++ b/configure.ac @@ -141,11 +141,16 @@ AC_SUBST(SASL_LIBS) AC_CACHE_CHECK([for Berkley DB >= 4.2], ac_cv_berkdb4, [ac_cv_berkdb4=no + sav_LDFLAGS=$LDFLAGS + LDFLAGS="$LDFLAGS -ldb" AC_TRY_LINK([#include ], [DB *db; + db_create(&db, 0, 0); db->truncate(db, 0, 0, 0); db->open(db, 0, "foo", "foo", DB_HASH, DB_CREATE, 0)], - [ac_cv_berkdb4=yes])]) + [ac_cv_berkdb4=yes]) + LDFLAGS=$sav_LDFLAGS + ]) if test "x$ac_cv_berkdb4" = xyes; then AC_SUBST([DB_LIBS], ["-ldb"]) AC_DEFINE(USE_DB, 1, [if Berkley DB should be used]) From 79ef2ab360cc9a50bd1dca4713e254f908fafa5d Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Fri, 1 May 2015 11:48:55 +0200 Subject: [PATCH 02/12] the minimum required bdb version is in fact 4.1 this is the one that introduced the transaction argument to db->open(). --- README | 2 +- configure.ac | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README b/README index aa020de..0800c2e 100644 --- a/README +++ b/README @@ -61,7 +61,7 @@ isync executable still exists; it is a compatibility wrapper around mbsync. * Requirements - Berkley DB 4.2+ (optional) + Berkley DB 4.1+ (optional) OpenSSL for TLS/SSL support (optional) * Installation diff --git a/configure.ac b/configure.ac index d9a0800..d35a2c5 100644 --- a/configure.ac +++ b/configure.ac @@ -139,7 +139,7 @@ if test "x$ob_cv_with_sasl" != xno; then fi AC_SUBST(SASL_LIBS) -AC_CACHE_CHECK([for Berkley DB >= 4.2], ac_cv_berkdb4, +AC_CACHE_CHECK([for Berkley DB >= 4.1], ac_cv_berkdb4, [ac_cv_berkdb4=no sav_LDFLAGS=$LDFLAGS LDFLAGS="$LDFLAGS -ldb" From ef1f80abe3836d166a91ec760178fce4770f6a37 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Fri, 1 May 2015 11:55:27 +0200 Subject: [PATCH 03/12] fix consistent misspelling of Berkeley --- README | 2 +- configure.ac | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/README b/README index 0800c2e..fba2770 100644 --- a/README +++ b/README @@ -61,7 +61,7 @@ isync executable still exists; it is a compatibility wrapper around mbsync. * Requirements - Berkley DB 4.1+ (optional) + Berkeley DB 4.1+ (optional) OpenSSL for TLS/SSL support (optional) * Installation diff --git a/configure.ac b/configure.ac index d35a2c5..1190c68 100644 --- a/configure.ac +++ b/configure.ac @@ -139,7 +139,7 @@ if test "x$ob_cv_with_sasl" != xno; then fi AC_SUBST(SASL_LIBS) -AC_CACHE_CHECK([for Berkley DB >= 4.1], ac_cv_berkdb4, +AC_CACHE_CHECK([for Berkeley DB >= 4.1], ac_cv_berkdb4, [ac_cv_berkdb4=no sav_LDFLAGS=$LDFLAGS LDFLAGS="$LDFLAGS -ldb" @@ -153,7 +153,7 @@ AC_CACHE_CHECK([for Berkley DB >= 4.1], ac_cv_berkdb4, ]) if test "x$ac_cv_berkdb4" = xyes; then AC_SUBST([DB_LIBS], ["-ldb"]) - AC_DEFINE(USE_DB, 1, [if Berkley DB should be used]) + AC_DEFINE(USE_DB, 1, [if Berkeley DB should be used]) fi have_zlib= @@ -194,8 +194,8 @@ else AC_MSG_RESULT([Not using zlib]) fi if test "x$ac_cv_berkdb4" = xyes; then - AC_MSG_RESULT([Using Berkley DB]) + AC_MSG_RESULT([Using Berkeley DB]) else - AC_MSG_RESULT([Not using Berkley DB]) + AC_MSG_RESULT([Not using Berkeley DB]) fi AC_MSG_RESULT() From ea9f4f0b964685c0d9d66d860b08e5dbd4584d4b Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Fri, 1 May 2015 18:39:04 +0200 Subject: [PATCH 04/12] use \fB and \fI consistently, take 2 \fB means literal, while \fI means placeholder, value for placeholder, or emphasis. --- src/mbsync.1 | 96 ++++++++++++++++++++++++++-------------------------- 1 file changed, 48 insertions(+), 48 deletions(-) diff --git a/src/mbsync.1 b/src/mbsync.1 index d525637..b24a854 100644 --- a/src/mbsync.1 +++ b/src/mbsync.1 @@ -192,17 +192,17 @@ See \fBRECOMMENDATIONS\fR and \fBINHERENT PROBLEMS\fR below. (Default: none) .. .TP -\fBTrashNewOnly\fR \fIyes\fR|\fIno\fR +\fBTrashNewOnly\fR \fByes\fR|\fBno\fR When trashing, copy only not yet propagated messages. This makes sense if the -remote Store has a \fBTrash\fR as well (with \fBTrashNewOnly\fR \fIno\fR). -(Default: \fIno\fR) +remote Store has a \fBTrash\fR as well (with \fBTrashNewOnly\fR \fBno\fR). +(Default: \fBno\fR) .. .TP -\fBTrashRemoteNew\fR \fIyes\fR|\fIno\fR +\fBTrashRemoteNew\fR \fByes\fR|\fBno\fR When expunging the remote Store, copy not yet propagated messages to this Store's \fBTrash\fR. When using this, the remote Store does not need an own \fBTrash\fR at all, yet all messages are archived. -(Default: \fIno\fR) +(Default: \fBno\fR) .. .SS Maildir Stores The reference point for relative \fBPath\fRs is the current working directory. @@ -237,11 +237,11 @@ Use \fBmdconvert\fR to convert mailboxes from one scheme to the other. Define the Maildir Store \fIname\fR, opening a section for its parameters. .. .TP -\fBAltMap\fR \fIyes\fR|\fIno\fR +\fBAltMap\fR \fByes\fR|\fBno\fR Use the \fBalternative\fR UID storage scheme for mailboxes in this Store. This does not affect mailboxes that do already have a UID storage scheme; use \fBmdconvert\fR to change it. -(Default: \fIno\fR) +(Default: \fBno\fR) .. .TP \fBInbox\fR \fIpath\fR @@ -266,7 +266,7 @@ Define the IMAP4 Account \fIname\fR, opening a section for its parameters. Specify the DNS name or IP address of the IMAP server. .br If \fBTunnel\fR is used, this setting is needed only if \fBSSLType\fR is -not \fINone\fR and \fBCertificateFile\fR is not used, +not \fBNone\fR and \fBCertificateFile\fR is not used, in which case the host name is used for certificate subject verification. .. .TP @@ -283,7 +283,7 @@ Specify the login name on the IMAP server. .TP \fBPass\fR \fIpassword\fR Specify the password for \fIusername\fR on the IMAP server. -Note that this option is \fBNOT\fR required. +Note that this option is \fInot\fR required. If neither a password nor a password command is specified in the configuration file, \fBmbsync\fR will prompt you for a password. .. @@ -315,21 +315,21 @@ of this list, the list supplied by the server, and the installed SASL modules. (Default: \fB*\fR) .. .TP -\fBSSLType\fR {\fINone\fR|\fISTARTTLS\fR|\fIIMAPS\fR} +\fBSSLType\fR {\fBNone\fR|\fBSTARTTLS\fR|\fBIMAPS\fR} Select the connection security/encryption method: .br -\fINone\fR - no security. +\fBNone\fR - no security. This is the default when \fBTunnel\fR is set, as tunnels are usually secure. .br -\fISTARTTLS\fR - security is established via the STARTTLS extension +\fBSTARTTLS\fR - security is established via the STARTTLS extension after connecting the regular IMAP port 143. Most servers support this, so it is the default (unless a tunnel is used). .br -\fIIMAPS\fR - security is established by starting SSL/TLS negotiation +\fBIMAPS\fR - security is established by starting SSL/TLS negotiation right after connecting the secure IMAP port 993. .. .TP -\fBSSLVersions\fR [\fISSLv2\fR] [\fISSLv3\fR] [\fITLSv1\fR] [\fITLSv1.1\fR] [\fITLSv1.2\fR] +\fBSSLVersions\fR [\fBSSLv2\fR] [\fBSSLv3\fR] [\fBTLSv1\fR] [\fBTLSv1.1\fR] [\fBTLSv1.2\fR] Select the acceptable SSL/TLS versions. Use of SSLv2 is strongly discouraged for security reasons, but might be the only option on some very old servers. @@ -337,9 +337,9 @@ Generally, the newest TLS version is recommended, but as this confuses some servers, \fBTLSv1\fR is the default. .. .TP -\fBSystemCertificates\fR \fIyes\fR|\fIno\fR +\fBSystemCertificates\fR \fByes\fR|\fBno\fR Whether the system's default root cerificate store should be loaded. -(Default: \fIyes\fR) +(Default: \fByes\fR) .. .TP \fBCertificateFile\fR \fIpath\fR @@ -374,18 +374,18 @@ directly in the Store's section - this makes sense if an Account is used for one Store only anyway. .. .TP -\fBUseNamespace\fR \fIyes\fR|\fIno\fR +\fBUseNamespace\fR \fByes\fR|\fBno\fR Selects whether the server's first "personal" NAMESPACE should be prefixed to mailbox names. Disabling this makes sense for some broken IMAP servers. This option is meaningless if a \fBPath\fR was specified. -(Default: \fIyes\fR) +(Default: \fByes\fR) .. .TP \fBPathDelimiter\fR \fIdelim\fR Specify the server's hierarchy delimiter. (Default: taken from the server's first "personal" NAMESPACE) .br -Do \fBNOT\fR abuse this to re-interpret the hierarchy. +Do \fInot\fR abuse this to re-interpret the hierarchy. Use \fBFlatten\fR instead. .. .SS Channels @@ -438,56 +438,56 @@ If \fIcount\fR is 0, the maximum number of messages is \fBunlimited\fR (Default: \fI0\fR). .. .TP -\fBExpireUnread\fR \fIyes\fR|\fIno\fR +\fBExpireUnread\fR \fByes\fR|\fBno\fR Selects whether unread messages should be affected by \fBMaxMessages\fR. Normally, unread messages are considered important and thus never expired. This ensures that you never miss new messages even after an extended absence. However, if your archive contains large amounts of unread messages by design, treating them as important would practically defeat \fBMaxMessages\fR. In this case you need to enable this option. -(Default: \fIno\fR). +(Default: \fBno\fR). .. .TP -\fBSync\fR {\fINone\fR|[\fIPull\fR] [\fIPush\fR] [\fINew\fR] [\fIReNew\fR] [\fIDelete\fR] [\fIFlags\fR]|\fIAll\fR} +\fBSync\fR {\fBNone\fR|[\fBPull\fR] [\fBPush\fR] [\fBNew\fR] [\fBReNew\fR] [\fBDelete\fR] [\fBFlags\fR]|\fBAll\fR} Select the synchronization operation(s) to perform: .br -\fIPull\fR - propagate changes from Master to Slave. +\fBPull\fR - propagate changes from Master to Slave. .br -\fIPush\fR - propagate changes from Slave to Master. +\fBPush\fR - propagate changes from Slave to Master. .br -\fINew\fR - propagate newly appeared messages. +\fBNew\fR - propagate newly appeared messages. .br -\fIReNew\fR - previously refused messages are re-evaluated for propagation. +\fBReNew\fR - previously refused messages are re-evaluated for propagation. Useful after flagging affected messages in the source Store or enlarging MaxSize in the destination Store. .br -\fIDelete\fR - propagate message deletions. This applies only to messages that +\fBDelete\fR - propagate message deletions. This applies only to messages that are actually gone, i.e., were expunged. The affected messages in the remote Store are marked as deleted only, i.e., they won't be really deleted until that Store is expunged. .br -\fIFlags\fR - propagate flag changes. Note that Deleted/Trashed is a flag as +\fBFlags\fR - propagate flag changes. Note that Deleted/Trashed is a flag as well; this is particularly interesting if you use \fBmutt\fR with the maildir_trash option. .br -\fIAll\fR (\fB--full\fR on the command line) - all of the above. +\fBAll\fR (\fB--full\fR on the command line) - all of the above. This is the global default. .br -\fINone\fR (\fB--noop\fR on the command line) - don't propagate anything. +\fBNone\fR (\fB--noop\fR on the command line) - don't propagate anything. Useful if you want to expunge only. .IP -\fIPull\fR and \fIPush\fR are direction flags, while \fINew\fR, \fIReNew\fR, -\fIDelete\fR and \fIFlags\fR are type flags. The two flag classes make up a +\fBPull\fR and \fBPush\fR are direction flags, while \fBNew\fR, \fBReNew\fR, +\fBDelete\fR and \fBFlags\fR are type flags. The two flag classes make up a two-dimensional matrix (a table). Its cells are the individual actions to perform. There are two styles of asserting the cells: .br In the first style, the flags select entire rows/colums in the matrix. Only the cells which are selected both horizontally and vertically are asserted. Specifying no flags from a class is like specifying all flags from this class. -For example, "\fBSync\fR\ \fIPull\fR\ \fINew\fR\ \fIFlags\fR" will propagate +For example, "\fBSync\fR\ \fBPull\fR\ \fBNew\fR\ \fBFlags\fR" will propagate new messages and flag changes from the Master to the Slave, -"\fBSync\fR\ \fINew\fR\ \fIDelete\fR" will propagate message arrivals and -deletions both ways, and "\fBSync\fR\ \fIPush\fR" will propagate all changes +"\fBSync\fR\ \fBNew\fR\ \fBDelete\fR" will propagate message arrivals and +deletions both ways, and "\fBSync\fR\ \fBPush\fR" will propagate all changes from the Slave to the Master. .br In the second style, direction flags are concatenated with type flags; every @@ -495,22 +495,22 @@ compound flag immediately asserts a cell in the matrix. In addition to at least one compound flag, the individual flags can be used as well, but as opposed to the first style, they immediately assert all cells in their respective row/column. For example, -"\fBSync\fR\ \fIPullNew\fR\ \fIPullDelete\fR\ \fIPush\fR" will propagate +"\fBSync\fR\ \fBPullNew\fR\ \fBPullDelete\fR\ \fBPush\fR" will propagate message arrivals and deletions from the Master to the Slave and any changes from the Slave to the Master. Note that it is not allowed to assert a cell in two ways, e.g. -"\fBSync\fR\ \fIPullNew\fR\ \fIPull\fR" and -"\fBSync\fR\ \fIPullNew\fR\ \fIDelete\fR\ \fIPush\fR" induce error messages. +"\fBSync\fR\ \fBPullNew\fR\ \fBPull\fR" and +"\fBSync\fR\ \fBPullNew\fR\ \fBDelete\fR\ \fBPush\fR" induce error messages. .. .TP -\fBCreate\fR {\fINone\fR|\fIMaster\fR|\fISlave\fR|\fIBoth\fR} +\fBCreate\fR {\fBNone\fR|\fBMaster\fR|\fBSlave\fR|\fBBoth\fR} Automatically create missing mailboxes [on the Master/Slave]. Otherwise print an error message and skip that mailbox pair if a mailbox and the corresponding sync state does not exist. -(Global default: \fINone\fR) +(Global default: \fBNone\fR) .. .TP -\fBRemove\fR {\fINone\fR|\fIMaster\fR|\fISlave\fR|\fIBoth\fR} +\fBRemove\fR {\fBNone\fR|\fBMaster\fR|\fBSlave\fR|\fBBoth\fR} Propagate mailbox deletions [to the Master/Slave]. Otherwise print an error message and skip that mailbox pair if a mailbox does not exist but the corresponding sync state does. @@ -520,23 +520,23 @@ mark them as deleted. This ensures compatibility with \fBSyncState *\fR. .br Note that for safety, non-empty mailboxes are never deleted. .br -(Global default: \fINone\fR) +(Global default: \fBNone\fR) .. .TP -\fBExpunge\fR {\fINone\fR|\fIMaster\fR|\fISlave\fR|\fIBoth\fR} +\fBExpunge\fR {\fBNone\fR|\fBMaster\fR|\fBSlave\fR|\fBBoth\fR} Permanently remove all messages [on the Master/Slave] marked for deletion. See \fBRECOMMENDATIONS\fR below. -(Global default: \fINone\fR) +(Global default: \fBNone\fR) .. .TP -\fBCopyArrivalDate\fR {\fIyes\fR|\fIno\fR} +\fBCopyArrivalDate\fR {\fByes\fR|\fBno\fR} Selects whether their arrival time should be propagated together with the messages. Enabling this makes sense in order to keep the time stamp based message sorting intact. Note that IMAP does not guarantee that the time stamp (termed \fBinternal date\fR) is actually the arrival time, but it is usually close enough. -(Default: \fIno\fR) +(Default: \fBno\fR) .. .P \fBSync\fR, \fBCreate\fR, \fBRemove\fR, \fBExpunge\fR, @@ -582,7 +582,7 @@ times within a Group. .. .SS Global Options .TP -\fBFSync\fR \fIyes\fR|\fIno\fR +\fBFSync\fR \fByes\fR|\fBno\fR .br Selects whether \fBmbsync\fR performs forced flushing, which determines the level of data safety after system crashes and power outages. @@ -591,7 +591,7 @@ data=ordered mode. Enabling it is a wise choice for file systems mounted with data=writeback, in particular modern systems like ext4, btrfs and xfs. The performance impact on older file systems may be disproportionate. -(Default: \fIyes\fR) +(Default: \fByes\fR) .. .TP \fBFieldDelimiter\fR \fIdelim\fR From d0494fef43fedcec9fc6d2224c7dae9dd4ad395d Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Fri, 1 May 2015 19:23:16 +0200 Subject: [PATCH 05/12] remove obsolete TODO item amends 74c78c70+b85153f8. --- TODO | 3 --- 1 file changed, 3 deletions(-) diff --git a/TODO b/TODO index 0888b65..6716599 100644 --- a/TODO +++ b/TODO @@ -3,9 +3,6 @@ f{,data}sync() usage could be optimized by batching the calls. add some marker about message being already [remotely] trashed. real transactions would be certainly not particularly useful ... -make sync_chans() aware of servers, so a bad server (e.g., wrong password) -won't cause the same error message for every attached store. - make SSL (connect) timeouts produce a bit more than "Unidentified socket error". network timeout handling in general would be a good idea. From 9ce90dfe015c00daf5baad1a03a6acdb72c9b480 Mon Sep 17 00:00:00 2001 From: Felix Janda Date: Sat, 2 May 2015 18:59:18 +0200 Subject: [PATCH 06/12] Add configure option for zlib --- configure.ac | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index 1190c68..9a30083 100644 --- a/configure.ac +++ b/configure.ac @@ -157,13 +157,18 @@ if test "x$ac_cv_berkdb4" = xyes; then fi have_zlib= -AC_CHECK_LIB([z], [deflate], - [AC_CHECK_HEADER(zlib.h, - [have_zlib=1 - AC_SUBST([Z_LIBS], ["-lz"]) - AC_DEFINE([HAVE_LIBZ], 1, [if you have the zlib library])] - )] -) +AC_ARG_WITH(zlib, + AS_HELP_STRING([--with-zlib], [use zlib [detect]]), + [ob_cv_with_zlib=$withval]) +if test "x$ob_cv_with_zlib" != xno; then + AC_CHECK_LIB([z], [deflate], + [AC_CHECK_HEADER(zlib.h, + [have_zlib=1 + AC_SUBST([Z_LIBS], ["-lz"]) + AC_DEFINE([HAVE_LIBZ], 1, [if you have the zlib library])] + )] + ) +fi AC_ARG_ENABLE(compat, AC_HELP_STRING([--disable-compat], [don't include isync compatibility wrapper [no]]), From 16aa17053dff538463bc39d8b427b1c5713a9ec4 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sat, 9 May 2015 17:06:24 +0200 Subject: [PATCH 07/12] mask AUTHENTICATE PLAIN commands in error output as well amends bd0f3af5. --- src/drv_imap.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/drv_imap.c b/src/drv_imap.c index 85fae81..d3b1447 100644 --- a/src/drv_imap.c +++ b/src/drv_imap.c @@ -1380,7 +1380,11 @@ imap_socket_read( void *aux ) } else /*if (!strcmp( "BAD", arg ))*/ resp = RESP_CANCEL; error( "IMAP command '%s' returned an error: %s %s\n", - !starts_with( cmdp->cmd, -1, "LOGIN", 5 ) ? cmdp->cmd : "LOGIN ", + starts_with( cmdp->cmd, -1, "LOGIN", 5 ) ? + "LOGIN " : + starts_with( cmdp->cmd, -1, "AUTHENTICATE PLAIN", 18 ) ? + "AUTHENTICATE PLAIN " : + cmdp->cmd, arg, cmd ? cmd : "" ); } doresp: From 2f7e60a3ed352fc21ab828c01280f38706bcdd68 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sat, 9 May 2015 17:12:31 +0200 Subject: [PATCH 08/12] fix #ifdefs around AuthMech & RequireCRAM these options don't depend on HAVE_LIBSSL. --- src/drv_imap.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/drv_imap.c b/src/drv_imap.c index d3b1447..f8618d1 100644 --- a/src/drv_imap.c +++ b/src/drv_imap.c @@ -2686,8 +2686,9 @@ imap_parse_store( conffile_t *cfg, store_conf_t **storep ) /* Legacy SSL options */ int require_ssl = -1, use_imaps = -1; int use_sslv2 = -1, use_sslv3 = -1, use_tlsv1 = -1, use_tlsv11 = -1, use_tlsv12 = -1; - int require_cram = -1; #endif + /* Legacy SASL option */ + int require_cram = -1; if (!strcasecmp( "IMAPAccount", cfg->cmd )) { server = nfcalloc( sizeof(*server) ); @@ -2803,6 +2804,7 @@ imap_parse_store( conffile_t *cfg, store_conf_t **storep ) use_tlsv11 = parse_bool( cfg ); else if (!strcasecmp( "UseTLSv1.2", cfg->cmd )) use_tlsv12 = parse_bool( cfg ); +#endif else if (!strcasecmp( "AuthMech", cfg->cmd ) || !strcasecmp( "AuthMechs", cfg->cmd )) { arg = cfg->val; @@ -2811,7 +2813,6 @@ imap_parse_store( conffile_t *cfg, store_conf_t **storep ) while ((arg = get_arg( cfg, ARG_OPTIONAL, 0 ))); } else if (!strcasecmp( "RequireCRAM", cfg->cmd )) require_cram = parse_bool( cfg ); -#endif else if (!strcasecmp( "Tunnel", cfg->cmd )) server->sconf.tunnel = nfstrdup( cfg->val ); else if (store) { @@ -2891,7 +2892,6 @@ imap_parse_store( conffile_t *cfg, store_conf_t **storep ) server->ssl_type = server->sconf.tunnel ? SSL_None : SSL_STARTTLS; } #endif -#ifdef HAVE_LIBSSL if (require_cram >= 0) { if (server->auth_mechs) { error( "%s '%s': The deprecated RequireCRAM option is mutually exlusive with AuthMech.\n", type, name ); @@ -2902,7 +2902,6 @@ imap_parse_store( conffile_t *cfg, store_conf_t **storep ) if (require_cram) add_string_list(&server->auth_mechs, "CRAM-MD5"); } -#endif if (!server->auth_mechs) add_string_list( &server->auth_mechs, "*" ); if (!server->sconf.port) From 6c08f568d0653fb35826b5d1c7cace56d2f02c5a Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sat, 9 May 2015 19:17:41 +0200 Subject: [PATCH 09/12] fix socket_write() recursion the synchronous writing to the socket would have typically invoked the write callback, which would flush further commands, thus recursing. we take the easy way out and make it fully asynchronous, i.e., no data is sent before (re-)entering the event loop. this also has the effect that socket_write() cannot fail any more, and any errors will be reported asynchronously. this is consistent with socket_read(), and produces cleaner code. this introduces a marginal performance regression: the maildir driver is synchronous, so all messages (which fit into memory) will be read before any data is sent. this is not considered relevant. --- src/drv_imap.c | 90 ++++++++++++++++++++++---------------------------- src/socket.c | 30 ++++++++--------- src/socket.h | 4 ++- 3 files changed, 55 insertions(+), 69 deletions(-) diff --git a/src/drv_imap.c b/src/drv_imap.c index f8618d1..6a1394d 100644 --- a/src/drv_imap.c +++ b/src/drv_imap.c @@ -266,7 +266,7 @@ done_imap_cmd( imap_store_t *ctx, struct imap_cmd *cmd, int response ) free( cmd ); } -static int +static void send_imap_cmd( imap_store_t *ctx, struct imap_cmd *cmd ) { int bufl, litplus, iovcnt = 1; @@ -312,19 +312,13 @@ send_imap_cmd( imap_store_t *ctx, struct imap_cmd *cmd ) iov[2].takeOwn = KeepOwn; iovcnt = 3; } - if (socket_write( &ctx->conn, iov, iovcnt ) < 0) - goto bail; + socket_write( &ctx->conn, iov, iovcnt ); if (cmd->param.to_trash && ctx->trashnc == TrashUnknown) ctx->trashnc = TrashChecking; cmd->next = 0; *ctx->in_progress_append = cmd; ctx->in_progress_append = &cmd->next; ctx->num_in_progress++; - return 0; - - bail: - done_imap_cmd( ctx, cmd, RESP_CANCEL ); - return -1; } static int @@ -346,11 +340,10 @@ flush_imap_cmds( imap_store_t *ctx ) { struct imap_cmd *cmd; - while ((cmd = ctx->pending) && cmd_submittable( ctx, cmd )) { + if ((cmd = ctx->pending) && cmd_submittable( ctx, cmd )) { if (!(ctx->pending = cmd->next)) ctx->pending_append = &ctx->pending; - if (send_imap_cmd( ctx, cmd ) < 0) - return -1; + send_imap_cmd( ctx, cmd ); } return 0; } @@ -379,7 +372,7 @@ cancel_submitted_imap_cmds( imap_store_t *ctx ) } } -static int +static void submit_imap_cmd( imap_store_t *ctx, struct imap_cmd *cmd ) { assert( ctx ); @@ -396,10 +389,9 @@ submit_imap_cmd( imap_store_t *ctx, struct imap_cmd *cmd ) *ctx->pending_append = cmd; ctx->pending_append = &cmd->next; } - return 0; + } else { + send_imap_cmd( ctx, cmd ); } - - return send_imap_cmd( ctx, cmd ); } /* Minimal printf() replacement that supports an %\s format sequence to print backslash-escaped @@ -484,7 +476,7 @@ imap_vprintf( const char *fmt, va_list ap ) } } -static int +static void imap_exec( imap_store_t *ctx, struct imap_cmd *cmdp, void (*done)( imap_store_t *ctx, struct imap_cmd *cmd, int response ), const char *fmt, ... ) @@ -497,7 +489,7 @@ imap_exec( imap_store_t *ctx, struct imap_cmd *cmdp, va_start( ap, fmt ); cmdp->cmd = imap_vprintf( fmt, ap ); va_end( ap ); - return submit_imap_cmd( ctx, cmdp ); + submit_imap_cmd( ctx, cmdp ); } static void @@ -1332,8 +1324,7 @@ imap_socket_read( void *aux ) iov[1].buf = "\r\n"; iov[1].len = 2; iov[1].takeOwn = KeepOwn; - if (socket_write( &ctx->conn, iov, 2 ) < 0) - return; + socket_write( &ctx->conn, iov, 2 ); } else if (cmdp->param.cont) { if (cmdp->param.cont( ctx, cmdp, cmd )) return; @@ -1369,9 +1360,8 @@ imap_socket_read( void *aux ) cmd2->orig_cmd = cmdp; cmd2->gen.param.high_prio = 1; p = strchr( cmdp->cmd, '"' ); - if (imap_exec( ctx, &cmd2->gen, get_cmd_result_p2, - "CREATE %.*s", imap_strchr( p + 1, '"' ) - p + 1, p ) < 0) - return; + imap_exec( ctx, &cmd2->gen, get_cmd_result_p2, + "CREATE %.*s", imap_strchr( p + 1, '"' ) - p + 1, p ); continue; } resp = RESP_NO; @@ -1402,8 +1392,7 @@ imap_socket_read( void *aux ) return; } } - if (flush_imap_cmds( ctx ) < 0) - return; + flush_imap_cmds( ctx ); } imap_invoke_bad_callback( ctx ); } @@ -1921,7 +1910,8 @@ do_sasl_auth( imap_store_t *ctx, struct imap_cmd *cmdp ATTR_UNUSED, const char * iov[iovcnt].len = 2; iov[iovcnt].takeOwn = KeepOwn; iovcnt++; - return socket_write( &ctx->conn, iov, iovcnt ); + socket_write( &ctx->conn, iov, iovcnt ); + return 0; bail: imap_open_store_bail( ctx, FAIL_FINAL ); @@ -2281,7 +2271,7 @@ imap_prepare_load_box( store_t *gctx, int opts ) gctx->opts = opts; } -static int imap_submit_load( imap_store_t *, const char *, int, struct imap_cmd_refcounted_state * ); +static void imap_submit_load( imap_store_t *, const char *, int, struct imap_cmd_refcounted_state * ); static void imap_load_box( store_t *gctx, int minuid, int maxuid, int newuid, int *excs, int nexcs, @@ -2308,16 +2298,14 @@ imap_load_box( store_t *gctx, int minuid, int maxuid, int newuid, int *excs, int if (i != j) bl += sprintf( buf + bl, ":%d", excs[i] ); } - if (imap_submit_load( ctx, buf, 0, sts ) < 0) - goto done; + imap_submit_load( ctx, buf, 0, sts ); } if (maxuid == INT_MAX) maxuid = ctx->gen.uidnext ? ctx->gen.uidnext - 1 : 1000000000; if (maxuid >= minuid) { if ((ctx->gen.opts & OPEN_FIND) && minuid < newuid) { sprintf( buf, "%d:%d", minuid, newuid - 1 ); - if (imap_submit_load( ctx, buf, 0, sts ) < 0) - goto done; + imap_submit_load( ctx, buf, 0, sts ); if (newuid > maxuid) goto done; sprintf( buf, "%d:%d", newuid, maxuid ); @@ -2332,14 +2320,14 @@ imap_load_box( store_t *gctx, int minuid, int maxuid, int newuid, int *excs, int } } -static int +static void imap_submit_load( imap_store_t *ctx, const char *buf, int tuids, struct imap_cmd_refcounted_state *sts ) { - return imap_exec( ctx, imap_refcounted_new_cmd( sts ), imap_refcounted_done_box, - "UID FETCH %s (UID%s%s%s)", buf, - (ctx->gen.opts & OPEN_FLAGS) ? " FLAGS" : "", - (ctx->gen.opts & OPEN_SIZE) ? " RFC822.SIZE" : "", - tuids ? " BODY.PEEK[HEADER.FIELDS (X-TUID)]" : ""); + imap_exec( ctx, imap_refcounted_new_cmd( sts ), imap_refcounted_done_box, + "UID FETCH %s (UID%s%s%s)", buf, + (ctx->gen.opts & OPEN_FLAGS) ? " FLAGS" : "", + (ctx->gen.opts & OPEN_SIZE) ? " RFC822.SIZE" : "", + tuids ? " BODY.PEEK[HEADER.FIELDS (X-TUID)]" : ""); } /******************* imap_fetch_msg *******************/ @@ -2396,15 +2384,15 @@ imap_make_flags( int flags, char *buf ) return d; } -static int +static void imap_flags_helper( imap_store_t *ctx, int uid, char what, int flags, struct imap_cmd_refcounted_state *sts ) { char buf[256]; buf[imap_make_flags( flags, buf )] = 0; - return imap_exec( ctx, imap_refcounted_new_cmd( sts ), imap_set_flags_p2, - "UID STORE %d %cFLAGS.SILENT %s", uid, what, buf ); + imap_exec( ctx, imap_refcounted_new_cmd( sts ), imap_set_flags_p2, + "UID STORE %d %cFLAGS.SILENT %s", uid, what, buf ); } static void @@ -2422,8 +2410,10 @@ imap_set_msg_flags( store_t *gctx, message_t *msg, int uid, int add, int del, } if (add || del) { struct imap_cmd_refcounted_state *sts = imap_refcounted_new_state( cb, aux ); - if ((add && imap_flags_helper( ctx, uid, '+', add, sts ) < 0) || - (del && imap_flags_helper( ctx, uid, '-', del, sts ) < 0)) {} + if (add) + imap_flags_helper( ctx, uid, '+', add, sts ); + if (del) + imap_flags_helper( ctx, uid, '-', del, sts ); imap_refcounted_done( sts ); } else { cb( DRV_OK, aux ); @@ -2474,9 +2464,8 @@ imap_close_box( store_t *gctx, } if (!bl) break; - if (imap_exec( ctx, imap_refcounted_new_cmd( sts ), imap_refcounted_done_box, - "UID EXPUNGE %s", buf ) < 0) - break; + imap_exec( ctx, imap_refcounted_new_cmd( sts ), imap_refcounted_done_box, + "UID EXPUNGE %s", buf ); } imap_refcounted_done( sts ); } else { @@ -2617,13 +2606,12 @@ imap_list_store( store_t *gctx, int flags, imap_store_t *ctx = (imap_store_t *)gctx; struct imap_cmd_refcounted_state *sts = imap_refcounted_new_state( cb, aux ); - if (((flags & LIST_PATH) && (!(flags & LIST_INBOX) || !is_inbox( ctx, ctx->prefix, -1 )) && - imap_exec( ctx, imap_refcounted_new_cmd( sts ), imap_refcounted_done_box, - "LIST \"\" \"%\\s*\"", ctx->prefix ) < 0) || - ((flags & LIST_INBOX) && (!(flags & LIST_PATH) || *ctx->prefix) && - imap_exec( ctx, imap_refcounted_new_cmd( sts ), imap_refcounted_done_box, - "LIST \"\" INBOX*" ) < 0)) - {} + if ((flags & LIST_PATH) && (!(flags & LIST_INBOX) || !is_inbox( ctx, ctx->prefix, -1 ))) + imap_exec( ctx, imap_refcounted_new_cmd( sts ), imap_refcounted_done_box, + "LIST \"\" \"%\\s*\"", ctx->prefix ); + if ((flags & LIST_INBOX) && (!(flags & LIST_PATH) || *ctx->prefix)) + imap_exec( ctx, imap_refcounted_new_cmd( sts ), imap_refcounted_done_box, + "LIST \"\" INBOX*" ); imap_refcounted_done( sts ); } diff --git a/src/socket.c b/src/socket.c index 5cde674..8a3b33b 100644 --- a/src/socket.c +++ b/src/socket.c @@ -755,6 +755,7 @@ do_queued_write( conn_t *conn ) return -1; if (n != len) { conn->write_offset += n; + conn->writing = 1; return 0; } conn->write_offset = 0; @@ -764,6 +765,7 @@ do_queued_write( conn_t *conn ) if (conn->ssl && SSL_pending( conn->ssl )) conf_wakeup( &conn->ssl_fake, 0 ); #endif + conn->writing = 0; return conn->write_callback( conn->callback_aux ); } @@ -787,6 +789,8 @@ do_flush( conn_t *conn ) #ifdef HAVE_LIBZ if (conn->out_z) { int buf_avail = conn->append_avail; + if (!conn->z_written) + return; do { if (!bc) { buf_avail = WRITE_CHUNK_SIZE; @@ -812,6 +816,7 @@ do_flush( conn_t *conn ) } while (!conn->out_z->avail_out); conn->append_buf = bc; conn->append_avail = buf_avail; + conn->z_written = 0; } else #endif if (bc) { @@ -823,15 +828,15 @@ do_flush( conn_t *conn ) } } -int +void socket_write( conn_t *conn, conn_iovec_t *iov, int iovcnt ) { int i, buf_avail, len, offset = 0, total = 0; - buff_chunk_t *bc, *exwb = conn->write_buf; + buff_chunk_t *bc; for (i = 0; i < iovcnt; i++) total += iov[i].len; - if (total >= WRITE_CHUNK_SIZE && pending_wakeup( &conn->fd_fake )) { + if (total >= WRITE_CHUNK_SIZE) { /* If the new data is too big, queue the pending buffer to avoid latency. */ do_flush( conn ); } @@ -870,6 +875,7 @@ socket_write( conn_t *conn, conn_iovec_t *iov, int iovcnt ) bc->len = (char *)conn->out_z->next_out - bc->data; buf_avail = conn->out_z->avail_out; len -= conn->out_z->avail_in; + conn->z_written = 1; } else #endif { @@ -898,17 +904,7 @@ socket_write( conn_t *conn, conn_iovec_t *iov, int iovcnt ) #ifdef HAVE_LIBZ conn->append_avail = buf_avail; #endif - /* Queue the pending write once the main loop goes idle. */ - conf_wakeup( &conn->fd_fake, -#ifdef HAVE_LIBZ - /* Always give zlib a chance to flush its internal buffer. */ - conn->out_z || -#endif - bc ? 0 : -1 ); - /* If no writes were queued before, ensure that flushing commences. */ - if (!exwb) - return do_queued_write( conn ); - return 0; + conf_wakeup( &conn->fd_fake, 0 ); } static void @@ -963,10 +959,10 @@ socket_fake_cb( void *aux ) { conn_t *conn = (conn_t *)aux; - buff_chunk_t *exwb = conn->write_buf; + /* Ensure that a pending write gets queued. */ do_flush( conn ); - /* If no writes were queued before, ensure that flushing commences. */ - if (!exwb) + /* If no writes are ongoing, start writing now. */ + if (!conn->writing) do_queued_write( conn ); } diff --git a/src/socket.h b/src/socket.h index 7ea6086..dac2576 100644 --- a/src/socket.h +++ b/src/socket.h @@ -83,6 +83,7 @@ typedef struct { #ifdef HAVE_LIBZ z_streamp in_z, out_z; wakeup_t z_fake; + int z_written; #endif void (*bad_callback)( void *aux ); /* async fail while sending or listening */ @@ -100,6 +101,7 @@ typedef struct { /* writing */ buff_chunk_t *append_buf; /* accumulating buffer */ buff_chunk_t *write_buf, **write_buf_append; /* buffer head & tail */ + int writing; #ifdef HAVE_LIBZ int append_avail; /* space left in accumulating buffer */ #endif @@ -145,6 +147,6 @@ typedef struct conn_iovec { int len; ownership_t takeOwn; } conn_iovec_t; -int socket_write( conn_t *sock, conn_iovec_t *iov, int iovcnt ); +void socket_write( conn_t *sock, conn_iovec_t *iov, int iovcnt ); #endif From 02af3f4c732333a9586f733cdceea36417500072 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sat, 9 May 2015 19:18:40 +0200 Subject: [PATCH 10/12] ensure direct exit after calling back any structures may be invalid after callback invocation. this has the side effect that the socket write callback now returns void, like all other callbacks do. --- src/drv_imap.c | 5 ++--- src/socket.c | 5 +++-- src/socket.h | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/drv_imap.c b/src/drv_imap.c index 6a1394d..31fdf28 100644 --- a/src/drv_imap.c +++ b/src/drv_imap.c @@ -335,7 +335,7 @@ cmd_submittable( imap_store_t *ctx, struct imap_cmd *cmd ) ctx->num_in_progress < ((imap_store_conf_t *)ctx->gen.conf)->server->max_in_progress; } -static int +static void flush_imap_cmds( imap_store_t *ctx ) { struct imap_cmd *cmd; @@ -345,7 +345,6 @@ flush_imap_cmds( imap_store_t *ctx ) ctx->pending_append = &ctx->pending; send_imap_cmd( ctx, cmd ); } - return 0; } static void @@ -1589,7 +1588,7 @@ imap_open_store( store_conf_t *conf, const char *label, socket_init( &ctx->conn, &srvc->sconf, (void (*)( void * ))imap_invoke_bad_callback, - imap_socket_read, (int (*)(void *))flush_imap_cmds, ctx ); + imap_socket_read, (void (*)(void *))flush_imap_cmds, ctx ); socket_connect( &ctx->conn, imap_open_store_connected ); } diff --git a/src/socket.c b/src/socket.c index 8a3b33b..5c1ce93 100644 --- a/src/socket.c +++ b/src/socket.c @@ -80,7 +80,7 @@ ssl_return( const char *func, conn_t *conn, int ret ) /* Callers take the short path out, so signal higher layers from here. */ conn->state = SCK_EOF; conn->read_callback( conn->callback_aux ); - return 0; + return -1; } sys_error( "Socket error: secure %s %s", func, conn->name ); } else { @@ -766,7 +766,8 @@ do_queued_write( conn_t *conn ) conf_wakeup( &conn->ssl_fake, 0 ); #endif conn->writing = 0; - return conn->write_callback( conn->callback_aux ); + conn->write_callback( conn->callback_aux ); + return -1; } static void diff --git a/src/socket.h b/src/socket.h index dac2576..84ae0a2 100644 --- a/src/socket.h +++ b/src/socket.h @@ -88,7 +88,7 @@ typedef struct { void (*bad_callback)( void *aux ); /* async fail while sending or listening */ void (*read_callback)( void *aux ); /* data available for reading */ - int (*write_callback)( void *aux ); /* all *queued* data was sent */ + void (*write_callback)( void *aux ); /* all *queued* data was sent */ union { void (*connect)( int ok, void *aux ); void (*starttls)( int ok, void *aux ); @@ -123,7 +123,7 @@ static INLINE void socket_init( conn_t *conn, const server_conf_t *conf, void (*bad_callback)( void *aux ), void (*read_callback)( void *aux ), - int (*write_callback)( void *aux ), + void (*write_callback)( void *aux ), void *aux ) { conn->conf = conf; From 2013e50b1c6c1f37a791e72a59ee8e54d23653fb Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sat, 9 May 2015 17:44:36 +0200 Subject: [PATCH 11/12] rename misnamed functions concerning sending imap commands cmd_submittable() => cmd_sendable() cancel_submitted_imap_cmds() => cancel_sent_imap_cmds() the sequence is exec -> submit -> send. --- src/drv_imap.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/drv_imap.c b/src/drv_imap.c index 31fdf28..dcb372d 100644 --- a/src/drv_imap.c +++ b/src/drv_imap.c @@ -322,7 +322,7 @@ send_imap_cmd( imap_store_t *ctx, struct imap_cmd *cmd ) } static int -cmd_submittable( imap_store_t *ctx, struct imap_cmd *cmd ) +cmd_sendable( imap_store_t *ctx, struct imap_cmd *cmd ) { struct imap_cmd *cmdp; @@ -340,7 +340,7 @@ flush_imap_cmds( imap_store_t *ctx ) { struct imap_cmd *cmd; - if ((cmd = ctx->pending) && cmd_submittable( ctx, cmd )) { + if ((cmd = ctx->pending) && cmd_sendable( ctx, cmd )) { if (!(ctx->pending = cmd->next)) ctx->pending_append = &ctx->pending; send_imap_cmd( ctx, cmd ); @@ -360,7 +360,7 @@ cancel_pending_imap_cmds( imap_store_t *ctx ) } static void -cancel_submitted_imap_cmds( imap_store_t *ctx ) +cancel_sent_imap_cmds( imap_store_t *ctx ) { struct imap_cmd *cmd; @@ -379,7 +379,7 @@ submit_imap_cmd( imap_store_t *ctx, struct imap_cmd *cmd ) assert( cmd ); assert( cmd->param.done ); - if ((ctx->pending && !cmd->param.high_prio) || !cmd_submittable( ctx, cmd )) { + if ((ctx->pending && !cmd->param.high_prio) || !cmd_sendable( ctx, cmd )) { if (ctx->pending && cmd->param.high_prio) { cmd->next = ctx->pending; ctx->pending = cmd; @@ -1425,7 +1425,7 @@ imap_cancel_store( store_t *gctx ) sasl_dispose( &ctx->sasl ); #endif socket_close( &ctx->conn ); - cancel_submitted_imap_cmds( ctx ); + cancel_sent_imap_cmds( ctx ); cancel_pending_imap_cmds( ctx ); free_generic_messages( ctx->gen.msgs ); free_string_list( ctx->gen.boxes ); From 4106de5c14755e8587a446d6b851ef6d1659f489 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sat, 9 May 2015 18:00:36 +0200 Subject: [PATCH 12/12] bump version --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 9a30083..aa6708d 100644 --- a/configure.ac +++ b/configure.ac @@ -1,4 +1,4 @@ -AC_INIT([isync], [1.2.0]) +AC_INIT([isync], [1.2.1]) AC_CONFIG_HEADERS([autodefs.h]) AM_INIT_AUTOMAKE