Fix error handling in sync_helper() and imap_sync_mailbox(). (closes #3817)

This patch is based on the one Richard Russon found in the Fedora package.

If an error occurs during one of the imap_exec() calls in
imap_sync_mailbox(), the mailbox could end up being closed.  This
would cause idata->ctx to be NULL.  Add a check in sync_helper() for
the case where idata->ctx == NULL.

In imap_sync_mailbox(), check the return value of sync_helper().  To
keep the code simple, change rc from being the sum of the calls to the
bitwise-OR of the calls.  (We only need to know if a single flag needs
to be updated, and bitwise-OR will detect negatives.)

Below the calls to sync_helper(), if the call to imap_exec() fails,
make sure rc is set to -1.

diff --git a/imap/imap.c b/imap/imap.c
index 0c58ce7bf78cf118ca487f7bab9743ad638e3358..1b63b3a31ca6030951ebaa84cdc5482761ef7eca 100644 (file)
--- a/imap/imap.c
+++ b/imap/imap.c
@@ -1120,9 +1120,11 @@ static int sync_helper (IMAP_DATA* idata, int right, int flag, const char* name)
 {
   int count = 0;
   int rc;
-
   char buf[LONG_STRING];
 
+  if (!idata->ctx)
+    return -1;
+
   if (!mutt_bit_isset (idata->ctx->rights, right))
     return 0;
 
@@ -1238,9 +1240,6 @@ int imap_sync_mailbox (CONTEXT* ctx, int expunge, int* index_hint)
   imap_hcache_close (idata);
 #endif
 
-  /* sync +/- flags for the five flags mutt cares about */
-  rc = 0;
-
   /* presort here to avoid doing 10 resorts in imap_exec_msgset */
   oldsort = Sort;
   if (Sort != SORT_ORDER)
@@ -1254,11 +1253,15 @@ int imap_sync_mailbox (CONTEXT* ctx, int expunge, int* index_hint)
            mutt_get_sort_func (SORT_ORDER));
   }
 
-  rc += sync_helper (idata, M_ACL_DELETE, M_DELETED, "\\Deleted");
-  rc += sync_helper (idata, M_ACL_WRITE, M_FLAG, "\\Flagged");
-  rc += sync_helper (idata, M_ACL_WRITE, M_OLD, "Old");
-  rc += sync_helper (idata, M_ACL_SEEN, M_READ, "\\Seen");
-  rc += sync_helper (idata, M_ACL_WRITE, M_REPLIED, "\\Answered");
+  rc = sync_helper (idata, M_ACL_DELETE, M_DELETED, "\\Deleted");
+  if (rc >= 0)
+    rc |= sync_helper (idata, M_ACL_WRITE, M_FLAG, "\\Flagged");
+  if (rc >= 0)
+    rc |= sync_helper (idata, M_ACL_WRITE, M_OLD, "Old");
+  if (rc >= 0)
+    rc |= sync_helper (idata, M_ACL_SEEN, M_READ, "\\Seen");
+  if (rc >= 0)
+    rc |= sync_helper (idata, M_ACL_WRITE, M_REPLIED, "\\Answered");
 
   if (oldsort != Sort)
   {
@@ -1267,7 +1270,12 @@ int imap_sync_mailbox (CONTEXT* ctx, int expunge, int* index_hint)
     ctx->hdrs = hdrs;
   }
 
-  if (rc && (imap_exec (idata, NULL, 0) != IMAP_CMD_OK))
+  /* Flush the queued flags if any were changed in sync_helper. */
+  if (rc > 0)
+    if (imap_exec (idata, NULL, 0) != IMAP_CMD_OK)
+      rc = -1;
+
+  if (rc < 0)
   {
     if (ctx->closing)
     {
@@ -1280,6 +1288,7 @@ int imap_sync_mailbox (CONTEXT* ctx, int expunge, int* index_hint)
     }
     else
       mutt_error _("Error saving flags");
+    rc = -1;
     goto out;
   }