From 56515abe9498cbeac4cd9b690171fcc99a18f352 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 21 Jun 2017 09:33:20 +0200 Subject: [PATCH 1/5] socket: use next addrinfo if opening socket fails The `socket_connect_one` function previously did an `exit(1)` when encountering any errors with opening the socket. This would break connecting to a host where multiple possible addrinfos are returned, where the leading addrinfos are in fact impossible to connect to. E.g. with a kernel configured without support for IPv6, the `getaddrinfo` call may still return a hint containing an IPv6 address alongside another hint with an IPv4 address. Creating the socket with the IPv6 address, which will cause an error, lead us to exiting early without even trying remaining hints. While one can argue that the user should have compiled without HAVE_IPV6 or used an appropriate DNS configuration, we can do better by simply skipping over the current addrinfo causing an error. To do so, we split out a new function `socket_connect_next`, which selects the next available address info and subsequently calls `socket_connect_one` again. When no hints remain, `sock_connect_one` will error out at that point. --- src/socket.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/socket.c b/src/socket.c index 3736fd5..c806e2e 100644 --- a/src/socket.c +++ b/src/socket.c @@ -329,6 +329,7 @@ static void socket_fd_cb( int, void * ); static void socket_fake_cb( void * ); static void socket_connect_one( conn_t * ); +static void socket_connect_next( conn_t * ); static void socket_connect_failed( conn_t * ); static void socket_connected( conn_t * ); static void socket_connect_bail( conn_t * ); @@ -469,8 +470,8 @@ socket_connect_one( conn_t *sock ) s = socket( PF_INET, SOCK_STREAM, 0 ); #endif if (s < 0) { - perror( "socket" ); - exit( 1 ); + socket_connect_next( sock ); + return; } socket_open_internal( sock, s ); @@ -494,10 +495,9 @@ socket_connect_one( conn_t *sock ) } static void -socket_connect_failed( conn_t *conn ) +socket_connect_next( conn_t *conn ) { sys_error( "Cannot connect to %s", conn->name ); - socket_close_internal( conn ); free( conn->name ); conn->name = 0; #ifdef HAVE_IPV6 @@ -508,6 +508,13 @@ socket_connect_failed( conn_t *conn ) socket_connect_one( conn ); } +static void +socket_connect_failed( conn_t *conn ) +{ + socket_close_internal( conn ); + socket_connect_next( conn ); +} + static void socket_connected( conn_t *conn ) { From 366ed7d7621f70afc761166bde330aac32e1b2cb Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sun, 30 Jul 2017 13:47:51 +0200 Subject: [PATCH 2/5] improve zlib error reporting zlib is not exactly thorough about always populating z_stream->msg, so fall back to the error code if necessary. --- src/socket.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/socket.c b/src/socket.c index c806e2e..4677c67 100644 --- a/src/socket.c +++ b/src/socket.c @@ -292,6 +292,14 @@ static void start_tls_p3( conn_t *conn, int ok ) static void z_fake_cb( void * ); +static const char * +z_err_msg( int code, z_streamp strm ) +{ + /* zlib's consistency in populating z_stream->msg is somewhat + * less than stellar. zError() is undocumented. */ + return strm->msg ? strm->msg : zError( code ); +} + void socket_start_deflate( conn_t *conn ) { @@ -303,7 +311,7 @@ socket_start_deflate( conn_t *conn ) -15 /* Use raw deflate */ ); if (result != Z_OK) { - error( "Fatal: Cannot initialize decompression: %s\n", conn->in_z->msg ); + error( "Fatal: Cannot initialize decompression: %s\n", z_err_msg( result, conn->in_z ) ); abort(); } @@ -317,7 +325,7 @@ socket_start_deflate( conn_t *conn ) Z_DEFAULT_STRATEGY /* Don't try to do anything fancy */ ); if (result != Z_OK) { - error( "Fatal: Cannot initialize compression: %s\n", conn->out_z->msg ); + error( "Fatal: Cannot initialize compression: %s\n", z_err_msg( result, conn->out_z ) ); abort(); } @@ -630,7 +638,7 @@ socket_fill_z( conn_t *sock ) ret = inflate( sock->in_z, Z_SYNC_FLUSH ); if (ret != Z_OK && ret != Z_STREAM_END) { - error( "Error decompressing data from %s: %s\n", sock->name, sock->in_z->msg ); + error( "Error decompressing data from %s: %s\n", sock->name, z_err_msg( ret, sock->in_z ) ); socket_fail( sock ); return; } @@ -806,6 +814,7 @@ do_flush( conn_t *conn ) if (!conn->z_written) return; do { + int ret; if (!bc) { buf_avail = WRITE_CHUNK_SIZE; bc = nfmalloc( offsetof(buff_chunk_t, data) + buf_avail ); @@ -815,8 +824,8 @@ do_flush( conn_t *conn ) conn->out_z->avail_in = 0; conn->out_z->next_out = (uchar *)bc->data + bc->len; conn->out_z->avail_out = buf_avail; - if (deflate( conn->out_z, Z_PARTIAL_FLUSH ) != Z_OK) { - error( "Fatal: Compression error: %s\n", conn->out_z->msg ); + if ((ret = deflate( conn->out_z, Z_PARTIAL_FLUSH )) != Z_OK) { + error( "Fatal: Compression error: %s\n", z_err_msg( ret, conn->out_z ) ); abort(); } bc->len = (char *)conn->out_z->next_out - bc->data; @@ -878,12 +887,13 @@ socket_write( conn_t *conn, conn_iovec_t *iov, int iovcnt ) len = iov->len - offset; #ifdef HAVE_LIBZ if (conn->out_z) { + int ret; conn->out_z->next_in = (uchar *)iov->buf + offset; conn->out_z->avail_in = len; conn->out_z->next_out = (uchar *)bc->data + bc->len; conn->out_z->avail_out = buf_avail; - if (deflate( conn->out_z, Z_NO_FLUSH ) != Z_OK) { - error( "Fatal: Compression error: %s\n", conn->out_z->msg ); + if ((ret = deflate( conn->out_z, Z_NO_FLUSH )) != Z_OK) { + error( "Fatal: Compression error: %s\n", z_err_msg( ret, conn->out_z ) ); abort(); } bc->len = (char *)conn->out_z->next_out - bc->data; From ada0ae4b8e972fbdb6a4f0b6854b81177f6c5cf4 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sun, 30 Jul 2017 18:37:58 +0200 Subject: [PATCH 3/5] fix spurious decompression errors while that's just bad api, inflate() can return Z_BUF_ERROR during normal operation. contrary to the zpipe example and what the documentation implies, deflate() actually isn't that braindead. add respective comments. REFMAIL: CALA3aExMjtRL0tAmgUANpDTnn-_HJ0sYkOEXWzoO6DVaiNFUHQ@mail.gmail.com --- src/socket.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/socket.c b/src/socket.c index 4677c67..9357b8c 100644 --- a/src/socket.c +++ b/src/socket.c @@ -637,7 +637,9 @@ socket_fill_z( conn_t *sock ) sock->in_z->next_out = (unsigned char *)buf; ret = inflate( sock->in_z, Z_SYNC_FLUSH ); - if (ret != Z_OK && ret != Z_STREAM_END) { + /* Z_BUF_ERROR happens here when the previous call both consumed + * all input and exactly filled up the output buffer. */ + if (ret != Z_OK && ret != Z_BUF_ERROR && ret != Z_STREAM_END) { error( "Error decompressing data from %s: %s\n", sock->name, z_err_msg( ret, sock->in_z ) ); socket_fail( sock ); return; @@ -824,6 +826,9 @@ do_flush( conn_t *conn ) conn->out_z->avail_in = 0; conn->out_z->next_out = (uchar *)bc->data + bc->len; conn->out_z->avail_out = buf_avail; + /* Z_BUF_ERROR cannot happen here, as zlib suppresses the error + * both upon increasing the flush level (1st iteration) and upon + * a no-op after the output buffer was full (later iterations). */ if ((ret = deflate( conn->out_z, Z_PARTIAL_FLUSH )) != Z_OK) { error( "Fatal: Compression error: %s\n", z_err_msg( ret, conn->out_z ) ); abort(); @@ -892,6 +897,8 @@ socket_write( conn_t *conn, conn_iovec_t *iov, int iovcnt ) conn->out_z->avail_in = len; conn->out_z->next_out = (uchar *)bc->data + bc->len; conn->out_z->avail_out = buf_avail; + /* Z_BUF_ERROR is impossible here, as the input buffer always has data, + * and the output buffer always has space. */ if ((ret = deflate( conn->out_z, Z_NO_FLUSH )) != Z_OK) { error( "Fatal: Compression error: %s\n", z_err_msg( ret, conn->out_z ) ); abort(); From 906dc989e488c82a8cd94f71fc0438fb0d33e4a5 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sat, 5 Aug 2017 20:24:11 +0200 Subject: [PATCH 4/5] mention the need for renaming Maildir files upon move mu4e config line offered by Ben Maughan . --- src/mbsync.1 | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/mbsync.1 b/src/mbsync.1 index fc572b2..51f356a 100644 --- a/src/mbsync.1 +++ b/src/mbsync.1 @@ -244,6 +244,7 @@ Define the Maildir Store \fIname\fR, opening a section for its parameters. 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. +See \fBRECOMMENDATIONS\fR below. (Default: \fBno\fR) .. .TP @@ -656,6 +657,14 @@ If your server supports auto-trashing (as Gmail does), it is probably a good idea to rely on that instead of \fBmbsync\fR's trash functionality. If you do that, and intend to synchronize the trash like other mailboxes, you should not use \fBmbsync\fR's \fBTrash\fR option at all. +.P +When using the more efficient default UID mapping scheme, it is important +that the MUA renames files when moving them between Maildir folders. +Mutt always does that, while mu4e needs to be configured to do it: +.br +.in +4 +(setq mu4e-change-filenames-when-moving t) +.in -4 .. .SH INHERENT PROBLEMS Changes done after \fBmbsync\fR has retrieved the message list will not be From fed0dcc60e342507ef69083ea32bca19f6b3005b Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sat, 5 Aug 2017 20:28:52 +0200 Subject: [PATCH 5/5] bump version --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 1e46bd0..6f8d44a 100644 --- a/configure.ac +++ b/configure.ac @@ -1,4 +1,4 @@ -AC_INIT([isync], [1.2.1]) +AC_INIT([isync], [1.2.2]) AC_CONFIG_HEADERS([autodefs.h]) AM_INIT_AUTOMAKE