Rework and simplify how compose_queue() return values are handled
authorAndrej Kacian <ticho@claws-mail.org>
Tue, 10 Apr 2018 17:46:01 +0000 (19:46 +0200)
committerAndrej Kacian <ticho@claws-mail.org>
Fri, 13 Apr 2018 18:03:06 +0000 (20:03 +0200)
Also closes bug #3960 - Sends unencrypted emails when encryption fails
because we now return from compose_queue() with an error when
that happens, instead of ignoring it.

src/compose.c
src/compose.h
src/folder.c

index efb3b9c23af219b67bb58b76b68821ae1806bc3f..4cfa88b0c39d9562390b049cecf50dbb02df9faa 100644 (file)
@@ -314,7 +314,7 @@ static gint compose_write_body_to_file              (Compose        *compose,
 static gint compose_remove_reedit_target       (Compose        *compose,
                                                 gboolean        force);
 static void compose_remove_draft                       (Compose        *compose);
 static gint compose_remove_reedit_target       (Compose        *compose,
                                                 gboolean        force);
 static void compose_remove_draft                       (Compose        *compose);
-static gint compose_queue_sub                  (Compose        *compose,
+static ComposeQueueResult compose_queue_sub                    (Compose        *compose,
                                                 gint           *msgnum,
                                                 FolderItem     **item,
                                                 gchar          **msgpath,
                                                 gint           *msgnum,
                                                 FolderItem     **item,
                                                 gchar          **msgpath,
@@ -5204,11 +5204,47 @@ static gboolean compose_check_entries(Compose *compose, gboolean check_everythin
        return TRUE;
 }
 
        return TRUE;
 }
 
+static void _display_queue_error(ComposeQueueResult val)
+{
+       switch (val) {
+               case COMPOSE_QUEUE_SUCCESS:
+                       break;
+               case COMPOSE_QUEUE_ERROR_NO_MSG:
+                       alertpanel_error(_("Could not queue message."));
+                       break;
+               case COMPOSE_QUEUE_ERROR_WITH_ERRNO:
+                       alertpanel_error(_("Could not queue message:\n\n%s."),
+                                       g_strerror(errno));
+                       break;
+               case COMPOSE_QUEUE_ERROR_SIGNING_FAILED:
+                       alertpanel_error(_("Could not queue message for sending:\n\n"
+                                               "Signature failed: %s"), privacy_get_error());
+                       break;
+               case COMPOSE_QUEUE_ERROR_ENCRYPT_FAILED:
+                       alertpanel_error(_("Could not queue message for sending:\n\n"
+                                               "Encryption failed: %s"), privacy_get_error());
+                       break;
+               case COMPOSE_QUEUE_ERROR_CHAR_CONVERSION:
+                       alertpanel_error(_("Could not queue message for sending:\n\n"
+                                               "Charset conversion failed."));
+                       break;
+               case COMPOSE_QUEUE_ERROR_NO_ENCRYPTION_KEY:
+                       alertpanel_error(_("Could not queue message for sending:\n\n"
+                                               "Couldn't get recipient encryption key."));
+                       break;
+               default:
+                       /* unhandled error */
+                       debug_print("oops, unhandled compose_queue() return value %d\n",
+                                       val);
+                       break;
+       }
+}
+
 gint compose_send(Compose *compose)
 {
        gint msgnum;
        FolderItem *folder = NULL;
 gint compose_send(Compose *compose)
 {
        gint msgnum;
        FolderItem *folder = NULL;
-       gint val = -1;
+       ComposeQueueResult val = COMPOSE_QUEUE_ERROR_NO_MSG;
        gchar *msgpath = NULL;
        gboolean discard_window = FALSE;
        gchar *errstr = NULL;
        gchar *msgpath = NULL;
        gboolean discard_window = FALSE;
        gchar *errstr = NULL;
@@ -5233,27 +5269,13 @@ gint compose_send(Compose *compose)
        inc_lock();
        val = compose_queue(compose, &msgnum, &folder, &msgpath, TRUE);
 
        inc_lock();
        val = compose_queue(compose, &msgnum, &folder, &msgpath, TRUE);
 
-       if (val) {
+       if (val != COMPOSE_QUEUE_SUCCESS) {
                if (compose->batch) {
                        gtk_widget_show_all(compose->window);
                }
                if (compose->batch) {
                        gtk_widget_show_all(compose->window);
                }
-               if (val == -4) {
-                       alertpanel_error(_("Could not queue message for sending:\n\n"
-                                          "Charset conversion failed."));
-               } else if (val == -5) {
-                       alertpanel_error(_("Could not queue message for sending:\n\n"
-                                          "Couldn't get recipient encryption key."));
-               } else if (val == -6) {
-                       /* silent error */
-               } else if (val == -3) {
-                       if (privacy_peek_error())
-                       alertpanel_error(_("Could not queue message for sending:\n\n"
-                                          "Signature failed: %s"), privacy_get_error());
-               } else if (val == -2 && errno != 0) {
-                       alertpanel_error(_("Could not queue message for sending:\n\n%s."), g_strerror(errno));
-               } else {
-                       alertpanel_error(_("Could not queue message for sending."));
-               }
+
+               _display_queue_error(val);
+
                goto bail;
        }
 
                goto bail;
        }
 
@@ -5730,7 +5752,7 @@ static gint compose_write_to_file(Compose *compose, FILE *fp, gint action, gbool
 
                        if (aval != G_ALERTALTERNATE) {
                                g_free(chars);
 
                        if (aval != G_ALERTALTERNATE) {
                                g_free(chars);
-                               return -3;
+                               return COMPOSE_QUEUE_ERROR_CHAR_CONVERSION;
                        } else {
                                buf = chars;
                                out_codeset = src_codeset;
                        } else {
                                buf = chars;
                                out_codeset = src_codeset;
@@ -5791,7 +5813,7 @@ static gint compose_write_to_file(Compose *compose, FILE *fp, gint action, gbool
                g_free(msg);
                if (aval != G_ALERTALTERNATE) {
                        g_free(buf);
                g_free(msg);
                if (aval != G_ALERTALTERNATE) {
                        g_free(buf);
-                       return -1;
+                       return COMPOSE_QUEUE_ERROR_NO_MSG;
                }
        }
        
                }
        }
        
@@ -5894,8 +5916,7 @@ static gint compose_write_to_file(Compose *compose, FILE *fp, gint action, gbool
                        if (!privacy_encrypt(compose->privacy_system, mimemsg, compose->encdata)) {
                                debug_print("Couldn't encrypt mime structure: %s.\n",
                                                privacy_get_error());
                        if (!privacy_encrypt(compose->privacy_system, mimemsg, compose->encdata)) {
                                debug_print("Couldn't encrypt mime structure: %s.\n",
                                                privacy_get_error());
-                               alertpanel_error(_("Couldn't encrypt the email: %s"),
-                                               privacy_get_error());
+                               return COMPOSE_QUEUE_ERROR_ENCRYPT_FAILED;
                        }
                }
        }
                        }
                }
        }
@@ -6001,7 +6022,7 @@ static void compose_remove_draft(Compose *compose)
 
 }
 
 
 }
 
-gint compose_queue(Compose *compose, gint *msgnum, FolderItem **item, gchar **msgpath,
+ComposeQueueResult compose_queue(Compose *compose, gint *msgnum, FolderItem **item, gchar **msgpath,
                   gboolean remove_reedit_target)
 {
        return compose_queue_sub (compose, msgnum, item, msgpath, FALSE, remove_reedit_target);
                   gboolean remove_reedit_target)
 {
        return compose_queue_sub (compose, msgnum, item, msgpath, FALSE, remove_reedit_target);
@@ -6032,7 +6053,7 @@ static gboolean compose_warn_encryption(Compose *compose)
        } 
 }
 
        } 
 }
 
-static gint compose_queue_sub(Compose *compose, gint *msgnum, FolderItem **item, 
+static ComposeQueueResult compose_queue_sub(Compose *compose, gint *msgnum, FolderItem **item, 
                              gchar **msgpath, gboolean perform_checks,
                              gboolean remove_reedit_target)
 {
                              gchar **msgpath, gboolean perform_checks,
                              gboolean remove_reedit_target)
 {
@@ -6051,12 +6072,12 @@ static gint compose_queue_sub(Compose *compose, gint *msgnum, FolderItem **item,
                if (compose->batch) {
                        gtk_widget_show_all(compose->window);
                }
                if (compose->batch) {
                        gtk_widget_show_all(compose->window);
                }
-                return -1;
+                return COMPOSE_QUEUE_ERROR_NO_MSG;
        }
 
        if (!compose->to_list && !compose->newsgroup_list) {
                g_warning("can't get recipient list.");
        }
 
        if (!compose->to_list && !compose->newsgroup_list) {
                g_warning("can't get recipient list.");
-                return -1;
+                return COMPOSE_QUEUE_ERROR_NO_MSG;
         }
 
        if (compose->to_list) {
         }
 
        if (compose->to_list) {
@@ -6066,7 +6087,7 @@ static gint compose_queue_sub(Compose *compose, gint *msgnum, FolderItem **item,
                        mailac = cur_account;
                else if (!(mailac = compose_current_mail_account())) {
                        alertpanel_error(_("No account for sending mails available!"));
                        mailac = cur_account;
                else if (!(mailac = compose_current_mail_account())) {
                        alertpanel_error(_("No account for sending mails available!"));
-                       return -1;
+                       return COMPOSE_QUEUE_ERROR_NO_MSG;
                }
        }
 
                }
        }
 
@@ -6075,7 +6096,7 @@ static gint compose_queue_sub(Compose *compose, gint *msgnum, FolderItem **item,
                         newsac = compose->account;
                 else {
                        alertpanel_error(_("Selected account isn't NNTP: Posting is impossible."));
                         newsac = compose->account;
                 else {
                        alertpanel_error(_("Selected account isn't NNTP: Posting is impossible."));
-                       return -1;
+                       return COMPOSE_QUEUE_ERROR_NO_MSG;
                }                       
        }
 
                }                       
        }
 
@@ -6086,7 +6107,7 @@ static gint compose_queue_sub(Compose *compose, gint *msgnum, FolderItem **item,
        if ((fp = g_fopen(tmp, "w+b")) == NULL) {
                FILE_OP_ERROR(tmp, "fopen");
                g_free(tmp);
        if ((fp = g_fopen(tmp, "w+b")) == NULL) {
                FILE_OP_ERROR(tmp, "fopen");
                g_free(tmp);
-               return -2;
+               return COMPOSE_QUEUE_ERROR_WITH_ERRNO;
        }
 
        if (change_file_mode_rw(fp, tmp) < 0) {
        }
 
        if (change_file_mode_rw(fp, tmp) < 0) {
@@ -6149,7 +6170,7 @@ static gint compose_queue_sub(Compose *compose, gint *msgnum, FolderItem **item,
                                fclose(fp);
                                claws_unlink(tmp);
                                g_free(tmp);
                                fclose(fp);
                                claws_unlink(tmp);
                                g_free(tmp);
-                               return -6;
+                               return COMPOSE_QUEUE_ERROR_NO_MSG;
                        }
                        if (mailac && mailac->encrypt_to_self) {
                                GSList *tmp_list = g_slist_copy(compose->to_list);
                        }
                        if (mailac && mailac->encrypt_to_self) {
                                GSList *tmp_list = g_slist_copy(compose->to_list);
@@ -6174,7 +6195,7 @@ static gint compose_queue_sub(Compose *compose, gint *msgnum, FolderItem **item,
                                fclose(fp);
                                claws_unlink(tmp);
                                g_free(tmp);
                                fclose(fp);
                                claws_unlink(tmp);
                                g_free(tmp);
-                               return -5;
+                               return COMPOSE_QUEUE_ERROR_NO_ENCRYPTION_KEY;
                        }
                }
        }
                        }
                }
        }
@@ -6227,7 +6248,7 @@ static gint compose_queue_sub(Compose *compose, gint *msgnum, FolderItem **item,
                        fclose(fp);
                        claws_unlink(tmp);
                        g_free(tmp);
                        fclose(fp);
                        claws_unlink(tmp);
                        g_free(tmp);
-                       return -2;
+                       return COMPOSE_QUEUE_ERROR_WITH_ERRNO;
                }
        } else {
                gint result = 0;
                }
        } else {
                gint result = 0;
@@ -6235,7 +6256,7 @@ static gint compose_queue_sub(Compose *compose, gint *msgnum, FolderItem **item,
                        fclose(fp);
                        claws_unlink(tmp);
                        g_free(tmp);
                        fclose(fp);
                        claws_unlink(tmp);
                        g_free(tmp);
-                       return result - 1; /* -2 for a generic error, -3 for signing error, -4 for encoding */
+                       return result;
                }
        }
        if (err == TRUE) {
                }
        }
        if (err == TRUE) {
@@ -6243,13 +6264,13 @@ static gint compose_queue_sub(Compose *compose, gint *msgnum, FolderItem **item,
                fclose(fp);
                claws_unlink(tmp);
                g_free(tmp);
                fclose(fp);
                claws_unlink(tmp);
                g_free(tmp);
-               return -2;
+               return COMPOSE_QUEUE_ERROR_WITH_ERRNO;
        }
        if (fclose(fp) == EOF) {
                FILE_OP_ERROR(tmp, "fclose");
                claws_unlink(tmp);
                g_free(tmp);
        }
        if (fclose(fp) == EOF) {
                FILE_OP_ERROR(tmp, "fclose");
                claws_unlink(tmp);
                g_free(tmp);
-               return -2;
+               return COMPOSE_QUEUE_ERROR_WITH_ERRNO;
        }
 
        if (item && *item) {
        }
 
        if (item && *item) {
@@ -6261,14 +6282,14 @@ static gint compose_queue_sub(Compose *compose, gint *msgnum, FolderItem **item,
                g_warning("can't find queue folder");
                claws_unlink(tmp);
                g_free(tmp);
                g_warning("can't find queue folder");
                claws_unlink(tmp);
                g_free(tmp);
-               return -1;
+               return COMPOSE_QUEUE_ERROR_NO_MSG;
        }
        folder_item_scan(queue);
        if ((num = folder_item_add_msg(queue, tmp, NULL, FALSE)) < 0) {
                g_warning("can't queue the message");
                claws_unlink(tmp);
                g_free(tmp);
        }
        folder_item_scan(queue);
        if ((num = folder_item_add_msg(queue, tmp, NULL, FALSE)) < 0) {
                g_warning("can't queue the message");
                claws_unlink(tmp);
                g_free(tmp);
-               return -1;
+               return COMPOSE_QUEUE_ERROR_NO_MSG;
        }
        
        if (msgpath == NULL) {
        }
        
        if (msgpath == NULL) {
@@ -6286,7 +6307,7 @@ static gint compose_queue_sub(Compose *compose, gint *msgnum, FolderItem **item,
                *item = queue;
        }
 
                *item = queue;
        }
 
-       return 0;
+       return COMPOSE_QUEUE_SUCCESS;
 }
 
 static int compose_add_attachments(Compose *compose, MimeInfo *parent)
 }
 
 static int compose_add_attachments(Compose *compose, MimeInfo *parent)
@@ -10194,7 +10215,7 @@ static void compose_send_cb(GtkAction *action, gpointer data)
 static void compose_send_later_cb(GtkAction *action, gpointer data)
 {
        Compose *compose = (Compose *)data;
 static void compose_send_later_cb(GtkAction *action, gpointer data)
 {
        Compose *compose = (Compose *)data;
-       gint val;
+       ComposeQueueResult val;
 
        inc_lock();
        compose_allow_user_actions(compose, FALSE);
 
        inc_lock();
        compose_allow_user_actions(compose, FALSE);
@@ -10202,25 +10223,12 @@ static void compose_send_later_cb(GtkAction *action, gpointer data)
        compose_allow_user_actions(compose, TRUE);
        inc_unlock();
 
        compose_allow_user_actions(compose, TRUE);
        inc_unlock();
 
-       if (!val) {
+       if (val == COMPOSE_QUEUE_SUCCESS) {
                compose_close(compose);
                compose_close(compose);
-       } else if (val == -1) {
-               alertpanel_error(_("Could not queue message."));
-       } else if (val == -2) {
-               alertpanel_error(_("Could not queue message:\n\n%s."), g_strerror(errno));
-       } else if (val == -3) {
-               if (privacy_peek_error())
-               alertpanel_error(_("Could not queue message for sending:\n\n"
-                                  "Signature failed: %s"), privacy_get_error());
-       } else if (val == -4) {
-               alertpanel_error(_("Could not queue message for sending:\n\n"
-                                  "Charset conversion failed."));
-       } else if (val == -5) {
-               alertpanel_error(_("Could not queue message for sending:\n\n"
-                                  "Couldn't get recipient encryption key."));
-       } else if (val == -6) {
-               /* silent error */
+       } else {
+               _display_queue_error(val);
        }
        }
+
        toolbar_main_set_sensitive(mainwindow_get_mainwindow());
 }
 
        toolbar_main_set_sensitive(mainwindow_get_mainwindow());
 }
 
index fee1956f5103b0200d247183257d028d77e498b9..d0d920aee6639fd04343dc55c64f2c7188e86290 100644 (file)
@@ -78,6 +78,17 @@ typedef enum
        COMPOSE_REEDIT
 } ComposeMode;
 
        COMPOSE_REEDIT
 } ComposeMode;
 
+typedef enum
+{
+       COMPOSE_QUEUE_SUCCESS = 0,
+       COMPOSE_QUEUE_ERROR_NO_MSG = -1,
+       COMPOSE_QUEUE_ERROR_WITH_ERRNO = -2,
+       COMPOSE_QUEUE_ERROR_SIGNING_FAILED = -3,
+       COMPOSE_QUEUE_ERROR_ENCRYPT_FAILED = -4,
+       COMPOSE_QUEUE_ERROR_CHAR_CONVERSION = -5,
+       COMPOSE_QUEUE_ERROR_NO_ENCRYPTION_KEY = -6
+} ComposeQueueResult;
+
 typedef enum {
        PREF_ACCOUNT,
        PREF_FOLDER,
 typedef enum {
        PREF_ACCOUNT,
        PREF_FOLDER,
@@ -339,7 +350,7 @@ gboolean compose_search_string                      (Compose        *compose,
 gboolean compose_search_string_backward        (Compose        *compose,
                                                 const gchar    *str,
                                                 gboolean        case_sens);
 gboolean compose_search_string_backward        (Compose        *compose,
                                                 const gchar    *str,
                                                 gboolean        case_sens);
-gint compose_queue                     (Compose *compose, 
+ComposeQueueResult compose_queue       (Compose *compose,
                                         gint *msgnum, 
                                         FolderItem **item, 
                                         gchar **msgpath,
                                         gint *msgnum, 
                                         FolderItem **item, 
                                         gchar **msgpath,
index 3f35f5ec078c1a4c88a19e6ee9ae2f3040f98eda..503a0492268a70201f8eeeb887158dccd35f4e70 100644 (file)
@@ -3474,7 +3474,7 @@ static gint do_copy_msgs(FolderItem *dest, GSList *msglist, gboolean remove_sour
                for (; cur; cur = cur->next) {
                        Compose *compose = NULL;
                        FolderItem *queue = dest;
                for (; cur; cur = cur->next) {
                        Compose *compose = NULL;
                        FolderItem *queue = dest;
-                       int val = 0;
+                       ComposeQueueResult val = COMPOSE_QUEUE_SUCCESS;
                        
                        msginfo = (MsgInfo *)cur->data;
                        compose = compose_reedit(msginfo, TRUE);
                        
                        msginfo = (MsgInfo *)cur->data;
                        compose = compose_reedit(msginfo, TRUE);
@@ -3484,12 +3484,12 @@ static gint do_copy_msgs(FolderItem *dest, GSList *msglist, gboolean remove_sour
                        }
                        val = compose_queue(compose, NULL, &queue, NULL,
                                        FALSE);
                        }
                        val = compose_queue(compose, NULL, &queue, NULL,
                                        FALSE);
-                       if (val < 0) {
+                       if (val != COMPOSE_QUEUE_SUCCESS) {
                                queue_err = TRUE;
                        } else if (remove_source) {
                                folder_item_remove_msg(msginfo->folder, msginfo->msgnum);
                        }
                                queue_err = TRUE;
                        } else if (remove_source) {
                                folder_item_remove_msg(msginfo->folder, msginfo->msgnum);
                        }
-                       if (val == 0)
+                       if (val == COMPOSE_QUEUE_SUCCESS)
                                compose_close(compose);
                }
                return queue_err ? -1:0;
                                compose_close(compose);
                }
                return queue_err ? -1:0;