From: Timo Sirainen Date: Thu, 9 Feb 2023 15:04:22 +0000 (+0200) Subject: lib-imap-client: Fix/clarify selection state handling X-Git-Tag: 2.3.21~102 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=5b6f678009ab0ae3b6f47ccc8e408395485004ad;p=thirdparty%2Fdovecot%2Fcore.git lib-imap-client: Fix/clarify selection state handling The old code assumed that selected_box would be non-NULL while a new mailbox is being selected. That's not true though, the imapc-storage code closes the old mailbox before selecting the next one. So the QRESYNC-specific code for tracking selected-state was never being used. Fixed this, and clarified in general how the selected-state is supposed to work. --- diff --git a/src/lib-imap-client/imapc-client.c b/src/lib-imap-client/imapc-client.c index adb4dcb044..5806a80c50 100644 --- a/src/lib-imap-client/imapc-client.c +++ b/src/lib-imap-client/imapc-client.c @@ -416,7 +416,7 @@ void imapc_client_mailbox_close(struct imapc_client_mailbox **_box) box->closing = TRUE; /* cancel any pending commands */ - imapc_connection_unselect(box); + imapc_connection_unselect(box, FALSE); if (box->reconnecting) { /* need to abort the reconnection so it won't try to access diff --git a/src/lib-imap-client/imapc-connection.c b/src/lib-imap-client/imapc-connection.c index 1c3d67de5e..f025403481 100644 --- a/src/lib-imap-client/imapc-connection.c +++ b/src/lib-imap-client/imapc-connection.c @@ -102,7 +102,20 @@ struct imapc_connection { struct timeval last_connect; unsigned int reconnect_count; - struct imapc_client_mailbox *qresync_selecting_box, *selected_box; + /* If QRESYNC isn't used, this is set immediately after issuing + SELECT/EXAMINE. We could differentiate better whether a mailbox is + "being selected" vs "fully selected", but that code is already in + the imapc-storage side so it would have to be moved or duplicated + here. And since nothing actually cares about this distinction (yet), + don't bother with it for now. This is set to NULL when the mailbox + is closed from imapc-storage point of view, even if the server is + still in selected state (see selected_on_server). */ + struct imapc_client_mailbox *selected_box; + /* If QRESYNC is used, this is set when SELECT/EXAMINE is issued. + If the server is already in selected state, the selected_box is most + likely already NULL at this point, because imapc-storage has closed + it. */ + struct imapc_client_mailbox *qresync_selecting_box; enum imapc_connection_state state; char *disconnect_reason; @@ -141,6 +154,10 @@ struct imapc_connection { bool idle_stopping:1; bool idle_plus_waiting:1; bool select_waiting_reply:1; + /* TRUE if IMAP server is in SELECTED state. select_box may be NULL + though, if we already closed the mailbox from client point of + view. */ + bool selected_on_server:1; }; static void imapc_connection_capability_cb(const struct imapc_command_reply *reply, @@ -404,6 +421,7 @@ static void imapc_connection_set_state(struct imapc_connection *conn, conn->select_waiting_reply = FALSE; conn->qresync_selecting_box = NULL; conn->selected_box = NULL; + conn->selected_on_server = FALSE; /* fall through */ case IMAPC_CONNECTION_STATE_DONE: /* if we came from imapc_client_get_capabilities(), stop so @@ -785,6 +803,8 @@ imapc_connection_handle_resp_text_code(struct imapc_connection *conn, if (conn->qresync_selecting_box != NULL) { conn->selected_box = conn->qresync_selecting_box; conn->qresync_selecting_box = NULL; + } else { + conn->selected_on_server = FALSE; } } return 0; @@ -1501,7 +1521,7 @@ static int imapc_connection_input_tagged(struct imapc_connection *conn) (cmd->flags & IMAPC_COMMAND_FLAG_SELECT) != 0 && conn->selected_box != NULL) { /* EXAMINE/SELECT failed: mailbox is no longer selected */ - imapc_connection_unselect(conn->selected_box); + imapc_connection_unselect(conn->selected_box, TRUE); } if (conn->reconnect_command_count > 0 && @@ -2111,7 +2131,7 @@ static void imapc_connection_set_selecting(struct imapc_client_mailbox *box) i_assert(conn->qresync_selecting_box == NULL); - if (conn->selected_box != NULL && + if (conn->selected_on_server && (conn->capabilities & IMAPC_CAPABILITY_QRESYNC) != 0) { /* server will send a [CLOSED] once selected mailbox is closed */ @@ -2120,6 +2140,7 @@ static void imapc_connection_set_selecting(struct imapc_client_mailbox *box) /* we'll have to assume that all the future untagged messages are for the mailbox we're selecting */ conn->selected_box = box; + conn->selected_on_server = TRUE; } conn->select_waiting_reply = TRUE; } @@ -2466,17 +2487,37 @@ imapc_connection_get_capabilities(struct imapc_connection *conn) return conn->capabilities; } -void imapc_connection_unselect(struct imapc_client_mailbox *box) +void imapc_connection_unselect(struct imapc_client_mailbox *box, + bool via_tagged_reply) { struct imapc_connection *conn = box->conn; - if (conn->selected_box != NULL || conn->qresync_selecting_box != NULL) { - i_assert(conn->selected_box == box || - conn->qresync_selecting_box == box); - - conn->selected_box = NULL; + if (conn->select_waiting_reply) { + /* Mailbox closing was requested before SELECT/EXAMINE + replied. The connection state is now unknown and + shouldn't be used anymore. */ + imapc_connection_disconnect(conn); + } else if (conn->qresync_selecting_box == NULL && + conn->selected_box == NULL) { + /* There is no mailbox selected currently. */ + i_assert(!via_tagged_reply); + } else { + /* Mailbox was closed in a known state. Either due to + SELECT/EXAMINE failing (via_tagged_reply) or by + imapc-storage after the mailbox was already fully + selected. */ + i_assert(conn->qresync_selecting_box == box || + conn->selected_box == box); conn->qresync_selecting_box = NULL; + conn->selected_box = NULL; + if (via_tagged_reply) + conn->selected_on_server = FALSE; + else { + /* We didn't actually send UNSELECT command, so don't + touch selected_on_server state. */ + } } + imapc_connection_send_idle_done(conn); imapc_connection_abort_commands(conn, box, FALSE); } diff --git a/src/lib-imap-client/imapc-connection.h b/src/lib-imap-client/imapc-connection.h index 8fda69260c..20c249e6f8 100644 --- a/src/lib-imap-client/imapc-connection.h +++ b/src/lib-imap-client/imapc-connection.h @@ -48,7 +48,8 @@ imapc_connection_cmd(struct imapc_connection *conn, imapc_command_callback_t *callback, void *context) ATTR_NULL(3); -void imapc_connection_unselect(struct imapc_client_mailbox *box); +void imapc_connection_unselect(struct imapc_client_mailbox *box, + bool via_tagged_reply); enum imapc_connection_state imapc_connection_get_state(struct imapc_connection *conn);