From: Aki Tuomi Date: Thu, 6 Oct 2016 11:54:24 +0000 (+0300) Subject: imap-hibernate: Properly fix hibernation X-Git-Tag: 2.2.26~166 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=3b5562aeb6796e33bd8c17ba59f364b38c71d366;p=thirdparty%2Fdovecot%2Fcore.git 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 17146405ba..8964f99abc 100644 --- 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 IDLE\r\n" */ #define IMAP_MAX_INBUF 12 + 1 + 128 /* DONE\r\nIDLE\r\n + ' ' + */ +#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 dfee3de013..a68cf9666f 100644 --- 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 96178d20b0..4b4ed7485e 100644 --- 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 0c251039dd..ae518ab044 100644 --- 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 8e5732558c..35cf2bb3d3 100644 --- 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 3130c78fa8..73332eafa9 100644 --- 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 7e91ae17e8..ee0037607e 100644 --- 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