Merge remote-tracking branch 'peetah/externalCallWithGSpawnSync' into

peetah-external-cmd

Merge peetahs external command calling improvements from his mirror at
https://framagit.org/peetah/profanity.

src/command/cmd_funcs.c

@@ -4900,7 +4900,7 @@ cmd_sendfile(ProfWin *window, const char *const command, gchar **args)
 			free(filename);
 			return TRUE;
         }
-        
+
     if (access(filename, R_OK) != 0) {
         cons_show_error("Uploading '%s' failed: File not found!", filename);
         free(filename);
@@ -8921,9 +8921,10 @@ cmd_urlopen(ProfWin *window, const char *const command, gchar **args)
             return TRUE;
         }
 
-        gchar* cmd = prefs_get_string(PREF_URL_OPEN_CMD);
-        call_external(cmd, args[0]);
-        g_free(cmd);
+        gchar *argv[] = {prefs_get_string(PREF_URL_OPEN_CMD), args[0], NULL};
+        if (!call_external(argv, NULL, NULL)) {
+          cons_show_error("Unable to open url: check the logs for more information.");
+        }
     } else {
         cons_show("urlopen not supported in this window");
     }

src/common.c

@@ -484,15 +484,90 @@ get_mentions(gboolean whole_word, gboolean case_sensitive, const char *const mes
     return mentions;
 }
 
-void
-call_external(const char *const exe, const char *const param)
+/*
+ * Take an NULL-terminated array used as the tokens of a command, and optionally
+ * pointers to the string arrays that will store each lines of the call standard
+ * output and standard error.
+ *
+ * argv - NULL-terminated string array containing the tokens of the command
+ *        line to spawn
+ * output_ptr - a pointer to the string array where to store spawned command
+ *              standard output
+ *              set to NULL to ignore the command standard output
+ * error_ptr - a pointer to the string array where to store spawned command
+ *             standard error
+ *             set to NULL to ignore the command standard error
+ *
+ * Returns:
+ * - TRUE if the command has been successfully spawned and exited normally
+ * - FALSE otherwise
+ */
+gboolean
+call_external(gchar **argv, gchar ***const output_ptr, gchar ***const error_ptr)
 {
-    GString *cmd = g_string_new("");
+    gchar *stdout_str = NULL;
+    gchar **stdout_str_ptr = &stdout_str;
+    gchar *stderr_str = NULL;
+    gchar **stderr_str_ptr = &stderr_str;
+    GSpawnFlags flags = G_SPAWN_SEARCH_PATH;
+    gint status;
+    GError *error = NULL;
+    gchar *cmd = NULL;
 
-    g_string_append_printf(cmd, "%s %s > /dev/null 2>&1", exe, param);
-    log_debug("Calling external: %s", cmd->str);
-    FILE *stream = popen(cmd->str, "r");
+    cmd = g_strjoinv(" ", argv);
+    log_debug("Calling external: %s", cmd);
 
-    pclose(stream);
-    g_string_free(cmd, TRUE);
+    if (!output_ptr) {
+        stdout_str_ptr = NULL;
+        flags |= G_SPAWN_STDOUT_TO_DEV_NULL;
+    }
+
+    if (!error_ptr) {
+        stderr_str_ptr = NULL;
+        flags |= G_SPAWN_STDERR_TO_DEV_NULL;
+    }
+
+    if (!g_spawn_sync (NULL, argv, NULL, flags, NULL, NULL, stdout_str_ptr, stderr_str_ptr, &status, &error)) {
+        log_error("Spawning '%s' failed: %s.", cmd, error->message);
+        g_error_free(error);
+        error = NULL;
+        return FALSE;
+    }
+
+    if (!g_spawn_check_exit_status(status, &error)) {
+        log_error("Calling '%s' failed: %s.", cmd, error->message);
+        g_error_free(error);
+        error = NULL;
+        g_free(cmd);
+        cmd = NULL;
+        g_free(stdout_str);
+        stdout_str = NULL;
+        stdout_str_ptr = NULL;
+        if (stderr_str && strlen(stderr_str)) {
+            log_error("Called command returned the following on stderr: %s.", stderr_str);
+        }
+        g_free(stderr_str);
+        stderr_str = NULL;
+        stderr_str_ptr = NULL;
+        return FALSE;
+    }
+
+    g_free(cmd);
+    cmd = NULL;
+
+    if (output_ptr) {
+        *output_ptr = g_strsplit(stdout_str, "\n", 0);
+        g_free(stdout_str);
+        stdout_str = NULL;
+        stdout_str_ptr = NULL;
+    }
+
+    if (error_ptr) {
+        *error_ptr = g_strsplit(stderr_str, "\n", 0);
+        g_free(stderr_str);
+        stderr_str = NULL;
+        stderr_str_ptr = NULL;
+    }
+
+    return TRUE;
 }

src/common.h

@@ -106,6 +106,6 @@ void get_file_paths_recursive(const char *directory, GSList **contents);
 
 char* get_random_string(int length);
 
-void call_external(const char *const exe, const char *const param);
+gboolean call_external(gchar **argv, gchar ***const output_ptr, gchar ***const error_ptr);
 
 #endif

src/config/account.c

@@ -194,32 +194,30 @@ account_eval_password(ProfAccount *account)
     assert(account != NULL);
     assert(account->eval_password != NULL);
 
-    // Evaluate as shell command to retrieve password
-    GString *cmd = g_string_new("");
-    g_string_append_printf(cmd, "%s 2>/dev/null", account->eval_password);
+    gchar **output = NULL;
+    gchar **argv = g_strsplit(account->eval_password, " ", 0);
 
-    FILE *stream = popen(cmd->str, "r");
-    g_string_free(cmd, TRUE);
-    if (stream) {
-        // Limit to READ_BUF_SIZE bytes to prevent overflows in the case of a poorly chosen command
-        account->password = g_malloc(READ_BUF_SIZE);
-        if (!account->password) {
-            log_error("Failed to allocate enough memory to read eval_password output");
-            return FALSE;
-        }
-        account->password = fgets(account->password, READ_BUF_SIZE, stream);
-        pclose(stream);
-        if (!account->password) {
-            log_error("No result from eval_password.");
-            return FALSE;
-        }
+    if (!call_external(argv, &output, NULL)) {
+        g_strfreev(argv);
+        argv = NULL;
+        return FALSE;
+    }
 
-        // strip trailing newline
-        if (g_str_has_suffix(account->password, "\n")) {
-            account->password[strlen(account->password)-1] = '\0';
-        }
-    } else {
-        log_error("popen failed when running eval_password.");
+    g_strfreev(argv);
+
+    if (!output || !output[0]) {
+        log_error("Failed to read eval_password output");
+        g_strfreev(output);
+        output = NULL;
+        return FALSE;
+    }
+
+    account->password = strdup(output[0]);
+    g_strfreev(output);
+    output = NULL;
+
+    if (!account->password) {
+        log_error("Failed to allocate enough memory to read eval_password output");
         return FALSE;
     }
 

src/xmpp/avatar.c

@@ -266,7 +266,10 @@ _avatar_request_item_result_handler(xmpp_stanza_t *const stanza, void *const use
 
     // if we shall open it
     if (g_hash_table_contains(shall_open, from_attr)) {
-        call_external(prefs_get_string(PREF_AVATAR_CMD), filename->str);
+        gchar *argv[] = {prefs_get_string(PREF_AVATAR_CMD), filename->str, NULL};
+        if (!call_external(argv, NULL, NULL)) {
+          cons_show_error("Unable to display avatar: check the logs for more information.");
+        }
         g_hash_table_remove(shall_open, from_attr);
     }