From 32cfea4bd28ff70bae1c2e80c603877b5b35515c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Wennerstr=C3=B6m?= Date: Fri, 11 Dec 2020 15:51:01 +0100 Subject: [PATCH 1/3] Refactor call_external --- src/common.c | 97 +++++++++----------------------------------- src/common.h | 4 +- src/config/account.c | 30 +++++++++----- 3 files changed, 41 insertions(+), 90 deletions(-) diff --git a/src/common.c b/src/common.c index fc1f60ab..3d3b00dd 100644 --- a/src/common.c +++ b/src/common.c @@ -471,92 +471,35 @@ get_mentions(gboolean whole_word, gboolean case_sensitive, const char* const mes return mentions; } -/* - * 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) +call_external(gchar** argv, gchar** std_out, gchar** std_err) { - 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; - - cmd = g_strjoinv(" ", argv); - log_debug("Calling external: %s", cmd); - - if (!output_ptr) { - stdout_str_ptr = NULL; + GSpawnFlags flags = G_SPAWN_SEARCH_PATH | G_SPAWN_CHILD_INHERITS_STDIN; + if (std_out == NULL) flags |= G_SPAWN_STDOUT_TO_DEV_NULL; - } - - if (!error_ptr) { - stderr_str_ptr = NULL; + if (std_err == 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; - } + gint exit_status; + gboolean spawn_result; + GError* spawn_error; + spawn_result = g_spawn_sync(NULL, // Inherit the parent PWD. + argv, + NULL, // Inherit the parent environment. + flags, + NULL, NULL, // No func. before exec() in child. + std_out, std_err, + &exit_status, &spawn_error); - if (!g_spawn_check_exit_status(status, &error)) { - log_error("Calling '%s' failed: %s.", cmd, error->message); - g_error_free(error); - error = NULL; + if (!spawn_result + || !g_spawn_check_exit_status(exit_status, &spawn_error)) { + gchar* cmd = g_strjoinv(" ", argv); + log_error("Spawning '%s' failed with '%s'.", cmd, spawn_error->message); 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_error_free(spawn_error); } - 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; + return spawn_result; } gchar** diff --git a/src/common.h b/src/common.h index 44a61a79..bd33bf90 100644 --- a/src/common.h +++ b/src/common.h @@ -104,10 +104,10 @@ void get_file_paths_recursive(const char* directory, GSList** contents); char* get_random_string(int length); -gboolean call_external(gchar** argv, gchar*** const output_ptr, gchar*** const error_ptr); +gboolean call_external(gchar** argv, gchar** std_out, gchar** std_err); gchar** format_call_external_argv(const char* template, const char* url, const char* filename); gchar* unique_filename_from_url(const char* url, const char* path); -gchar* get_expanded_path(const char *path); +gchar* get_expanded_path(const char* path); #endif diff --git a/src/config/account.c b/src/config/account.c index c0f508b4..7e5297af 100644 --- a/src/config/account.c +++ b/src/config/account.c @@ -200,27 +200,35 @@ account_eval_password(ProfAccount* account) assert(account != NULL); assert(account->eval_password != NULL); - gchar** output = NULL; - gchar** error = NULL; + gchar* std_out = NULL; + gchar* std_err = NULL; gchar* argv[] = { "sh", "-c", account->eval_password, NULL }; - if (!call_external(argv, &output, &error)) { + if (!call_external(argv, &std_out, &std_err)) { + log_error("Password command failed with: %s", std_err); + g_free(std_out); + g_free(std_err); return FALSE; } - if (!output || !output[0]) { - log_error("Failed to read eval_password output"); - g_strfreev(output); - output = NULL; + if (!std_out || !std_out[0]) { + log_error("Password command returned empty output."); + g_free(std_out); + g_free(std_err); return FALSE; } - account->password = strdup(output[0]); - g_strfreev(output); - output = NULL; + // Remove leading and trailing whitespace from command output. + gchar* password = g_strdup(std_out); + g_strstrip(password); + + account->password = password; + g_free(std_out); + g_free(std_err); if (!account->password) { - log_error("Failed to allocate enough memory to read eval_password output"); + log_error("Failed to allocate enough memory to read password command " + "output"); return FALSE; } From 695a1d3c8c22b48fdf55b6d797d8b4b7e43abc27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Wennerstr=C3=B6m?= Date: Sun, 13 Dec 2020 17:19:40 +0100 Subject: [PATCH 2/3] Use popen for eval_password (reintroduce old behavior) Old commit that implemented the old behavior: bc9e6b79cdc246f7e97f6ddff7ea81474a698b05 --- src/config/account.c | 63 +++++++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 24 deletions(-) diff --git a/src/config/account.c b/src/config/account.c index 7e5297af..627c4a1c 100644 --- a/src/config/account.c +++ b/src/config/account.c @@ -36,6 +36,7 @@ #include #include #include +#include #include @@ -200,35 +201,49 @@ account_eval_password(ProfAccount* account) assert(account != NULL); assert(account->eval_password != NULL); - gchar* std_out = NULL; - gchar* std_err = NULL; + errno = 0; - gchar* argv[] = { "sh", "-c", account->eval_password, NULL }; - if (!call_external(argv, &std_out, &std_err)) { - log_error("Password command failed with: %s", std_err); - g_free(std_out); - g_free(std_err); + FILE* stream = popen(account->eval_password, "r"); + if (stream == NULL) { + const char* errmsg = strerror(errno); + if (errmsg) { + log_error("Could not execute `eval_password` command (%s).", + errmsg); + } else { + log_error("Failed to allocate memory for `eval_password` command."); + } return FALSE; } - if (!std_out || !std_out[0]) { - log_error("Password command returned empty output."); - g_free(std_out); - g_free(std_err); - return FALSE; - } - - // Remove leading and trailing whitespace from command output. - gchar* password = g_strdup(std_out); - g_strstrip(password); - - account->password = password; - g_free(std_out); - g_free(std_err); - + account->password = g_malloc(READ_BUF_SIZE); if (!account->password) { - log_error("Failed to allocate enough memory to read password command " - "output"); + log_error("Failed to allocate enough memory to read `eval_password` " + "output."); + return FALSE; + } + + account->password = fgets(account->password, READ_BUF_SIZE, stream); + if (!account->password) { + log_error("Failed to read password from stream."); + return FALSE; + } + + int exit_status = pclose(stream); + if (exit_status > 0) { + log_error("Command for `eval_password` returned error status (%s).", + exit_status); + return FALSE; + } else if (exit_status < 0) { + log_error("Failed to close stream for `eval_password` command output " + "(%s).", + strerror(errno)); + return FALSE; + }; + + // Remove leading and trailing whitespace from output. + g_strstrip(account->password); + if (!account->password) { + log_error("Empty password returned by `eval_password` command."); return FALSE; } From 0437fae1a850ee0bfddb195d56edd505c015310d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?William=20Wennerstr=C3=B6m?= Date: Tue, 15 Dec 2020 15:48:51 +0100 Subject: [PATCH 3/3] Do not inherit stdin of main process I see no reason to inherit the stdin, so let's have it disabled. --- src/common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common.c b/src/common.c index 3d3b00dd..658a3a4e 100644 --- a/src/common.c +++ b/src/common.c @@ -474,7 +474,7 @@ get_mentions(gboolean whole_word, gboolean case_sensitive, const char* const mes gboolean call_external(gchar** argv, gchar** std_out, gchar** std_err) { - GSpawnFlags flags = G_SPAWN_SEARCH_PATH | G_SPAWN_CHILD_INHERITS_STDIN; + GSpawnFlags flags = G_SPAWN_SEARCH_PATH; if (std_out == NULL) flags |= G_SPAWN_STDOUT_TO_DEV_NULL; if (std_err == NULL)