diff --git a/nets/net_imap.c b/nets/net_imap.c index d50c129..0f40b41 100644 --- a/nets/net_imap.c +++ b/nets/net_imap.c @@ -4207,7 +4207,30 @@ static int handle_idle(struct imap_session *imap) if (imap->client) { imap_client_integrity_check(imap, imap->client); /* Stop idling on the selected mailbox (remote) */ - imap_client_idle_stop(imap->client); + if (imap_client_idle_stop(imap->client)) { + /* If we failed to terminate IDLE, then the session is already dead, + * and would be cleaned up in imap_clients_renew_idle as we prune dead sessions, + * unless it's the foreground client, in which case it's not pruned + * and we're left with a dead client, so handle a dead foreground client here. + * + * This is unfortunately a somewhat common occurence, because + * a lot of IMAP servers don't seem to send us untagged data + * while idling until later we try to do something with them. + * + * This is especially true for servers that enforce OAuth2 authentication, + * such as Microsoft's, which will disconnect clients after 1 hour, + * no matter what, with this untagged message: + * + * * BYE Session invalidated - AccessTokenExpired + * + * There's no real good way to deal with this, since being disconnected is inevitable, + * other than immediately recreating the session when this happens. + * + * This here is the case where our local user did something, + * the case of getting activity on a particular client, + * and handling its disconnect in realtime, is handled below. */ + imap_recreate_client(imap, imap->client); + } } imap_clients_renew_idle(imap); /* In case some of them are close to expiring, renew them now before returning */ break; /* Client terminated the idle. Stop idling and return to read the next command. */ @@ -4220,7 +4243,7 @@ static int handle_idle(struct imap_session *imap) /* The remote peer probably closed the connection, and this client is now dead. * We need to remove this now or poll will keep triggering for this client. */ client->dead = 1; - imap_client_unlink(imap, client); + imap_recreate_client(imap, client); continue; } if (!client->idling) { diff --git a/nets/net_imap/imap_client.c b/nets/net_imap/imap_client.c index aa324e9..ef095d5 100644 --- a/nets/net_imap/imap_client.c +++ b/nets/net_imap/imap_client.c @@ -266,14 +266,67 @@ int imap_clients_next_idle_expiration(struct imap_session *imap) return (int) min_maxage; /* This is number of seconds until, it will fit in an int */ } +/*! + * \brief Create a client after it has just been purged + * \note The clients list must NOT be locked when calling this + */ +static struct imap_client *create_client_after_purge(struct imap_session *imap, const char *name, int foreground, int idling) +{ + struct imap_client *client; + int exists; + + client = load_virtual_mailbox(imap, name, &exists); + if (!client) { /* Will lock the list to insert it */ + /* Generally, it should exist, because we're creating a client that USED to exist! + * However, it is possible that .imapremote has been modified since the + * last time we parsed it (when the client was originally created), + * in which case, this will fail. In that case, just let it go. */ + if (exists) { + bbs_warning("Failed to recreate %s client %s\n", foreground ? "foreground" : "background", name); + } + return NULL; + } + bbs_verb(4, "Recreated %s IMAP client '%s'\n", foreground ? "foreground" : "background", name); + if (idling) { + imap_client_idle_start(client); /* Resume idling, if that's what it was doing before */ + } + return client; +} + +int imap_recreate_client(struct imap_session *imap, struct imap_client *client) +{ + struct imap_client *newclient; + int was_idling = client->idling; + int foreground = imap->client == client; + char name[256]; + + safe_strncpy(name, client->name, sizeof(name)); /* Store name before destroying the client */ + + /* Destroy old client before creating it again. */ + if (foreground) { + imap->client = NULL; + } + imap_client_unlink(imap, client); /* returns void, so can't check success */ + + /* Now, create it again. */ + newclient = create_client_after_purge(imap, name, foreground, was_idling); + if (newclient && foreground) { + imap->client = newclient; /* At this point, the new client has been swapped in, and the rest of the module is none the wiser. */ + } + return newclient ? 0 : -1; +} + void imap_clients_renew_idle(struct imap_session *imap) { + struct stringlist deadclients; + char *clientname; struct imap_client *client; time_t now = time(NULL); + stringlist_init(&deadclients); + /* Renew all the IDLEs on remote servers, periodically, * to keep the IMAP connection alive. */ - RWLIST_WRLOCK(&imap->clients); RWLIST_TRAVERSE_SAFE_BEGIN(&imap->clients, client, entry) { time_t maxage; @@ -286,11 +339,17 @@ void imap_clients_renew_idle(struct imap_session *imap) /* If we're not going to call this function to check for renewals before it's due to expire, renew it now */ if (maxage < now + 15) { /* Add a little bit of wiggle room */ time_t age = now - client->idlestarted; - bbs_debug(4, "Client '%s' needs to renew IDLE (%" TIME_T_FMT "/%d s elapsed)...\n", client->virtprefix, age, client->maxidlesec); + bbs_debug(4, "%s client '%s' needs to renew IDLE (%" TIME_T_FMT "/%d s elapsed)...\n", + imap->client == client ? "Foreground" : "Background", client->virtprefix, age, client->maxidlesec); if (imap_client_idle_stop(client) || imap_client_idle_start(client)) { client->dead = 1; if (imap->client != client) { RWLIST_REMOVE_CURRENT(entry); + /* The list is locked at the moment, + * and creating a client entails acquiring that lock, + * so for now, just keep track of what clients we removed, + * and recreate them afterwards. */ + stringlist_push_tail(&deadclients, client->name); client_destroy(client); } continue; @@ -299,6 +358,14 @@ void imap_clients_renew_idle(struct imap_session *imap) } RWLIST_TRAVERSE_SAFE_END; RWLIST_UNLOCK(&imap->clients); + + /* Now that we've pruned the client list of all dead clients, + * attempt to recreate those we just purged. */ + while ((clientname = stringlist_pop(&deadclients))) { + create_client_after_purge(imap, clientname, 0, 1); + free(clientname); + } + stringlist_destroy(&deadclients); } void imap_client_idle_notify(struct imap_client *client) diff --git a/nets/net_imap/imap_client.h b/nets/net_imap/imap_client.h index 0072ade..5e87a40 100644 --- a/nets/net_imap/imap_client.h +++ b/nets/net_imap/imap_client.h @@ -59,6 +59,12 @@ int imap_client_idle_stop(struct imap_client *client); */ int imap_clients_next_idle_expiration(struct imap_session *imap); +/*! + * \brief Recreate a client if it is dead + * \note The clients list must NOT be locked when calling this + */ +int imap_recreate_client(struct imap_session *imap, struct imap_client *client); + /*! \brief Renew IDLE on all idling clients that are close to expiring */ void imap_clients_renew_idle(struct imap_session *imap);