GNOME.org
 

[evolution-data-server] Bug #648468 - POP3 doesn't recover or claim error after lost connection

commit 3d2a277bb82d5b5710b78771a3f072f5825343ad
Author: Milan Crha <mcrha redhat com>
Date:   Tue Jun 21 12:27:40 2011 +0200

    Bug #648468 - POP3 doesn't recover or claim error after lost connection

 camel/providers/pop3/camel-pop3-engine.c |   14 +++---
 camel/providers/pop3/camel-pop3-engine.h |    2 +-
 camel/providers/pop3/camel-pop3-folder.c |   79 ++++++++++++++++++------------
 camel/providers/pop3/camel-pop3-folder.h |    2 +-
 camel/providers/pop3/camel-pop3-store.c  |   23 +++++----
 camel/providers/pop3/camel-pop3-stream.c |    8 +++-
 6 files changed, 76 insertions(+), 52 deletions(-)
---
diff --git a/camel/providers/pop3/camel-pop3-engine.c b/camel/providers/pop3/camel-pop3-engine.c
index 5d0b8878..99d5d08 100644
--- a/camel/providers/pop3/camel-pop3-engine.c
+++ b/camel/providers/pop3/camel-pop3-engine.c
@@ -221,14 +221,14 @@ get_capabilities (CamelPOP3Engine *pe)
 	CamelPOP3Command *pc;
 
 	if (!(pe->flags & CAMEL_POP3_ENGINE_DISABLE_EXTENSIONS)) {
-		pc = camel_pop3_engine_command_new(pe, CAMEL_POP3_COMMAND_MULTI, cmd_capa, NULL, "CAPA\r\n");
+		pc = camel_pop3_engine_command_new(pe, CAMEL_POP3_COMMAND_MULTI, cmd_capa, NULL, NULL, NULL, "CAPA\r\n");
 		while (camel_pop3_engine_iterate (pe, pc) > 0)
 			;
 		camel_pop3_engine_command_free (pe, pc);
 
 		if (pe->state == CAMEL_POP3_ENGINE_TRANSACTION && !(pe->capa & CAMEL_POP3_CAP_UIDL)) {
 			/* check for UIDL support manually */
-			pc = camel_pop3_engine_command_new (pe, CAMEL_POP3_COMMAND_SIMPLE, NULL, NULL, "UIDL 1\r\n");
+			pc = camel_pop3_engine_command_new (pe, CAMEL_POP3_COMMAND_SIMPLE, NULL, NULL, NULL, NULL, "UIDL 1\r\n");
 			while (camel_pop3_engine_iterate (pe, pc) > 0)
 				;
 
@@ -241,8 +241,8 @@ get_capabilities (CamelPOP3Engine *pe)
 }
 
 /* returns true if the command was sent, false if it was just queued */
-static gint
-engine_command_queue (CamelPOP3Engine *pe, CamelPOP3Command *pc)
+static gboolean
+engine_command_queue (CamelPOP3Engine *pe, CamelPOP3Command *pc, GCancellable *cancellable, GError **error)
 {
 	if (((pe->capa & CAMEL_POP3_CAP_PIPE) == 0 || (pe->sentlen + strlen (pc->data)) > CAMEL_POP3_SEND_LIMIT)
 	    && pe->current != NULL) {
@@ -251,7 +251,7 @@ engine_command_queue (CamelPOP3Engine *pe, CamelPOP3Command *pc)
 	}
 
 	/* ??? */
-	if (camel_stream_write ((CamelStream *) pe->stream, pc->data, strlen (pc->data), NULL, NULL) == -1) {
+	if (camel_stream_write ((CamelStream *) pe->stream, pc->data, strlen (pc->data), cancellable, error) == -1) {
 		camel_dlist_addtail (&pe->queue, (CamelDListNode *) pc);
 		return FALSE;
 	}
@@ -377,7 +377,7 @@ ioerror:
 }
 
 CamelPOP3Command *
-camel_pop3_engine_command_new (CamelPOP3Engine *pe, guint32 flags, CamelPOP3CommandFunc func, gpointer data, const gchar *fmt, ...)
+camel_pop3_engine_command_new (CamelPOP3Engine *pe, guint32 flags, CamelPOP3CommandFunc func, gpointer data, GCancellable *cancellable, GError **error, const gchar *fmt, ...)
 {
 	CamelPOP3Command *pc;
 	va_list ap;
@@ -392,7 +392,7 @@ camel_pop3_engine_command_new (CamelPOP3Engine *pe, guint32 flags, CamelPOP3Comm
 	pc->state = CAMEL_POP3_COMMAND_IDLE;
 
 	/* TODO: what about write errors? */
-	engine_command_queue (pe, pc);
+	engine_command_queue (pe, pc, cancellable, error);
 
 	return pc;
 }
diff --git a/camel/providers/pop3/camel-pop3-engine.h b/camel/providers/pop3/camel-pop3-engine.h
index d766cb4..e7ff7c5 100644
--- a/camel/providers/pop3/camel-pop3-engine.h
+++ b/camel/providers/pop3/camel-pop3-engine.h
@@ -146,7 +146,7 @@ void              camel_pop3_engine_command_free (CamelPOP3Engine *pe, CamelPOP3
 
 gint		  camel_pop3_engine_iterate	(CamelPOP3Engine *pe, CamelPOP3Command *pc);
 
-CamelPOP3Command *camel_pop3_engine_command_new	(CamelPOP3Engine *pe, guint32 flags, CamelPOP3CommandFunc func, gpointer data, const gchar *fmt, ...);
+CamelPOP3Command *camel_pop3_engine_command_new	(CamelPOP3Engine *pe, guint32 flags, CamelPOP3CommandFunc func, gpointer data, GCancellable *cancellable, GError **error, const gchar *fmt, ...);
 
 G_END_DECLS
 
diff --git a/camel/providers/pop3/camel-pop3-folder.c b/camel/providers/pop3/camel-pop3-folder.c
index 724899e..9e892e5 100644
--- a/camel/providers/pop3/camel-pop3-folder.c
+++ b/camel/providers/pop3/camel-pop3-folder.c
@@ -63,7 +63,7 @@ cmd_uidl (CamelPOP3Engine *pe,
 				if (fi) {
 					camel_operation_progress (NULL, (fi->index+1) * 100 / folder->uids->len);
 					fi->uid = g_strdup (uid);
-					g_hash_table_insert (folder->uids_uid, fi->uid, fi);
+					g_hash_table_insert (folder->uids_fi, fi->uid, fi);
 				} else {
 					g_warning("ID %u (uid: %s) not in previous LIST output", id, uid);
 				}
@@ -143,7 +143,7 @@ cmd_list (CamelPOP3Engine *pe, CamelPOP3Stream *stream, gpointer data)
 				fi->id = id;
 				fi->index = ((CamelPOP3Folder *) folder)->uids->len;
 				if ((pop3_store->engine->capa & CAMEL_POP3_CAP_UIDL) == 0)
-					fi->cmd = camel_pop3_engine_command_new(pe, CAMEL_POP3_COMMAND_MULTI, cmd_builduid, fi, "TOP %u 0\r\n", id);
+					fi->cmd = camel_pop3_engine_command_new(pe, CAMEL_POP3_COMMAND_MULTI, cmd_builduid, fi, NULL, NULL, "TOP %u 0\r\n", id);
 				g_ptr_array_add (((CamelPOP3Folder *) folder)->uids, fi);
 				g_hash_table_insert (((CamelPOP3Folder *) folder)->uids_id, GINT_TO_POINTER (id), fi);
 			}
@@ -201,29 +201,34 @@ pop3_folder_dispose (GObject *object)
 {
 	CamelPOP3Folder *pop3_folder = CAMEL_POP3_FOLDER (object);
 	CamelPOP3FolderInfo **fi = (CamelPOP3FolderInfo **) pop3_folder->uids->pdata;
-	CamelPOP3Store *pop3_store;
+	CamelPOP3Store *pop3_store = NULL;
 	CamelStore *parent_store;
-	gint i;
 
 	parent_store = camel_folder_get_parent_store (CAMEL_FOLDER (object));
-	if (parent_store) {
+	if (parent_store)
 		pop3_store = CAMEL_POP3_STORE (parent_store);
 
-		if (pop3_folder->uids) {
-			for (i = 0; i < pop3_folder->uids->len; i++, fi++) {
-				if (fi[0]->cmd) {
-					while (camel_pop3_engine_iterate (pop3_store->engine, fi[0]->cmd) > 0)
-						;
-					camel_pop3_engine_command_free (pop3_store->engine, fi[0]->cmd);
-				}
+	if (pop3_folder->uids) {
+		gint i;
 
-				g_free (fi[0]->uid);
-				g_free (fi[0]);
+		for (i = 0; i < pop3_folder->uids->len; i++, fi++) {
+			if (fi[0]->cmd && pop3_store) {
+				while (camel_pop3_engine_iterate (pop3_store->engine, fi[0]->cmd) > 0)
+					;
+				camel_pop3_engine_command_free (pop3_store->engine, fi[0]->cmd);
 			}
 
-			g_ptr_array_free (pop3_folder->uids, TRUE);
-			g_hash_table_destroy (pop3_folder->uids_uid);
+			g_free (fi[0]->uid);
+			g_free (fi[0]);
 		}
+
+		g_ptr_array_free (pop3_folder->uids, TRUE);
+		pop3_folder->uids = NULL;
+	}
+
+	if (pop3_folder->uids_fi) {
+		g_hash_table_destroy (pop3_folder->uids_fi);
+		pop3_folder->uids_fi = NULL;
 	}
 
 	/* Chain up to parent's dispose() method. */
@@ -269,7 +274,7 @@ pop3_folder_get_filename (CamelFolder *folder,
 	pop3_folder = CAMEL_POP3_FOLDER (folder);
 	pop3_store = CAMEL_POP3_STORE (parent_store);
 
-	fi = g_hash_table_lookup (pop3_folder->uids_uid, uid);
+	fi = g_hash_table_lookup (pop3_folder->uids_fi, uid);
 	if (fi == NULL) {
 		g_set_error (
 			error, CAMEL_FOLDER_ERROR,
@@ -292,7 +297,7 @@ pop3_folder_set_message_flags (CamelFolder *folder,
 	CamelPOP3FolderInfo *fi;
 	gboolean res = FALSE;
 
-	fi = g_hash_table_lookup (pop3_folder->uids_uid, uid);
+	fi = g_hash_table_lookup (pop3_folder->uids_fi, uid);
 	if (fi) {
 		guint32 new = (fi->flags & ~flags) | (set & flags);
 
@@ -328,7 +333,7 @@ pop3_folder_get_message_sync (CamelFolder *folder,
 	pop3_folder = CAMEL_POP3_FOLDER (folder);
 	pop3_store = CAMEL_POP3_STORE (parent_store);
 
-	fi = g_hash_table_lookup (pop3_folder->uids_uid, uid);
+	fi = g_hash_table_lookup (pop3_folder->uids_fi, uid);
 	if (fi == NULL) {
 		g_set_error (
 			error, CAMEL_FOLDER_ERROR,
@@ -388,7 +393,7 @@ pop3_folder_get_message_sync (CamelFolder *folder,
 		/* ref it, the cache storage routine unref's when done */
 		fi->stream = g_object_ref (stream);
 		fi->err = EIO;
-		pcr = camel_pop3_engine_command_new(pop3_store->engine, CAMEL_POP3_COMMAND_MULTI, cmd_tocache, fi, "RETR %u\r\n", fi->id);
+		pcr = camel_pop3_engine_command_new(pop3_store->engine, CAMEL_POP3_COMMAND_MULTI, cmd_tocache, fi, NULL, NULL, "RETR %u\r\n", fi->id);
 
 		/* Also initiate retrieval of some of the following messages, assume we'll be receiving them */
 		if (pop3_store->cache != NULL) {
@@ -404,7 +409,7 @@ pop3_folder_get_message_sync (CamelFolder *folder,
 					if (pfi->stream) {
 						pfi->err = EIO;
 						pfi->cmd = camel_pop3_engine_command_new (pop3_store->engine, CAMEL_POP3_COMMAND_MULTI,
-											 cmd_tocache, pfi, "RETR %u\r\n", pfi->id);
+											 cmd_tocache, pfi, NULL, NULL, "RETR %u\r\n", pfi->id);
 					}
 				}
 			}
@@ -479,6 +484,7 @@ pop3_folder_refresh_info_sync (CamelFolder *folder,
 	CamelPOP3Folder *pop3_folder = (CamelPOP3Folder *) folder;
 	CamelPOP3Command *pcl, *pcu = NULL;
 	gboolean success = TRUE;
+	GError *local_error = NULL;
 	gint i;
 
 	parent_store = camel_folder_get_parent_store (folder);
@@ -487,18 +493,19 @@ pop3_folder_refresh_info_sync (CamelFolder *folder,
 	camel_operation_push_message (
 		cancellable, _("Retrieving POP summary"));
 
-	pop3_folder->uids = g_ptr_array_new ();
-	pop3_folder->uids_uid = g_hash_table_new (g_str_hash, g_str_equal);
 	/* only used during setup */
 	pop3_folder->uids_id = g_hash_table_new (NULL, NULL);
 
-	pcl = camel_pop3_engine_command_new(pop3_store->engine, CAMEL_POP3_COMMAND_MULTI, cmd_list, folder, "LIST\r\n");
-	if (pop3_store->engine->capa & CAMEL_POP3_CAP_UIDL)
-		pcu = camel_pop3_engine_command_new(pop3_store->engine, CAMEL_POP3_COMMAND_MULTI, cmd_uidl, folder, "UIDL\r\n");
+	pcl = camel_pop3_engine_command_new(pop3_store->engine, CAMEL_POP3_COMMAND_MULTI, cmd_list, folder, cancellable, &local_error, "LIST\r\n");
+	if (!local_error && (pop3_store->engine->capa & CAMEL_POP3_CAP_UIDL) != 0)
+		pcu = camel_pop3_engine_command_new(pop3_store->engine, CAMEL_POP3_COMMAND_MULTI, cmd_uidl, folder, cancellable, &local_error, "UIDL\r\n");
 	while ((i = camel_pop3_engine_iterate (pop3_store->engine, NULL)) > 0)
 		;
 
-	if (i == -1) {
+	if (local_error) {
+		g_propagate_error (error, local_error);
+		success = FALSE;
+	} else if (i == -1) {
 		if (errno == EINTR)
 			g_set_error (
 				error, G_IO_ERROR,
@@ -515,9 +522,10 @@ pop3_folder_refresh_info_sync (CamelFolder *folder,
 
 	/* TODO: check every id has a uid & commands returned OK too? */
 
-	camel_pop3_engine_command_free (pop3_store->engine, pcl);
+	if (pcl)
+		camel_pop3_engine_command_free (pop3_store->engine, pcl);
 
-	if (pop3_store->engine->capa & CAMEL_POP3_CAP_UIDL) {
+	if (pcu) {
 		camel_pop3_engine_command_free (pop3_store->engine, pcu);
 	} else {
 		for (i=0;i<pop3_folder->uids->len;i++) {
@@ -526,16 +534,21 @@ pop3_folder_refresh_info_sync (CamelFolder *folder,
 				camel_pop3_engine_command_free (pop3_store->engine, fi->cmd);
 				fi->cmd = NULL;
 			}
-			if (fi->uid)
-				g_hash_table_insert (pop3_folder->uids_uid, fi->uid, fi);
+			if (fi->uid) {
+				g_hash_table_insert (pop3_folder->uids_fi, fi->uid, fi);
+			}
 		}
 	}
 
 	/* dont need this anymore */
 	g_hash_table_destroy (pop3_folder->uids_id);
+	pop3_folder->uids_id = NULL;
 
 	camel_operation_pop_message (cancellable);
 
+	if (!success)
+		camel_service_disconnect_sync ((CamelService *) pop3_store, TRUE, NULL);
+
 	return success;
 }
 
@@ -591,6 +604,7 @@ pop3_folder_synchronize_sync (CamelFolder *folder,
 								0,
 								NULL,
 								NULL,
+								NULL, NULL,
 								"DELE %u\r\n",
 								fi->id);
 
@@ -643,6 +657,8 @@ camel_pop3_folder_class_init (CamelPOP3FolderClass *class)
 static void
 camel_pop3_folder_init (CamelPOP3Folder *pop3_folder)
 {
+	pop3_folder->uids = g_ptr_array_new ();
+	pop3_folder->uids_fi = g_hash_table_new (g_str_hash, g_str_equal);
 }
 
 CamelFolder *
@@ -784,6 +800,7 @@ camel_pop3_delete_old (CamelFolder *folder,
 									0,
 									NULL,
 									NULL,
+									NULL, NULL,
 									"DELE %u\r\n",
 									fi->id);
 				/* also remove from cache */
diff --git a/camel/providers/pop3/camel-pop3-folder.h b/camel/providers/pop3/camel-pop3-folder.h
index 6a5592a..42e4be0 100644
--- a/camel/providers/pop3/camel-pop3-folder.h
+++ b/camel/providers/pop3/camel-pop3-folder.h
@@ -68,7 +68,7 @@ struct _CamelPOP3Folder {
 	CamelFolder parent;
 
 	GPtrArray *uids;
-	GHashTable *uids_uid;	/* messageinfo by uid */
+	GHashTable *uids_fi;	/* messageinfo uid to CamelPOP3FolderInfo *, which is stored in uids array */
 	GHashTable *uids_id;	/* messageinfo by id */
 };
 
diff --git a/camel/providers/pop3/camel-pop3-store.c b/camel/providers/pop3/camel-pop3-store.c
index c3bc1d3..22e5526 100644
--- a/camel/providers/pop3/camel-pop3-store.c
+++ b/camel/providers/pop3/camel-pop3-store.c
@@ -204,7 +204,7 @@ connect_to_server (CamelService *service,
 		goto stls_exception;
 	}
 
-	pc = camel_pop3_engine_command_new (store->engine, 0, NULL, NULL, "STLS\r\n");
+	pc = camel_pop3_engine_command_new (store->engine, 0, NULL, NULL, cancellable, error, "STLS\r\n");
 	while (camel_pop3_engine_iterate (store->engine, NULL) > 0)
 		;
 
@@ -253,7 +253,7 @@ connect_to_server (CamelService *service,
  stls_exception:
 	if (clean_quit) {
 		/* try to disconnect cleanly */
-		pc = camel_pop3_engine_command_new (store->engine, 0, NULL, NULL, "QUIT\r\n");
+		pc = camel_pop3_engine_command_new (store->engine, 0, NULL, NULL, NULL, NULL, "QUIT\r\n");
 		while (camel_pop3_engine_iterate (store->engine, NULL) > 0)
 			;
 		camel_pop3_engine_command_free (store->engine, pc);
@@ -436,10 +436,10 @@ pop3_try_authenticate (CamelService *service,
 	if (!url->authmech) {
 		/* pop engine will take care of pipelining ability */
 		pcu = camel_pop3_engine_command_new (
-			store->engine, 0, NULL, NULL,
+			store->engine, 0, NULL, NULL, cancellable, error,
 			"USER %s\r\n", url->user);
 		pcp = camel_pop3_engine_command_new (
-			store->engine, 0, NULL, NULL,
+			store->engine, 0, NULL, NULL, cancellable, error,
 			"PASS %s\r\n", url->passwd);
 	} else if (strcmp (url->authmech, "+APOP") == 0 && store->engine->apop) {
 		gchar *secret, *md5asc, *d;
@@ -467,8 +467,8 @@ pop3_try_authenticate (CamelService *service,
 		sprintf(secret, "%s%s",  store->engine->apop, url->passwd);
 		md5asc = g_compute_checksum_for_string (G_CHECKSUM_MD5, secret, -1);
 		pcp = camel_pop3_engine_command_new (
-			store->engine, 0, NULL, NULL, "APOP %s %s\r\n",
-			url->user, md5asc);
+			store->engine, 0, NULL, NULL, cancellable, error,
+			"APOP %s %s\r\n", url->user, md5asc);
 		g_free (md5asc);
 	} else {
 		CamelServiceAuthType *auth;
@@ -657,11 +657,12 @@ pop3_store_disconnect_sync (CamelService *service,
 {
 	CamelServiceClass *service_class;
 	CamelPOP3Store *store = CAMEL_POP3_STORE (service);
+	gboolean success;
 
 	if (clean) {
 		CamelPOP3Command *pc;
 
-		pc = camel_pop3_engine_command_new(store->engine, 0, NULL, NULL, "QUIT\r\n");
+		pc = camel_pop3_engine_command_new(store->engine, 0, NULL, NULL, cancellable, error, "QUIT\r\n");
 		while (camel_pop3_engine_iterate (store->engine, NULL) > 0)
 			;
 		camel_pop3_engine_command_free (store->engine, pc);
@@ -669,13 +670,13 @@ pop3_store_disconnect_sync (CamelService *service,
 
 	/* Chain up to parent's disconnect() method. */
 	service_class = CAMEL_SERVICE_CLASS (camel_pop3_store_parent_class);
-	if (!service_class->disconnect_sync (service, clean, cancellable, error))
-		return FALSE;
+
+	success = service_class->disconnect_sync (service, clean, cancellable, error);
 
 	g_object_unref (store->engine);
 	store->engine = NULL;
 
-	return TRUE;
+	return success;
 }
 
 static GList *
@@ -810,7 +811,7 @@ camel_pop3_store_expunge (CamelPOP3Store *store,
 	CamelPOP3Command *pc;
 
 	pc = camel_pop3_engine_command_new (
-		store->engine, 0, NULL, NULL, "QUIT\r\n");
+		store->engine, 0, NULL, NULL, NULL, error, "QUIT\r\n");
 
 	while (camel_pop3_engine_iterate (store->engine, NULL) > 0)
 		;
diff --git a/camel/providers/pop3/camel-pop3-stream.c b/camel/providers/pop3/camel-pop3-stream.c
index 8bbf8eb..979beb6 100644
--- a/camel/providers/pop3/camel-pop3-stream.c
+++ b/camel/providers/pop3/camel-pop3-stream.c
@@ -72,6 +72,7 @@ stream_fill (CamelPOP3Stream *is,
              GError **error)
 {
 	gint left = 0;
+	GError *local_error = NULL;
 
 	if (is->source) {
 		left = is->end - is->ptr;
@@ -81,7 +82,12 @@ stream_fill (CamelPOP3Stream *is,
 		left = camel_stream_read (
 			is->source, (gchar *) is->end,
 			CAMEL_POP3_STREAM_SIZE - (is->end - is->buf),
-			cancellable, error);
+			cancellable, &local_error);
+		if (local_error) {
+			dd (printf ("POP3_STREAM_FILL: Failed to read bytes: %s\n", local_error->message));
+			g_propagate_error (error, local_error);
+		}
+
 		if (left > 0) {
 			is->end += left;
 			is->end[0] = '\n';
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]