Add I/O error handling and use filenames instead of file descriptors

10 files changed, 99 insertions, 68 deletions

diff --git a/.gitignore b/.gitignore
index da19d7c6..e06f5136 100644
--- a/.gitignore
+++ b/.gitignore
@@ -86,3 +86,4 @@ breaks
 
 *.tar.*
 *.zip
+*.log*
diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c
index bf6d6843..1a554ce4 100644
--- a/src/command/cmd_funcs.c
+++ b/src/command/cmd_funcs.c
@@ -9192,19 +9192,13 @@ _url_open_fallback_method(ProfWin* window, const char* url)
 void
 _url_save_fallback_method(ProfWin* window, const char* url, const char* filename)
 {
-    FILE* fh = fopen(filename, "wb");
-    if (!fh) {
-        cons_show_error("Cannot open file '%s' for writing.", filename);
-        return;
-    }
-
     gchar* scheme = g_uri_parse_scheme(url);
 
     if (g_strcmp0(scheme, "aesgcm") == 0) {
         AESGCMDownload* download = malloc(sizeof(AESGCMDownload));
         download->window = window;
         download->url = strdup(url);
-        download->filehandle = fh;
+        download->filename = strdup(filename);
 
         pthread_create(&(download->worker), NULL, &aesgcm_file_get, download);
         aesgcm_download_add_download(download);
@@ -9212,7 +9206,7 @@ _url_save_fallback_method(ProfWin* window, const char* url, const char* filename
         HTTPDownload* download = malloc(sizeof(HTTPDownload));
         download->window = window;
         download->url = strdup(url);
-        download->filehandle = fh;
+        download->filename = strdup(filename);
 
         pthread_create(&(download->worker), NULL, &http_file_get, download);
         http_download_add_download(download);
diff --git a/src/omemo/crypto.c b/src/omemo/crypto.c
index a05e160e..a4a2d5fc 100644
--- a/src/omemo/crypto.c
+++ b/src/omemo/crypto.c
@@ -35,7 +35,6 @@
 #include <assert.h>
 #include <signal/signal_protocol.h>
 #include <signal/signal_protocol_types.h>
-#include <gcrypt.h>
 
 #include "log.h"
 #include "omemo/omemo.h"
@@ -377,7 +376,7 @@ out:
     return res;
 }
 
-int
+gcry_error_t
 aes256gcm_crypt_file(FILE* in, FILE* out, off_t file_size,
                      unsigned char key[], unsigned char nonce[], bool encrypt)
 {
@@ -416,7 +415,7 @@ aes256gcm_crypt_file(FILE* in, FILE* out, off_t file_size,
     off_t bytes_read = 0, bytes_available = 0, read_size = 0;
     while (bytes_read < file_size) {
         bytes_available = file_size - bytes_read;
-        if (!bytes_available) {
+        if (!bytes_available || ferror(in) != 0) {
             break;
         }
 
diff --git a/src/omemo/crypto.h b/src/omemo/crypto.h
index c1d508b9..5adbffd8 100644
--- a/src/omemo/crypto.h
+++ b/src/omemo/crypto.h
@@ -35,6 +35,7 @@
 #include <stdio.h>
 #include <stdbool.h>
 #include <signal/signal_protocol_types.h>
+#include <gcrypt.h>
 
 #define AES128_GCM_KEY_LENGTH 16
 #define AES128_GCM_IV_LENGTH  12
@@ -183,8 +184,8 @@ int aes128gcm_decrypt(unsigned char* plaintext,
                       size_t ciphertext_len, const unsigned char* const iv, size_t iv_len,
                       const unsigned char* const key, const unsigned char* const tag);
 
-int aes256gcm_crypt_file(FILE* in, FILE* out, off_t file_size,
-                         unsigned char key[], unsigned char nonce[], bool encrypt);
+gcry_error_t aes256gcm_crypt_file(FILE* in, FILE* out, off_t file_size,
+                                  unsigned char key[], unsigned char nonce[], bool encrypt);
 
 char* aes256gcm_create_secure_fragment(unsigned char* key,
                                        unsigned char* nonce);
diff --git a/src/omemo/omemo.c b/src/omemo/omemo.c
index e19b724e..8b426320 100644
--- a/src/omemo/omemo.c
+++ b/src/omemo/omemo.c
@@ -1713,7 +1713,7 @@ _bytes_from_hex(const char* hex, size_t hex_size,
     }
 }
 
-int
+gcry_error_t
 omemo_decrypt_file(FILE* in, FILE* out, off_t file_size, const char* fragment)
 {
     char nonce_hex[AESGCM_URL_NONCE_LEN];
@@ -1733,7 +1733,8 @@ omemo_decrypt_file(FILE* in, FILE* out, off_t file_size, const char* fragment)
     _bytes_from_hex(key_hex, AESGCM_URL_KEY_LEN,
                     key, OMEMO_AESGCM_KEY_LENGTH);
 
-    int crypt_res = aes256gcm_crypt_file(in, out, file_size, key, nonce, false);
+    gcry_error_t crypt_res;
+    crypt_res = aes256gcm_crypt_file(in, out, file_size, key, nonce, false);
 
     gcry_free(key);
 
diff --git a/src/omemo/omemo.h b/src/omemo/omemo.h
index a0e89916..d90f11cf 100644
--- a/src/omemo/omemo.h
+++ b/src/omemo/omemo.h
@@ -101,6 +101,5 @@ char* omemo_on_message_send(ProfWin* win, const char* const message, gboolean re
 char* omemo_on_message_recv(const char* const from, uint32_t sid, const unsigned char* const iv, size_t iv_len, GList* keys, const unsigned char* const payload, size_t payload_len, gboolean muc, gboolean* trusted);
 
 char* omemo_encrypt_file(FILE* in, FILE* out, off_t file_size, int* gcry_res);
-int omemo_decrypt_file(FILE* in, FILE* out, off_t file_size, const char* fragment);
-void omemo_free(void* a);
+gcry_error_t omemo_decrypt_file(FILE* in, FILE* out, off_t file_size, const char* fragment); void omemo_free(void* a);
 int omemo_parse_aesgcm_url(const char* aesgcm_url, char** https_url, char** fragment);
diff --git a/src/tools/aesgcm_download.c b/src/tools/aesgcm_download.c
index 693eabe7..6e8b89c5 100644
--- a/src/tools/aesgcm_download.c
+++ b/src/tools/aesgcm_download.c
@@ -67,56 +67,87 @@ aesgcm_file_get(void* userdata)
     char* https_url = NULL;
     char* fragment = NULL;
 
+    const size_t err_len = 100;
+    char err_buf[err_len];
+
     if (omemo_parse_aesgcm_url(aesgcm_dl->url, &https_url, &fragment) != 0) {
         http_print_transfer_update(aesgcm_dl->window, aesgcm_dl->url,
-                                   "Download failed: Cannot parse URL.");
+                                   "Download failed: Cannot parse URL '%s'.",
+                                   aesgcm_dl->url);
         return NULL;
     }
 
-    int tmpfd;
     char* tmpname = NULL;
-    if ((tmpfd = g_file_open_tmp("profanity.XXXXXX", &tmpname, NULL)) == -1) {
+    if (g_file_open_tmp("profanity.XXXXXX", &tmpname, NULL) == -1) {
+        strerror_r(errno, err_buf, err_len);
         http_print_transfer_update(aesgcm_dl->window, aesgcm_dl->url,
                                    "Downloading '%s' failed: Unable to create "
-                                   "temporary ciphertext file for writing.",
-                                   https_url);
+                                   "temporary ciphertext file for writing "
+                                   "(%s).",
+                                   https_url, err_buf);
         return NULL;
     }
-    FILE* tmpfh = fdopen(tmpfd, "wb");
 
-    // Remove the file once it is closed.
-    remove(tmpname);
-    free(tmpname);
+    FILE* outfh = fopen(aesgcm_dl->filename, "wb");
+    if (outfh == NULL) {
+        strerror_r(errno, err_buf, err_len);
+        http_print_transfer_update(aesgcm_dl->window, aesgcm_dl->url,
+                                   "Downloading '%s' failed: Unable to open "
+                                   "output file at '%s' for writing (%s).",
+                                   https_url, aesgcm_dl->filename, err_buf);
+        return NULL;
+    }
 
     HTTPDownload* http_dl = malloc(sizeof(HTTPDownload));
     http_dl->window = aesgcm_dl->window;
     http_dl->worker = aesgcm_dl->worker;
-    http_dl->url = https_url;
-    http_dl->filehandle = tmpfh;
-    http_dl->close = 0;
+    http_dl->url = strdup(https_url);
+    http_dl->filename = strdup(tmpname);
 
     aesgcm_dl->http_dl = http_dl;
 
-    // TODO: Verify result.
-    http_file_get(http_dl);
+    http_file_get(http_dl); // TODO(wstrm): Verify result.
+
+    FILE* tmpfh = fopen(tmpname, "rb");
+    if (tmpfh == NULL) {
+        strerror_r(errno, err_buf, err_len);
+        http_print_transfer_update(aesgcm_dl->window, aesgcm_dl->url,
+                                   "Downloading '%s' failed: Unable to open "
+                                   "temporary file at '%s' for reading (%s).",
+                                   aesgcm_dl->url, aesgcm_dl->filename, err_buf);
+        return NULL;
+    }
 
-    // Force flush as the decrypt function will read from the same stream.
-    fflush(tmpfh);
-    rewind(tmpfh);
+    gcry_error_t crypt_res;
+    crypt_res = omemo_decrypt_file(tmpfh, outfh,
+                                   http_dl->bytes_received, fragment);
 
-    int crypt_res = omemo_decrypt_file(tmpfh, aesgcm_dl->filehandle,
-                                       http_dl->bytes_received, fragment);
+    if (fclose(tmpfh) == EOF) {
+        strerror_r(errno, err_buf, err_len);
+        cons_show_error(err_buf);
+    }
 
-    fclose(tmpfh);
+    remove(tmpname);
+    free(tmpname);
 
-    if (crypt_res != 0) {
+    if (crypt_res != GPG_ERR_NO_ERROR) {
         http_print_transfer_update(aesgcm_dl->window, aesgcm_dl->url,
-                                   "Downloading '%s' failed: Failed to decrypt"
-                                   "file.",
-                                   https_url);
+                                   "Downloading '%s' failed: Failed to decrypt "
+                                   "file (%s).",
+                                   https_url, gcry_strerror(crypt_res));
+    }
+
+    if (fclose(outfh) == EOF) {
+        strerror_r(errno, err_buf, err_len);
+        cons_show_error(err_buf);
     }
 
-    fclose(aesgcm_dl->filehandle);
+    free(https_url);
+    free(fragment);
+
+    free(aesgcm_dl->filename);
+    free(aesgcm_dl->url);
+    free(aesgcm_dl);
 
     return NULL;
 }
diff --git a/src/tools/aesgcm_download.h b/src/tools/aesgcm_download.h
index fc29a99e..ebc8a5f9 100644
--- a/src/tools/aesgcm_download.h
+++ b/src/tools/aesgcm_download.h
@@ -50,7 +50,7 @@
 typedef struct aesgcm_download_t
 {
     char* url;
-    FILE* filehandle;
+    char* filename;
     ProfWin* window;
     pthread_t worker;
     HTTPDownload* http_dl;
diff --git a/src/tools/http_download.c b/src/tools/http_download.c
index a86af172..5859dc70 100644
--- a/src/tools/http_download.c
+++ b/src/tools/http_download.c
@@ -109,6 +109,9 @@ http_file_get(void* userdata)
 {
     HTTPDownload* download = (HTTPDownload*)userdata;
 
+    const size_t err_len = 100;
+    char err_buf[err_len];
+
     char* err = NULL;
 
     CURL* curl;
@@ -118,12 +121,18 @@ http_file_get(void* userdata)
     download->bytes_received = 0;
 
     pthread_mutex_lock(&lock);
-    char* msg;
-    if (asprintf(&msg, "Downloading '%s': 0%%", download->url) == -1) {
-        msg = strdup(FALLBACK_MSG);
+    http_print_transfer_update(download->window, download->url,
+                               "Downloading '%s': 0%%", download->url);
+
+    FILE* outfh = fopen(download->filename, "wb");
+    if (outfh == NULL) {
+        strerror_r(errno, err_buf, err_len);
+        http_print_transfer_update(download->window, download->url,
+                                   "Downloading '%s' failed: Unable to open "
+                                   "output file at '%s' for writing (%s).",
+                                   download->url, download->filename, err_buf);
+        return NULL;
     }
-    win_print_http_transfer(download->window, msg, download->url);
-    free(msg);
 
     char* cert_path = prefs_get_string(PREF_TLS_CERTPATH);
     pthread_mutex_unlock(&lock);
@@ -142,7 +151,7 @@ http_file_get(void* userdata)
 #endif
     curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 0L);
 
-    curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void*)download->filehandle);
+    curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void*)outfh);
 
     curl_easy_setopt(curl, CURLOPT_USERAGENT, "profanity");
 
@@ -157,35 +166,31 @@ http_file_get(void* userdata)
     curl_easy_cleanup(curl);
     curl_global_cleanup();
 
-    if (download->filehandle && download->close) {
-        fclose(download->filehandle);
+    if (fclose(outfh) == EOF) {
+        strerror_r(errno, err_buf, err_len);
+        err = strdup(err_buf);
     }
 
     pthread_mutex_lock(&lock);
     g_free(cert_path);
     if (err) {
-        char* msg;
         if (download->cancel) {
-            if (asprintf(&msg, "Downloading '%s' failed: Download was canceled", download->url) == -1) {
-                msg = strdup(FALLBACK_MSG);
-            }
+            http_print_transfer_update(download->window, download->url,
+                                       "Downloading '%s' failed: "
+                                       "Download was canceled",
+                                       download->url);
         } else {
-            if (asprintf(&msg, "Downloading '%s' failed: %s", download->url, err) == -1) {
-                msg = strdup(FALLBACK_MSG);
-            }
-            win_update_entry_message(download->window, download->url, msg);
+            http_print_transfer_update(download->window, download->url,
+                                       "Downloading '%s' failed: %s",
+                                       download->url, err);
         }
-        cons_show_error(msg);
-        free(msg);
         free(err);
     } else {
         if (!download->cancel) {
-            if (asprintf(&msg, "Downloading '%s': 100%%", download->url) == -1) {
-                msg = strdup(FALLBACK_MSG);
-            }
-            win_update_entry_message(download->window, download->url, msg);
+            http_print_transfer_update(download->window, download->url,
+                                       "Downloading '%s': 100%%",
+                                       download->url);
             win_mark_received(download->window, download->url);
-            free(msg);
         }
     }
 
@@ -193,6 +198,7 @@ http_file_get(void* userdata)
     pthread_mutex_unlock(&lock);
 
     free(download->url);
+    free(download->filename);
     free(download);
 
     return NULL;
diff --git a/src/tools/http_download.h b/src/tools/http_download.h
index 797e1603..077e3e3d 100644
--- a/src/tools/http_download.h
+++ b/src/tools/http_download.h
@@ -49,12 +49,11 @@
 typedef struct http_download_t
 {
     char* url;
-    FILE* filehandle;
+    char* filename;
     curl_off_t bytes_received;
     ProfWin* window;
     pthread_t worker;
     int cancel;
-    int close;
 } HTTPDownload;
 
 void* http_file_get(void* userdata);