This is an automated email from the git hooks/post-receive script. x2go pushed a commit to branch master in repository libx2goclient. commit ee1dca720dde30271b5580e7d08945f035f6097e Author: Mihai Moldovan <ionic@ionic.de> Date: Sat Jun 20 09:51:16 2020 +0200 src/x2goclient-{network-ssh,openssh-version}.{c,h}: split up x2goclient_network_ssh_fetch_openssh_version into same function and x2goclient_openssh_version_fetch_openssh_version which does the actual work. Essentially moves the most code to the other place. It really belongs there. --- src/x2goclient-network-ssh.c | 183 +++------------------------------------ src/x2goclient-network-ssh.h | 2 - src/x2goclient-openssh-version.c | 171 ++++++++++++++++++++++++++++++++++++ src/x2goclient-openssh-version.h | 4 + 4 files changed, 188 insertions(+), 172 deletions(-) diff --git a/src/x2goclient-network-ssh.c b/src/x2goclient-network-ssh.c index e26ad58..c87e433 100644 --- a/src/x2goclient-network-ssh.c +++ b/src/x2goclient-network-ssh.c @@ -979,185 +979,28 @@ static gboolean x2goclient_network_ssh_parent_connect (X2GoClientNetwork *parent return (ret); } -/* FIXME: this code should probably be part of X2GoClientOpenSSHVersion. */ static gboolean x2goclient_network_ssh_fetch_openssh_version (X2GoClientNetworkSSH *self, GError **gerr) { gboolean ret = FALSE; g_return_val_if_fail (X2GOCLIENT_IS_NETWORK_SSH (self), ret); g_return_val_if_fail (((NULL == gerr) || (NULL == *gerr)), ret); - GPtrArray *ssh_cmd = g_ptr_array_new_with_free_func (&x2goclient_clear_strings); - g_ptr_array_add (ssh_cmd, g_strdup ("ssh")); - g_ptr_array_add (ssh_cmd, g_strdup ("-V")); - g_ptr_array_add (ssh_cmd, NULL); - - { - gchar *tmp = NULL; - for (gsize i = 0; i < ssh_cmd->len; ++i) { - gchar *tmp_old = tmp; - - if (tmp) { - tmp = g_strdup_printf ("%s [%s]", tmp_old, (gchar *)g_ptr_array_index (ssh_cmd, i)); - } - else { - tmp = g_strdup_printf ("[%s]", (gchar *)g_ptr_array_index (ssh_cmd, i)); - } - - g_free (tmp_old); - tmp_old = NULL; - } - - g_log (NULL, G_LOG_LEVEL_DEBUG, "Fetching OpenSSH version via: %s", tmp); - - g_free (tmp); - tmp = NULL; - } - - GError *ssh_err = NULL; - GSubprocess *ssh_proc = g_subprocess_newv ((const gchar* const*)(ssh_cmd->pdata), (G_SUBPROCESS_FLAGS_STDOUT_PIPE | G_SUBPROCESS_FLAGS_STDERR_PIPE), &ssh_err); - - ret = (ssh_proc != NULL); - - if (ret) { - g_log (NULL, G_LOG_LEVEL_DEBUG, "OpenSSH version fetching process started/executed successfully!"); - - if (ssh_err) { - g_log (NULL, G_LOG_LEVEL_DEBUG, "Successful execution, but ssh_err set? Weird, here's the message: %s", ssh_err->message); - } - - GCancellable *ssh_proc_comm_cancel = g_cancellable_new (); - g_clear_error (&ssh_err); - - GBytes *ssh_stdout = NULL, *ssh_stderr = NULL; - if (!(g_subprocess_communicate (ssh_proc, NULL, ssh_proc_comm_cancel, &ssh_stdout, &ssh_stderr, &ssh_err))) { - g_propagate_prefixed_error (gerr, ssh_err, "Communication with OpenSSH version fetching subprocess failed: "); - } - else { - gsize ssh_stdout_size = 0, ssh_stderr_size = 0; - const gchar *ssh_stdout_str = g_bytes_get_data (ssh_stdout, &ssh_stdout_size), - *ssh_stderr_str = g_bytes_get_data (ssh_stderr, &ssh_stderr_size); - int ssh_stdout_size_sanitized = 0, ssh_stderr_size_sanitized = 0; - gchar *ssh_stdout_str_sanitized = 0, *ssh_stderr_str_sanitized = NULL; - - /* - * FIXME: something about this section looks odd, revisit. - * Also, we should probably drop everything but the first line - * early on. - */ - /* Sanity check on output size. */ - if (INT_MAX < ssh_stdout_size) { - g_log (NULL, G_LOG_LEVEL_WARNING, "OpenSSH returned more than %d bytes on stdout, this is unusual and will be truncated.", INT_MAX); + X2GoClientOpenSSHVersion *version = x2goclient_openssh_version_fetch_openssh_version (gerr); - ssh_stdout_size_sanitized = INT_MAX; - } - else { - ssh_stdout_size_sanitized = ssh_stdout_size; - } - - if (ssh_stdout_str) { - /* - * Do NOT use g_strndup () here. - * - * It might sound exactly like what we want, but really isn't. - * - * The issue is that g_strndup () will always allocate an n-bytes-sized - * buffer and optionally pad the string with NULL bytes. - * - * That's not really a problem if the string actually is to be - * truncated, since in that case the result will be smaller than the - * original string anyway, but a problem if the size is huge, but the - * string small. - * - * In the latter case, we don't want to create a useless 2 GiB (bounded - * by INT_MAX) string for a string that is actually quite small. - * - * Interestingly, POSIX describes strndup as allocating memory "as if - * by using malloc ()", but doesn't mention the actual resulting size. - * In the informal section, buffer sizes are described as either - * (size + 1) or ((strnlen (s, size)) + 1), which, again, could lead - * to the same problem. - */ - ssh_stdout_str_sanitized = g_strdup_printf ("%.*s", ssh_stdout_size_sanitized, ssh_stdout_str); - } - else { - ssh_stderr_size_sanitized = ssh_stderr_size; - } - - if (INT_MAX < ssh_stderr_size) { - g_log (NULL, G_LOG_LEVEL_WARNING, "OpenSSH returned more than %d bytes on stderr, this is unusual and will be truncated.", INT_MAX); - - ssh_stderr_size_sanitized = INT_MAX; - } - - if (ssh_stderr_str) { - ssh_stderr_str_sanitized = g_strdup_printf ("%.*s", ssh_stderr_size_sanitized, ssh_stderr_str); - } - - g_log (NULL, G_LOG_LEVEL_DEBUG, "Stdout:\n>>>%.*s<<<\nStderr:\n>>>%.*s<<<", ssh_stdout_size_sanitized, ssh_stdout_str_sanitized, ssh_stderr_size_sanitized, ssh_stderr_str_sanitized); - - if (ssh_stdout_size_sanitized) { - g_log (NULL, G_LOG_LEVEL_WARNING, "OpenSSH version command wrote data on stdout, this is unexpected and will be ignored.\nData: %*.s", ssh_stdout_size_sanitized, ssh_stdout_str_sanitized); - } - - if (!(ssh_stderr_size_sanitized)) { - g_set_error_literal (gerr, X2GOCLIENT_NETWORK_SSH_ERROR, X2GOCLIENT_NETWORK_SSH_ERROR_OPENSSH_VERSION_NO_STDERR, "OpenSSH version command wrote nothing to stderr, this is unexpected. Can't parse version string."); - ret = FALSE; - } - else { - X2GoClientOpenSSHVersion *version = x2goclient_openssh_version_new (); - - if (!(version)) { - g_set_error_literal (gerr, X2GOCLIENT_NETWORK_SSH_ERROR, X2GOCLIENT_NETWORK_SSH_ERROR_OPENSSH_VERSION_ALLOCATE, "Unable to allocate buffe for OpenSSH version structure - memory issues?"); - ret = FALSE; - } - else { - /* gerr is supposed to be empty at that point. */ - g_assert ((NULL == gerr) || (NULL == *gerr)); - - ret = x2goclient_openssh_version_parse (version, ssh_stderr_str_sanitized, gerr); - - if (ret) { - /* - * Everything went well, copy to the property. - * - * ... unfortunately, the actual property is read-only (which - * totally makes sense because we only want this class to handle - * it), so we'll have to duplicate a bit of code and can't use the - * GObject property setter directly. - */ - x2goclient_openssh_version_free (self->openssh_version); - self->openssh_version = version; - } - else { - /* Get rid of the version struct. It's bogus anyway. */ - x2goclient_openssh_version_free (version); - } - - version = NULL; - } - } - - g_free (ssh_stdout_str_sanitized); - g_free (ssh_stderr_str_sanitized); - - g_bytes_unref (ssh_stdout); - g_bytes_unref (ssh_stderr); - } + if (version) { + /* + * Everything went well, copy to the property. + * + * ... unfortunately, the actual property is read-only (which + * totally makes sense because we only want this class to handle + * it), so we'll have to duplicate a bit of code and can't use the + * GObject property setter directly. + */ + x2goclient_openssh_version_free (self->openssh_version); + self->openssh_version = version; - g_clear_object (&ssh_proc_comm_cancel); - ssh_proc_comm_cancel = NULL; - } - else { - g_propagate_prefixed_error (gerr, ssh_err, "OpenSSH vesion fetching process didn't execute/start successfully! Error: "); + ret = TRUE; } - g_clear_error (&ssh_err); - - g_clear_object (&ssh_proc); - ssh_proc = NULL; - - g_ptr_array_unref (ssh_cmd); - ssh_cmd = NULL; - return (ret); } diff --git a/src/x2goclient-network-ssh.h b/src/x2goclient-network-ssh.h index b85f49b..80cc10f 100644 --- a/src/x2goclient-network-ssh.h +++ b/src/x2goclient-network-ssh.h @@ -67,8 +67,6 @@ enum { X2GOCLIENT_NETWORK_SSH_ERROR_CONNECT_NATIVE_FETCH, X2GOCLIENT_NETWORK_SSH_ERROR_CONNECT_SOCK_ADDR_UNKNOWN, X2GOCLIENT_NETWORK_SSH_ERROR_CONNECT_OPTIONS_INVALID, - X2GOCLIENT_NETWORK_SSH_ERROR_OPENSSH_VERSION_NO_STDERR, - X2GOCLIENT_NETWORK_SSH_ERROR_OPENSSH_VERSION_ALLOCATE, }; diff --git a/src/x2goclient-openssh-version.c b/src/x2goclient-openssh-version.c index 440e533..fbdabfd 100644 --- a/src/x2goclient-openssh-version.c +++ b/src/x2goclient-openssh-version.c @@ -26,6 +26,7 @@ #include <glib.h> #include <glib/gprintf.h> +#include <gio/gio.h> #ifdef HAVE_CONFIG_H #include "config.h" @@ -366,3 +367,173 @@ _Bool x2goclient_openssh_version_parse (X2GoClientOpenSSHVersion *openssh_versio return (ret); } + +X2GoClientOpenSSHVersion* x2goclient_openssh_version_fetch_openssh_version (GError **gerr) { + X2GoClientOpenSSHVersion *ret = NULL; + gboolean cont = FALSE; + + g_return_val_if_fail (((NULL == gerr) || (NULL == *gerr)), ret); + + GPtrArray *ssh_cmd = g_ptr_array_new_with_free_func (&x2goclient_clear_strings); + g_ptr_array_add (ssh_cmd, g_strdup ("ssh")); + g_ptr_array_add (ssh_cmd, g_strdup ("-V")); + g_ptr_array_add (ssh_cmd, NULL); + + { + gchar *tmp = NULL; + for (gsize i = 0; i < ssh_cmd->len; ++i) { + gchar *tmp_old = tmp; + + if (tmp) { + tmp = g_strdup_printf ("%s [%s]", tmp_old, (gchar *)g_ptr_array_index (ssh_cmd, i)); + } + else { + tmp = g_strdup_printf ("[%s]", (gchar *)g_ptr_array_index (ssh_cmd, i)); + } + + g_free (tmp_old); + tmp_old = NULL; + } + + g_log (NULL, G_LOG_LEVEL_DEBUG, "Fetching OpenSSH version via: %s", tmp); + + g_free (tmp); + tmp = NULL; + } + + GError *ssh_err = NULL; + GSubprocess *ssh_proc = g_subprocess_newv ((const gchar* const*)(ssh_cmd->pdata), (G_SUBPROCESS_FLAGS_STDOUT_PIPE | G_SUBPROCESS_FLAGS_STDERR_PIPE), &ssh_err); + + cont = (ssh_proc != NULL); + + if (cont) { + g_log (NULL, G_LOG_LEVEL_DEBUG, "OpenSSH version fetching process started/executed successfully!"); + + if (ssh_err) { + g_log (NULL, G_LOG_LEVEL_DEBUG, "Successful execution, but ssh_err set? Weird, here's the message: %s", ssh_err->message); + } + + GCancellable *ssh_proc_comm_cancel = g_cancellable_new (); + g_clear_error (&ssh_err); + + GBytes *ssh_stdout = NULL, *ssh_stderr = NULL; + if (!(g_subprocess_communicate (ssh_proc, NULL, ssh_proc_comm_cancel, &ssh_stdout, &ssh_stderr, &ssh_err))) { + g_propagate_prefixed_error (gerr, ssh_err, "Communication with OpenSSH version fetching subprocess failed: "); + } + else { + gsize ssh_stdout_size = 0, ssh_stderr_size = 0; + const gchar *ssh_stdout_str = g_bytes_get_data (ssh_stdout, &ssh_stdout_size), + *ssh_stderr_str = g_bytes_get_data (ssh_stderr, &ssh_stderr_size); + int ssh_stdout_size_sanitized = 0, ssh_stderr_size_sanitized = 0; + gchar *ssh_stdout_str_sanitized = 0, *ssh_stderr_str_sanitized = NULL; + + /* + * FIXME: something about this section looks odd, revisit. + * Also, we should probably drop everything but the first line + * early on. + */ + /* Sanity check on output size. */ + if (INT_MAX < ssh_stdout_size) { + g_log (NULL, G_LOG_LEVEL_WARNING, "OpenSSH returned more than %d bytes on stdout, this is unusual and will be truncated.", INT_MAX); + + ssh_stdout_size_sanitized = INT_MAX; + } + else { + ssh_stdout_size_sanitized = ssh_stdout_size; + } + + if (ssh_stdout_str) { + /* + * Do NOT use g_strndup () here. + * + * It might sound exactly like what we want, but really isn't. + * + * The issue is that g_strndup () will always allocate an n-bytes-sized + * buffer and optionally pad the string with NULL bytes. + * + * That's not really a problem if the string actually is to be + * truncated, since in that case the result will be smaller than the + * original string anyway, but a problem if the size is huge, but the + * string small. + * + * In the latter case, we don't want to create a useless 2 GiB (bounded + * by INT_MAX) string for a string that is actually quite small. + * + * Interestingly, POSIX describes strndup as allocating memory "as if + * by using malloc ()", but doesn't mention the actual resulting size. + * In the informal section, buffer sizes are described as either + * (size + 1) or ((strnlen (s, size)) + 1), which, again, could lead + * to the same problem. + */ + ssh_stdout_str_sanitized = g_strdup_printf ("%.*s", ssh_stdout_size_sanitized, ssh_stdout_str); + } + else { + ssh_stderr_size_sanitized = ssh_stderr_size; + } + + if (INT_MAX < ssh_stderr_size) { + g_log (NULL, G_LOG_LEVEL_WARNING, "OpenSSH returned more than %d bytes on stderr, this is unusual and will be truncated.", INT_MAX); + + ssh_stderr_size_sanitized = INT_MAX; + } + + if (ssh_stderr_str) { + ssh_stderr_str_sanitized = g_strdup_printf ("%.*s", ssh_stderr_size_sanitized, ssh_stderr_str); + } + + g_log (NULL, G_LOG_LEVEL_DEBUG, "Stdout:\n>>>%.*s<<<\nStderr:\n>>>%.*s<<<", ssh_stdout_size_sanitized, ssh_stdout_str_sanitized, ssh_stderr_size_sanitized, ssh_stderr_str_sanitized); + + if (ssh_stdout_size_sanitized) { + g_log (NULL, G_LOG_LEVEL_WARNING, "OpenSSH version command wrote data on stdout, this is unexpected and will be ignored.\nData: %*.s", ssh_stdout_size_sanitized, ssh_stdout_str_sanitized); + } + + if (!(ssh_stderr_size_sanitized)) { + g_set_error_literal (gerr, X2GOCLIENT_OPENSSH_VERSION_ERROR, X2GOCLIENT_OPENSSH_VERSION_ERROR_OPENSSH_VERSION_NO_STDERR, "OpenSSH version command wrote nothing to stderr, this is unexpected. Can't parse version string."); + cont = FALSE; + } + else { + ret = x2goclient_openssh_version_new (); + + if (!(ret)) { + g_set_error_literal (gerr, X2GOCLIENT_OPENSSH_VERSION_ERROR, X2GOCLIENT_OPENSSH_VERSION_ERROR_OPENSSH_VERSION_ALLOCATE, "Unable to allocate buffer for OpenSSH version structure - memory issues?"); + cont = FALSE; + } + else { + /* gerr is supposed to be empty at that point. */ + g_assert ((NULL == gerr) || (NULL == *gerr)); + + cont = x2goclient_openssh_version_parse (ret, ssh_stderr_str_sanitized, gerr); + + if (!(cont)) { + /* Get rid of the version struct. It's bogus anyway. */ + x2goclient_openssh_version_free (ret); + + ret = NULL; + } + } + } + + g_free (ssh_stdout_str_sanitized); + g_free (ssh_stderr_str_sanitized); + + g_bytes_unref (ssh_stdout); + g_bytes_unref (ssh_stderr); + } + + g_clear_object (&ssh_proc_comm_cancel); + ssh_proc_comm_cancel = NULL; + } + else { + g_propagate_prefixed_error (gerr, ssh_err, "OpenSSH vesion fetching process didn't execute/start successfully! Error: "); + } + + g_clear_error (&ssh_err); + + g_clear_object (&ssh_proc); + ssh_proc = NULL; + + g_ptr_array_unref (ssh_cmd); + ssh_cmd = NULL; + + return (ret); +} diff --git a/src/x2goclient-openssh-version.h b/src/x2goclient-openssh-version.h index 0df8ada..42b1273 100644 --- a/src/x2goclient-openssh-version.h +++ b/src/x2goclient-openssh-version.h @@ -75,10 +75,14 @@ enum { X2GOCLIENT_OPENSSH_VERSION_ERROR_PATCH_OVERFLOW, X2GOCLIENT_OPENSSH_VERSION_ERROR_PATCH_TOO_BIG, X2GOCLIENT_OPENSSH_VERSION_ERROR_PATCH_NEGATIVE, + X2GOCLIENT_OPENSSH_VERSION_ERROR_OPENSSH_VERSION_NO_STDERR, + X2GOCLIENT_OPENSSH_VERSION_ERROR_OPENSSH_VERSION_ALLOCATE, }; _Bool x2goclient_openssh_version_parse (X2GoClientOpenSSHVersion *openssh_version, const gchar * const version_string, GError **gerr); +X2GoClientOpenSSHVersion* x2goclient_openssh_version_fetch_openssh_version (GError **gerr); + G_END_DECLS -- Alioth's /home/x2go-admin/maintenancescripts/git/hooks/post-receive-email on /srv/git/code.x2go.org/libx2goclient.git