dovecot-2.2: imapc: Reorganize code so that imapc_list works wit...

dovecot at dovecot.org dovecot at dovecot.org
Wed Jul 10 06:55:08 EEST 2013


details:   http://hg.dovecot.org/dovecot-2.2/rev/620876853f6f
changeset: 16586:620876853f6f
user:      Timo Sirainen <tss at iki.fi>
date:      Wed Jul 10 06:54:57 2013 +0300
description:
imapc: Reorganize code so that imapc_list works without imapc_storage.
Most importantly fixes crashes when imapc_list is trying to lookup hierarchy
separator before storage is created.

diffstat:

 src/lib-storage/index/imapc/imapc-list.c       |  150 ++++++++++++--
 src/lib-storage/index/imapc/imapc-list.h       |    7 +-
 src/lib-storage/index/imapc/imapc-mail-fetch.c |    4 +-
 src/lib-storage/index/imapc/imapc-save.c       |   13 +-
 src/lib-storage/index/imapc/imapc-storage.c    |  241 +++++++++---------------
 src/lib-storage/index/imapc/imapc-storage.h    |   43 +++-
 src/lib-storage/index/imapc/imapc-sync.c       |    6 +-
 7 files changed, 260 insertions(+), 204 deletions(-)

diffs (truncated from 931 to 300 lines):

diff -r e45ad5524f1a -r 620876853f6f src/lib-storage/index/imapc/imapc-list.c
--- a/src/lib-storage/index/imapc/imapc-list.c	Wed Jul 10 06:05:20 2013 +0300
+++ b/src/lib-storage/index/imapc/imapc-list.c	Wed Jul 10 06:54:57 2013 +0300
@@ -41,6 +41,12 @@
 
 extern struct mailbox_list imapc_mailbox_list;
 
+static void imapc_list_send_hierarcy_sep_lookup(struct imapc_mailbox_list *list);
+static void imapc_untagged_list(const struct imapc_untagged_reply *reply,
+				struct imapc_storage_client *client);
+static void imapc_untagged_lsub(const struct imapc_untagged_reply *reply,
+				struct imapc_storage_client *client);
+
 static struct mailbox_list *imapc_list_alloc(void)
 {
 	struct imapc_mailbox_list *list;
@@ -50,20 +56,41 @@
 	list = p_new(pool, struct imapc_mailbox_list, 1);
 	list->list = imapc_mailbox_list;
 	list->list.pool = pool;
-	/* separator is set when storage is created */
+	/* separator is set lazily */
 	list->mailboxes = mailbox_tree_init('\0');
 	mailbox_tree_set_parents_nonexistent(list->mailboxes);
 	return &list->list;
 }
 
-static int
-imapc_list_init(struct mailbox_list *_list, const char **error_r ATTR_UNUSED)
+static int imapc_list_init(struct mailbox_list *_list, const char **error_r)
 {
 	struct imapc_mailbox_list *list = (struct imapc_mailbox_list *)_list;
+	char sep;
 
 	list->set = mail_user_set_get_driver_settings(_list->ns->user->set_info,
 						      _list->ns->user->set,
 						      IMAPC_STORAGE_NAME);
+	if (imapc_storage_client_create(_list->ns, list->set, _list->mail_set,
+					&list->client, error_r) < 0)
+		return -1;
+	list->client->_list = list;
+
+	imapc_storage_client_register_untagged(list->client, "LIST",
+					       imapc_untagged_list);
+	imapc_storage_client_register_untagged(list->client, "LSUB",
+					       imapc_untagged_lsub);
+	imapc_list_send_hierarcy_sep_lookup(list);
+	if ((_list->ns->flags & NAMESPACE_FLAG_INBOX_USER) != 0) {
+		/* we're using imapc for the INBOX namespace. wait and make
+		   sure we can successfully access the IMAP server (so if the
+		   username is invalid we don't just keep failing every
+		   command). */
+		if (imapc_list_try_get_root_sep(list, &sep) < 0) {
+			imapc_storage_client_unref(&list->client);
+			*error_r = "Failed to access imapc backend";
+			return -1;
+		}
+	}
 	return 0;
 }
 
@@ -76,21 +103,43 @@
 	mailbox_tree_deinit(&list->mailboxes);
 	if (list->tmp_subscriptions != NULL)
 		mailbox_tree_deinit(&list->tmp_subscriptions);
+	imapc_storage_client_unref(&list->client);
 	pool_unref(&list->list.pool);
 }
 
+static void
+imapc_list_copy_error_from_reply(struct imapc_mailbox_list *list,
+				 enum mail_error default_error,
+				 const struct imapc_command_reply *reply)
+{
+	enum mail_error error;
+
+	if (imap_resp_text_code_parse(reply->resp_text_key, &error)) {
+		mailbox_list_set_error(&list->list, error,
+				       reply->text_without_resp);
+	} else {
+		mailbox_list_set_error(&list->list, default_error,
+				       reply->text_without_resp);
+	}
+}
+
 static void imapc_list_simple_callback(const struct imapc_command_reply *reply,
 				       void *context)
 {
 	struct imapc_simple_context *ctx = context;
-	const char *str;
-	enum mail_error error;
 
-	imapc_simple_callback(reply, context);
-	if (ctx->ret < 0) {
-		str = mail_storage_get_last_error(&ctx->storage->storage, &error);
-		mailbox_list_set_error(&ctx->storage->list->list, error, str);
+	if (reply->state == IMAPC_COMMAND_STATE_OK)
+		ctx->ret = 0;
+	else if (reply->state == IMAPC_COMMAND_STATE_NO) {
+		imapc_list_copy_error_from_reply(ctx->client->_list,
+						 MAIL_ERROR_PARAMS, reply);
+		ctx->ret = -1;
+	} else {
+		mailbox_list_set_critical(&ctx->client->_list->list,
+			"imapc: Command failed: %s", reply->text_full);
+		ctx->ret = -1;
 	}
+	imapc_client_stop(ctx->client->client);
 }
 
 static bool
@@ -144,13 +193,13 @@
 }
 
 static void imapc_untagged_list(const struct imapc_untagged_reply *reply,
-				struct imapc_storage *storage)
+				struct imapc_storage_client *client)
 {
-	struct imapc_mailbox_list *list = storage->list;
+	struct imapc_mailbox_list *list = client->_list;
 	const struct imap_arg *args = reply->args;
 	const char *sep, *name;
 
-	if (storage->root_sep == '\0') {
+	if (list->root_sep == '\0') {
 		/* we haven't asked for the separator yet.
 		   lets see if this is the reply for its request. */
 		if (args[0].type == IMAP_ARG_EOL ||
@@ -159,21 +208,21 @@
 			return;
 
 		/* we can't handle NIL separator yet */
-		storage->root_sep = sep == NULL ? '/' : sep[0];
-		mailbox_tree_set_separator(list->mailboxes, storage->root_sep);
+		list->root_sep = sep == NULL ? '/' : sep[0];
+		mailbox_tree_set_separator(list->mailboxes, list->root_sep);
 	} else {
 		(void)imapc_list_update_tree(list, list->mailboxes, args);
 	}
 }
 
 static void imapc_untagged_lsub(const struct imapc_untagged_reply *reply,
-				struct imapc_storage *storage)
+				struct imapc_storage_client *client)
 {
-	struct imapc_mailbox_list *list = storage->list;
+	struct imapc_mailbox_list *list = client->_list;
 	const struct imap_arg *args = reply->args;
 	struct mailbox_node *node;
 
-	if (storage->root_sep == '\0') {
+	if (list->root_sep == '\0') {
 		/* we haven't asked for the separator yet */
 		return;
 	}
@@ -191,12 +240,61 @@
 	}
 }
 
-void imapc_list_register_callbacks(struct imapc_mailbox_list *list)
+static void imapc_list_sep_verify(struct imapc_mailbox_list *list)
 {
-	imapc_storage_register_untagged(list->storage, "LIST",
-					imapc_untagged_list);
-	imapc_storage_register_untagged(list->storage, "LSUB",
-					imapc_untagged_lsub);
+	const char *imapc_list_prefix = list->set->imapc_list_prefix;
+
+	if (list->root_sep == '\0') {
+		mailbox_list_set_critical(&list->list,
+			"imapc: LIST didn't return hierarchy separator");
+	} else if (imapc_list_prefix[0] != '\0' &&
+		   imapc_list_prefix[strlen(imapc_list_prefix)-1] == list->root_sep) {
+		mailbox_list_set_critical(&list->list,
+			"imapc_list_prefix must not end with hierarchy separator");
+	}
+}
+
+static void imapc_storage_sep_callback(const struct imapc_command_reply *reply,
+				       void *context)
+{
+	struct imapc_mailbox_list *list = context;
+
+	list->root_sep_pending = FALSE;
+	if (reply->state == IMAPC_COMMAND_STATE_OK)
+		imapc_list_sep_verify(list);
+	else if (reply->state == IMAPC_COMMAND_STATE_NO)
+		imapc_list_copy_error_from_reply(list, MAIL_ERROR_PARAMS, reply);
+	else {
+		mailbox_list_set_critical(&list->list,
+			"imapc: Command failed: %s", reply->text_full);
+	}
+	imapc_client_stop(list->client->client);
+}
+
+static void imapc_list_send_hierarcy_sep_lookup(struct imapc_mailbox_list *list)
+{
+	struct imapc_command *cmd;
+
+	if (list->root_sep_pending)
+		return;
+	list->root_sep_pending = TRUE;
+
+	cmd = imapc_client_cmd(list->client->client,
+			       imapc_storage_sep_callback, list);
+	imapc_command_send(cmd, "LIST \"\" \"\"");
+}
+
+int imapc_list_try_get_root_sep(struct imapc_mailbox_list *list, char *sep_r)
+{
+	if (list->root_sep == '\0') {
+		imapc_list_send_hierarcy_sep_lookup(list);
+		while (list->root_sep_pending)
+			imapc_client_run(list->client->client);
+		if (list->root_sep == '\0')
+			return -1;
+	}
+	*sep_r = list->root_sep;
+	return 0;
 }
 
 static char imapc_list_get_hierarchy_sep(struct mailbox_list *_list)
@@ -204,7 +302,7 @@
 	struct imapc_mailbox_list *list = (struct imapc_mailbox_list *)_list;
 	char sep;
 
-	if (imapc_storage_try_get_root_sep(list->storage, &sep) < 0) {
+	if (imapc_list_try_get_root_sep(list, &sep) < 0) {
 		/* we can't really fail here. just return a common separator
 		   and keep failing all list commands until it succeeds. */
 		return '/';
@@ -367,8 +465,8 @@
 imapc_list_simple_context_init(struct imapc_simple_context *ctx,
 			       struct imapc_mailbox_list *list)
 {
-	imapc_simple_context_init(ctx, list->storage);
-	return imapc_client_cmd(list->storage->client,
+	imapc_simple_context_init(ctx, list->client);
+	return imapc_client_cmd(list->client->client,
 				imapc_list_simple_callback, ctx);
 }
 
@@ -690,7 +788,7 @@
 	struct imapc_command *cmd;
 	struct imapc_simple_context ctx;
 
-	capa = imapc_client_get_capabilities(list->storage->client);
+	capa = imapc_client_get_capabilities(list->client->client);
 
 	cmd = imapc_list_simple_context_init(&ctx, list);
 	imapc_command_set_flags(cmd, IMAPC_COMMAND_FLAG_SELECT);
diff -r e45ad5524f1a -r 620876853f6f src/lib-storage/index/imapc/imapc-list.h
--- a/src/lib-storage/index/imapc/imapc-list.h	Wed Jul 10 06:05:20 2013 +0300
+++ b/src/lib-storage/index/imapc/imapc-list.h	Wed Jul 10 06:54:57 2013 +0300
@@ -10,21 +10,22 @@
 struct imapc_mailbox_list {
 	struct mailbox_list list;
 	const struct imapc_settings *set;
-	struct imapc_storage *storage;
+	struct imapc_storage_client *client;
 	struct mailbox_list *index_list;
 
 	struct mailbox_tree_context *mailboxes, *tmp_subscriptions;
+	char root_sep;
 
 	unsigned int iter_count;
 
 	unsigned int refreshed_subscriptions:1;
 	unsigned int refreshed_mailboxes:1;
 	unsigned int index_list_failed:1;
+	unsigned int root_sep_pending:1;
 };
 
 int imapc_list_get_mailbox_flags(struct mailbox_list *list, const char *name,
 				 enum mailbox_info_flags *flags_r);
-
-void imapc_list_register_callbacks(struct imapc_mailbox_list *list);
+int imapc_list_try_get_root_sep(struct imapc_mailbox_list *list, char *sep_r);
 
 #endif
diff -r e45ad5524f1a -r 620876853f6f src/lib-storage/index/imapc/imapc-mail-fetch.c
--- a/src/lib-storage/index/imapc/imapc-mail-fetch.c	Wed Jul 10 06:05:20 2013 +0300
+++ b/src/lib-storage/index/imapc/imapc-mail-fetch.c	Wed Jul 10 06:54:57 2013 +0300
@@ -48,7 +48,7 @@
 			"imapc: Mail prefetch failed: %s", reply->text_full);
 	}
 	pool_unref(&mail->imail.mail.pool);
-	imapc_client_stop(mbox->storage->client);
+	imapc_client_stop(mbox->storage->client->client);
 }
 
 static int
@@ -407,6 +407,6 @@
 	if (!match) {
 		/* this is only a FETCH FLAGS update for the wanted mail */
 	} else {
-		imapc_client_stop(mbox->storage->client);
+		imapc_client_stop(mbox->storage->client->client);
 	}
 }


More information about the dovecot-cvs mailing list