From ec7aec974ee60a48cb04def2cd4440eff01bc3b7 Mon Sep 17 00:00:00 2001 From: Timo Sirainen Date: Thu, 18 May 2017 19:42:03 +0300 Subject: [PATCH] imapc: Fix infinite reconnection when server keeps sending corrupted state When corrupted state was found, imapc_client_mailbox_reconnect() is called to reconnect. This call skipped the normal "is it safe to reconnect?" checks causing potentially infinite reconnections. --- src/lib-imap-client/imapc-client.c | 12 +++--------- src/lib-imap-client/imapc-client.h | 3 ++- src/lib-imap-client/imapc-connection.c | 20 ++++++++++++-------- src/lib-imap-client/imapc-connection.h | 3 +++ src/lib-storage/index/imapc/imapc-mailbox.c | 5 +++-- src/lib-storage/index/imapc/imapc-storage.c | 6 +++--- 6 files changed, 26 insertions(+), 23 deletions(-) diff --git a/src/lib-imap-client/imapc-client.c b/src/lib-imap-client/imapc-client.c index c4133dc1af..7a119ffaf7 100644 --- a/src/lib-imap-client/imapc-client.c +++ b/src/lib-imap-client/imapc-client.c @@ -404,18 +404,12 @@ bool imapc_client_mailbox_can_reconnect(struct imapc_client_mailbox *box) return box->reopen_callback != NULL && box->reconnect_ok; } -void imapc_client_mailbox_reconnect(struct imapc_client_mailbox *box) +void imapc_client_mailbox_reconnect(struct imapc_client_mailbox *box, + const char *errmsg) { i_assert(!box->reconnecting); - box->reconnecting = TRUE; - /* if we fail again, avoid reconnecting immediately. if the server is - broken we could just get into an infinitely failing reconnection - loop. */ - box->reconnect_ok = FALSE; - - imapc_connection_disconnect_full(box->conn, TRUE); - imapc_connection_connect(box->conn); + imapc_connection_try_reconnect(box->conn, errmsg, 0); } void imapc_client_mailbox_close(struct imapc_client_mailbox **_box) diff --git a/src/lib-imap-client/imapc-client.h b/src/lib-imap-client/imapc-client.h index 03afe4f752..86709d6661 100644 --- a/src/lib-imap-client/imapc-client.h +++ b/src/lib-imap-client/imapc-client.h @@ -221,7 +221,8 @@ void imapc_client_mailbox_set_reopen_cb(struct imapc_client_mailbox *box, void *context); void imapc_client_mailbox_close(struct imapc_client_mailbox **box); bool imapc_client_mailbox_can_reconnect(struct imapc_client_mailbox *box); -void imapc_client_mailbox_reconnect(struct imapc_client_mailbox *box); +void imapc_client_mailbox_reconnect(struct imapc_client_mailbox *box, + const char *errmsg); struct imapc_command * imapc_client_mailbox_cmd(struct imapc_client_mailbox *box, imapc_command_callback_t *callback, void *context); diff --git a/src/lib-imap-client/imapc-connection.c b/src/lib-imap-client/imapc-connection.c index 7e71b333c1..bdec9a370c 100644 --- a/src/lib-imap-client/imapc-connection.c +++ b/src/lib-imap-client/imapc-connection.c @@ -508,17 +508,21 @@ static void imapc_connection_reconnect(struct imapc_connection *conn) conn->reconnect_ok = FALSE; conn->reconnect_waiting = FALSE; - if (conn->selected_box != NULL) - imapc_client_mailbox_reconnect(conn->selected_box); - else { - imapc_connection_disconnect_full(conn, TRUE); - imapc_connection_connect(conn); + if (conn->selected_box != NULL) { + i_assert(!conn->selected_box->reconnecting); + conn->selected_box->reconnecting = TRUE; + /* if we fail again, avoid reconnecting immediately. if the + server is broken we could just get into an infinitely + failing reconnection loop. */ + conn->selected_box->reconnect_ok = FALSE; } + imapc_connection_disconnect_full(conn, TRUE); + imapc_connection_connect(conn); } -static void -imapc_connection_try_reconnect(struct imapc_connection *conn, - const char *errstr, unsigned int delay_msecs) +void imapc_connection_try_reconnect(struct imapc_connection *conn, + const char *errstr, + unsigned int delay_msecs) { if (conn->prev_connect_idx + 1 < conn->ips_count) { conn->reconnect_ok = TRUE; diff --git a/src/lib-imap-client/imapc-connection.h b/src/lib-imap-client/imapc-connection.h index 5afaca7348..b22e4369fe 100644 --- a/src/lib-imap-client/imapc-connection.h +++ b/src/lib-imap-client/imapc-connection.h @@ -33,6 +33,9 @@ void imapc_connection_set_no_reconnect(struct imapc_connection *conn); void imapc_connection_disconnect(struct imapc_connection *conn); void imapc_connection_disconnect_full(struct imapc_connection *conn, bool reconnecting); +void imapc_connection_try_reconnect(struct imapc_connection *conn, + const char *errstr, + unsigned int delay_msecs); void imapc_connection_abort_commands(struct imapc_connection *conn, struct imapc_client_mailbox *only_box, bool keep_retriable) ATTR_NULL(2); diff --git a/src/lib-storage/index/imapc/imapc-mailbox.c b/src/lib-storage/index/imapc/imapc-mailbox.c index 1c76ce702b..2100ab0f54 100644 --- a/src/lib-storage/index/imapc/imapc-mailbox.c +++ b/src/lib-storage/index/imapc/imapc-mailbox.c @@ -17,10 +17,11 @@ void imapc_mailbox_set_corrupted(struct imapc_mailbox *mbox, const char *reason, ...) { + const char *errmsg; va_list va; va_start(va, reason); - i_error("imapc: Mailbox '%s' state corrupted: %s", + errmsg = t_strdup_printf("Mailbox '%s' state corrupted: %s", mbox->box.name, t_strdup_vprintf(reason, va)); va_end(va); @@ -34,7 +35,7 @@ void imapc_mailbox_set_corrupted(struct imapc_mailbox *mbox, /* maybe the remote server is buggy and has become confused. try reconnecting. */ } - imapc_client_mailbox_reconnect(mbox->client_box); + imapc_client_mailbox_reconnect(mbox->client_box, errmsg); } static struct mail_index_view * diff --git a/src/lib-storage/index/imapc/imapc-storage.c b/src/lib-storage/index/imapc/imapc-storage.c index 3ee0e0eda2..11a123bea8 100644 --- a/src/lib-storage/index/imapc/imapc-storage.c +++ b/src/lib-storage/index/imapc/imapc-storage.c @@ -550,10 +550,10 @@ imapc_mailbox_reopen_callback(const struct imapc_command_reply *reply, mbox->storage->reopen_count--; mbox->selecting = FALSE; if (reply->state != IMAPC_COMMAND_STATE_OK) { - mail_storage_set_critical(mbox->box.storage, - "imapc: Reopening mailbox '%s' failed: %s", + const char *errmsg = t_strdup_printf( + "Reopening mailbox '%s' failed: %s", mbox->box.name, reply->text_full); - imapc_client_mailbox_reconnect(mbox->client_box); + imapc_client_mailbox_reconnect(mbox->client_box, errmsg); } imapc_client_stop(mbox->storage->client->client); }