imap-hibernate: Properly fix hibernation

The previous fix did not properly fix hibernation
as the clients still dropped out of hibernate.
Also the tag used was no longer following. This
change will track tag changes and keeps the
hibernation process going on until the user enters
something else than DONE\r\ntag IDLE\r\n in same
packet.

diff --git a/src/imap-hibernate/imap-client.c b/src/imap-hibernate/imap-client.c
index 17146405ba9466a520397e243e96a7d371cbaa07..8964f99abc1737ef4161f6afdb5e07daacff7c9b 100644 (file)
--- a/src/imap-hibernate/imap-client.c
+++ b/src/imap-hibernate/imap-client.c
@@ -28,6 +28,7 @@
 
 /* we only need enough for "DONE\r\n<tag> IDLE\r\n" */
 #define IMAP_MAX_INBUF 12 + 1 + 128 /* DONE\r\nIDLE\r\n + ' ' + <tag> */
+#define IMAP_MAX_OUTBUF 1024
 
 /* If client has sent input and we can't recreate imap process in this
    many seconds, disconnect the client. */
@@ -39,6 +40,8 @@
 /* How often to try to unhibernate clients. */
 #define IMAP_UNHIBERNATE_RETRY_MSECS 10
 
+#define IMAP_CLIENT_BUFFER_FULL_ERROR "Client output buffer is full"
+
 enum imap_client_input_state {
        IMAP_CLIENT_INPUT_STATE_UNKNOWN,
        IMAP_CLIENT_INPUT_STATE_BAD,
@@ -66,11 +69,11 @@ struct imap_client {
        int fd;
        struct io *io;
        struct istream *input;
+       struct ostream *output;
        struct timeout *to_keepalive;
        struct imap_master_connection *master_conn;
        struct ioloop_context *ioloop_ctx;
        const char *log_prefix;
-       unsigned int imap_still_here_text_pos;
        unsigned int next_read_threshold;
        bool bad_done, idle_done;
        bool unhibernate_queued;
@@ -92,7 +95,7 @@ static void imap_client_disconnected(struct imap_client **_client)
        struct imap_client *client = *_client;
        const char *reason;
 
-       reason = io_stream_get_disconnect_reason(client->input, NULL);
+       reason = io_stream_get_disconnect_reason(client->input, client->output);
        imap_client_destroy(_client, reason);
 }
 
@@ -130,6 +133,8 @@ imap_client_move_back_send_callback(void *context, struct ostream *output)
                str_append(str, "\tsession=");
                str_append_tabescaped(str, state->session_id);
        }
+       if (state->tag != NULL)
+               str_printfa(str, "\ttag=%s", client->state.tag);
        if (state->local_ip.family != 0)
                str_printfa(str, "\tlip=%s", net_ip2addr(&state->local_ip));
        if (state->remote_ip.family != 0)
@@ -154,12 +159,7 @@ imap_client_move_back_send_callback(void *context, struct ostream *output)
                str_append(str, "\tclient_input=");
                base64_encode(input_data, input_size, str);
        }
-       if (client->imap_still_here_text_pos != 0) {
-               str_append(str, "\tclient_output=");
-               base64_encode(imap_still_here_text + client->imap_still_here_text_pos,
-                             sizeof(imap_still_here_text)-1 - client->imap_still_here_text_pos,
-                             str);
-       }
+       i_assert(o_stream_get_buffer_used_size(client->output) == 0);
        if (client->idle_done) {
                if (client->bad_done)
                        str_append(str, "\tbad-done");
@@ -201,6 +201,12 @@ static bool imap_client_try_move_back(struct imap_client *client)
        const char *path, *error;
        int ret;
 
+       if (o_stream_get_buffer_used_size(client->output) > 0) {
+               /* there is data buffered, so we have to disconnect you */
+               imap_client_destroy(&client, IMAP_CLIENT_BUFFER_FULL_ERROR);
+               return TRUE;
+       }
+
        master_set = master_service_settings_get(master_service);
        path = t_strconcat(master_set->base_dir,
                           "/"IMAP_MASTER_SOCKET_NAME, NULL);
@@ -246,8 +252,10 @@ static void imap_client_move_back(struct imap_client *client)
 }
 
 static enum imap_client_input_state
-imap_client_input_parse(const unsigned char *data, size_t size)
+imap_client_input_parse(const unsigned char *data, size_t size, const char **tag_r)
 {
+       const unsigned char *tag_start, *tag_end;
+
        enum imap_client_input_state state = IMAP_CLIENT_INPUT_STATE_DONE_LF;
 
        /* skip over DONE[\r]\n */
@@ -267,16 +275,20 @@ imap_client_input_parse(const unsigned char *data, size_t size)
                return IMAP_CLIENT_INPUT_STATE_BAD;
        data++; size--;
 
+       tag_start = data;
+
        /* skip over tag */
-       while(size > 6 &&
-             data[0] != ' ' &&
+       while(data[0] != ' ' &&
              data[0] != '\r' &&
              data[0] != '\t' ) { data++; size--; }
 
+       tag_end = data;
+
        if (size == 0)
                return IMAP_CLIENT_INPUT_STATE_UNKNOWN;
        if (data[0] != ' ')
                return IMAP_CLIENT_INPUT_STATE_BAD;
+       data++; size--;
 
        /* skip over IDLE[\r]\n - checking this assumes that the DONE and IDLE
           are sent in the same IP packet, otherwise we'll unnecessarily
@@ -290,14 +302,21 @@ imap_client_input_parse(const unsigned char *data, size_t size)
        if (data[0] == '\r') {
                data++; size--;
        }
-       return size == 1 && data[0] == '\n' ?
-               IMAP_CLIENT_INPUT_STATE_DONEIDLE : state;
+       if (size == 1 && data[0] == '\n') {
+               *tag_r = t_strdup_until(tag_start, tag_end);
+               return IMAP_CLIENT_INPUT_STATE_DONEIDLE;
+       }
+       return state;
 }
 
 static void imap_client_input_idle_cmd(struct imap_client *client)
 {
+       char *old_tag;
+       const char *new_tag;
+       const char *output;
        const unsigned char *data;
        size_t size;
+       bool done = TRUE;
        int ret;
 
        /* we should read either DONE or disconnection. also handle if client
@@ -310,7 +329,7 @@ static void imap_client_input_idle_cmd(struct imap_client *client)
                return;
        }
        client->next_read_threshold = 0;
-       switch (imap_client_input_parse(data, size)) {
+       switch (imap_client_input_parse(data, size, &new_tag)) {
        case IMAP_CLIENT_INPUT_STATE_UNKNOWN:
                /* we haven't received a full DONE[\r]\n yet - wait */
                client->next_read_threshold = size;
@@ -328,12 +347,34 @@ static void imap_client_input_idle_cmd(struct imap_client *client)
        case IMAP_CLIENT_INPUT_STATE_DONEIDLE:
                /* we received DONE+IDLE, so the client simply wanted to notify
                   us that it's still there. continue hibernation. */
-               i_stream_skip(client->input, size);
-               return;
+               old_tag = client->state.tag;
+               client->state.tag = i_strdup(new_tag);
+               output = t_strdup_printf("%s OK Idle completed.\r\n+ idling\r\n", old_tag);
+               i_free(old_tag);
+               ret = o_stream_flush(client->output);
+               if (ret > 0)
+                       ret = o_stream_send_str(client->output, output);
+               if (ret < 0) {
+                       imap_client_disconnected(&client);
+                       return;
+               }
+               if ((size_t)ret != strlen(output)) {
+                       /* disconnect */
+                       imap_client_destroy(&client, IMAP_CLIENT_BUFFER_FULL_ERROR);
+                       return;
+               } else {
+                       done = FALSE;
+                       i_stream_skip(client->input, size);
+               }
+               break;
        }
-       client->idle_done = TRUE;
-       client->input_pending = TRUE;
-       imap_client_move_back(client);
+
+       if (done) {
+               client->idle_done = TRUE;
+               client->input_pending = TRUE;
+               imap_client_move_back(client);
+       } else
+               imap_client_add_idle_keepalive_timeout(client);
 }
 
 static void imap_client_input_nonidle(struct imap_client *client)
@@ -353,31 +394,22 @@ static void imap_client_input_notify(struct imap_client *client)
 
 static void keepalive_timeout(struct imap_client *client)
 {
-       unsigned int text_left = sizeof(imap_still_here_text)-1 -
-               client->imap_still_here_text_pos;
        ssize_t ret;
 
-       ret = write(client->fd,
-                   imap_still_here_text + client->imap_still_here_text_pos,
-                   text_left);
+       /* do not send this if there is data buffered */
+       if ((ret = o_stream_flush(client->output)) < 0) {
+               imap_client_disconnected(&client);
+               return;
+       } else if (ret == 0)
+               return;
+
+       ret = o_stream_send_str(client->output, imap_still_here_text);
        if (ret < 0) {
-               /* disconnect */
-               const char *reason = errno == EPIPE ? "Connection closed" :
-                       t_strdup_printf("Connection closed: %m");
-               imap_client_destroy(&client, reason);
+               imap_client_disconnected(&client);
                return;
        }
-       client->imap_still_here_text_pos += ret;
-       if (client->imap_still_here_text_pos >= sizeof(imap_still_here_text)-1)
-               client->imap_still_here_text_pos = 0;
-       else {
-               /* we didn't write the entire line, but that's ok. we could
-                  io_add(IO_WRITE) here, but that's probably not necessary.
-                  just continue writing the "still here" line after the next
-                  timeout. its main purpose is to see if the connection is
-                  still alive, and we're successfully doing that already.
-                  besides, it's highly unlikely we'll ever even get here.. */
-       }
+       /* ostream buffer size is definitely large enough for this text */
+       i_assert((size_t)ret == strlen(imap_still_here_text));
        imap_client_add_idle_keepalive_timeout(client);
 }
 
@@ -488,7 +520,7 @@ imap_client_create(int fd, const struct imap_client_state *state)
        client->pool = pool;
        client->fd = fd;
        client->input = i_stream_create_fd(fd, IMAP_MAX_INBUF, FALSE);
-
+       client->output = o_stream_create_fd(fd, IMAP_MAX_OUTBUF);
        client->state = *state;
        client->state.username = p_strdup(pool, state->username);
        client->state.session_id = p_strdup(pool, state->session_id);
@@ -567,9 +599,13 @@ void imap_client_destroy(struct imap_client **_client, const char *reason)
                io_loop_context_unref(&client->ioloop_ctx);
        }
 
+       if (client->state.tag != NULL)
+               i_free(client->state.tag);
+
        DLLIST_REMOVE(&imap_clients, client);
        imap_client_stop(client);
        i_stream_destroy(&client->input);
+       o_stream_destroy(&client->output);
        i_close_fd(&client->fd);
        pool_unref(&client->pool);
 

diff --git a/src/imap-hibernate/imap-client.h b/src/imap-hibernate/imap-client.h
index dfee3de0135bf8a1fe9fbf4968fbdb18978ab93d..a68cf9666f924bfe5fa87db3b8054a409d82b6d2 100644 (file)
--- a/src/imap-hibernate/imap-client.h
+++ b/src/imap-hibernate/imap-client.h
@@ -16,6 +16,7 @@ struct imap_client_state {
        dev_t peer_dev;
        ino_t peer_ino;
 
+       char *tag;
        const unsigned char *state;
        size_t state_size;
 

diff --git a/src/imap-hibernate/imap-hibernate-client.c b/src/imap-hibernate/imap-hibernate-client.c
index 96178d20b03b92f3ff571864fd8d3c9ccf1ccc3d..4b4ed7485e671bfba1e5b9aea9f1274eb03f2b95 100644 (file)
--- a/src/imap-hibernate/imap-hibernate-client.c
+++ b/src/imap-hibernate/imap-hibernate-client.c
@@ -127,6 +127,8 @@ imap_hibernate_client_parse_input(const char *const *args, pool_t pool,
                                        "Invalid idle_notify_interval value: %s", value);
                                return -1;
                        }
+               } else if (strcmp(key, "tag") == 0) {
+                       state_r->tag = i_strdup(value);
                } else if (strcmp(key, "state") == 0) {
                        buffer_t *state_buf;
 
@@ -141,6 +143,10 @@ imap_hibernate_client_parse_input(const char *const *args, pool_t pool,
                        state_r->state_size = state_buf->used;
                }
        }
+       if (state_r->tag == NULL) {
+               *error_r = "Missing tag";
+               return -1;
+       }
        if (peer_dev_major != 0 || peer_dev_minor != 0)
                state_r->peer_dev = makedev(peer_dev_major, peer_dev_minor);
        return 0;

diff --git a/src/imap/imap-client-hibernate.c b/src/imap/imap-client-hibernate.c
index 0c251039dddb08f527d814003004ef2ca2be2617..ae518ab044a9ec8253ae23f0876e02f1067984d3 100644 (file)
--- a/src/imap/imap-client-hibernate.c
+++ b/src/imap/imap-client-hibernate.c
@@ -44,6 +44,9 @@ static void imap_hibernate_write_cmd(struct client *client, string_t *cmd,
                                     const buffer_t *state, int fd_notify)
 {
        struct stat peer_st;
+       const char *tag;
+
+       tag = client->command_queue == NULL ? NULL : client->command_queue->tag;
 
        str_append_tabescaped(cmd, client->user->username);
        str_append_c(cmd, '\t');
@@ -81,6 +84,8 @@ static void imap_hibernate_write_cmd(struct client *client, string_t *cmd,
                str_printfa(cmd, "\tuid=%s", dec2str(client->user->uid));
        if (client->user->gid != (gid_t)-1)
                str_printfa(cmd, "\tgid=%s", dec2str(client->user->gid));
+       if (tag != NULL)
+               str_printfa(cmd, "\ttag=%s", tag);
        str_append(cmd, "\tstats=");
        str_append_tabescaped(cmd, client_stats(client));
        if (client->command_queue != NULL &&

diff --git a/src/imap/imap-master-client.c b/src/imap/imap-master-client.c
index 8e5732558c26c8bb52379385e6e4e0d7516d89b7..35cf2bb3d338bebc36838c00a63e4b4e5a26a4b4 100644 (file)
--- a/src/imap/imap-master-client.c
+++ b/src/imap/imap-master-client.c
@@ -26,6 +26,8 @@ struct imap_master_input {
        buffer_t *client_output;
        /* IMAP connection state */
        buffer_t *state;
+       /* command tag */
+       const char *tag;
 
        dev_t peer_dev;
        ino_t peer_ino;
@@ -136,6 +138,8 @@ imap_master_client_parse_input(const char *const *args, pool_t pool,
                                        "Invalid state base64 value: %s", value);
                                return -1;
                        }
+               } else if (strcmp(key, "tag") == 0) {
+                       master_input_r->tag = t_strdup(value);
                } else if (strcmp(key, "bad-done") == 0) {
                        master_input_r->state_import_bad_idle_done = TRUE;
                } else if (strcmp(key, "idle-continue") == 0) {
@@ -247,6 +251,9 @@ imap_master_client_input_args(struct connection *conn, const char *const *args,
                return -1;
        }
 
+       if (master_input.tag != NULL)
+               imap_state_import_idle_cmd_tag(imap_client, master_input.tag);
+
        /* make sure all pending input gets handled */
        i_assert(imap_client->to_delayed_input == NULL);
        if (master_input.client_input->used > 0) {

diff --git a/src/imap/imap-state.c b/src/imap/imap-state.c
index 3130c78fa824ea9f204360ac285eaa48b223eacc..73332eafa90c296a29e010749d75d9aaba39c086 100644 (file)
--- a/src/imap/imap-state.c
+++ b/src/imap/imap-state.c
@@ -25,7 +25,6 @@ enum imap_state_type_public {
 enum imap_state_type_internal {
        IMAP_STATE_TYPE_ID_LOGGED               = 'I',
        IMAP_STATE_TYPE_TLS_COMPRESSION         = 'C',
-       IMAP_STATE_TYPE_IDLE_CMD_TAG            = 'T'
 };
 
 enum imap_state_feature {
@@ -298,12 +297,6 @@ int imap_state_export_base(struct client *client, bool internal,
                        buffer_append_c(dest, IMAP_STATE_TYPE_ID_LOGGED);
                if (client->tls_compression)
                        buffer_append_c(dest, IMAP_STATE_TYPE_TLS_COMPRESSION);
-               if (client->command_queue != NULL) {
-                       i_assert(strcasecmp(client->command_queue->name, "IDLE") == 0);
-                       buffer_append_c(dest, IMAP_STATE_TYPE_IDLE_CMD_TAG);
-                       buffer_append(dest, client->command_queue->tag,
-                                     strlen(client->command_queue->tag) + 1);
-               }
        }
 
        /* IMAP SEARCHRES extension */
@@ -748,18 +741,8 @@ import_state_tls_compression(struct client *client,
        return 0;
 }
 
-static ssize_t
-import_state_idle_cmd_tag(struct client *client, const unsigned char *data,
-                         size_t size, const char **error_r)
+void imap_state_import_idle_cmd_tag(struct client *client, const char *tag)
 {
-       const unsigned char *p = data, *end = data + size;
-       const char *tag;
-
-       if (import_string(&p, end, &tag) < 0) {
-               *error_r = "Missing idle tag";
-               return 0;
-       }
-
        if (client->state_import_idle_continue) {
                /* IDLE command continues */
                struct client_command_context *cmd;
@@ -788,7 +771,6 @@ import_state_idle_cmd_tag(struct client *client, const unsigned char *data,
                        "%s %s Idle completed.", tag,
                        client->state_import_bad_idle_done ? "BAD" : "OK"));
        }
-       return p - data;
 }
 
 static struct {
@@ -807,8 +789,7 @@ static struct {
                          size_t size, const char **error_r);
 } imap_states_internal[] = {
        { IMAP_STATE_TYPE_ID_LOGGED, import_state_id_logged },
-       { IMAP_STATE_TYPE_TLS_COMPRESSION, import_state_tls_compression },
-       { IMAP_STATE_TYPE_IDLE_CMD_TAG, import_state_idle_cmd_tag },
+       { IMAP_STATE_TYPE_TLS_COMPRESSION, import_state_tls_compression }
 };
 
 static ssize_t

diff --git a/src/imap/imap-state.h b/src/imap/imap-state.h
index 7e91ae17e8d936148cf514d199e1f9a3d506247d..ee0037607e81f43eec853135ea2004336a26877b 100644 (file)
--- a/src/imap/imap-state.h
+++ b/src/imap/imap-state.h
@@ -26,5 +26,5 @@ int imap_state_export_base(struct client *client, bool internal,
 ssize_t imap_state_import_base(struct client *client, bool internal,
                               const unsigned char *data, size_t size,
                               const char **error_r);
-
+void imap_state_import_idle_cmd_tag(struct client *client, const char *tag);
 #endif