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

This is an automated email from the git hooks/post-receive script. x2go pushed a commit to branch master in repository libx2goclient. commit 3d73ff5d0ed48d28dde5a0824b893f765dcf1dbf Author: Mihai Moldovan <ionic@ionic.de> Date: Fri Jun 26 12:12:14 2020 +0200 src/x2goclient-network.c: fix socket-spec property. It's a boxed type, not a object, so handle as that. --- src/x2goclient-network.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/x2goclient-network.c b/src/x2goclient-network.c index c416a87..0dba284 100644 --- a/src/x2goclient-network.c +++ b/src/x2goclient-network.c @@ -188,9 +188,10 @@ static void x2goclient_network_set_property (GObject *object, guint prop_id, con switch (prop_id) { case (X2GO_NET_PROP_SOCKET_SPEC): - g_free (priv->socket_spec); - g_clear_object (&(priv->socket)); - priv->socket_spec = g_value_dup_object (value); + if (priv->socket_spec) { + g_boxed_free (G_TYPE_GSTRING, priv->socket_spec); + } + priv->socket_spec = g_value_dup_boxed (value); priv->socket = x2goclient_network_parse_sockspec (self, priv->socket_spec); break; case (X2GO_NET_PROP_SESSION_PATH): -- Alioth's /home/x2go-admin/maintenancescripts/git/hooks/post-receive-email on /srv/git/code.x2go.org/libx2goclient.git

This is an automated email from the git hooks/post-receive script. x2go pushed a commit to branch master in repository libx2goclient. commit d881f3633c951d0baf01fa7a84c018a3019349a2 Author: Mihai Moldovan <ionic@ionic.de> Date: Fri Jun 26 12:16:37 2020 +0200 src/x2goclient-network.c: replace g_string_free () with g_boxed_free (). It's probably better to use the more generic function, although we'll have to specify the type explicitly in that case. --- src/x2goclient-network.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/x2goclient-network.c b/src/x2goclient-network.c index 0dba284..a1a4a70 100644 --- a/src/x2goclient-network.c +++ b/src/x2goclient-network.c @@ -118,7 +118,7 @@ static void x2goclient_network_class_init (X2GoClientNetworkClass *klass) { "land there."), default_session_path->str, G_PARAM_CONSTRUCT_ONLY | G_PARAM_READWRITE); - g_string_free (default_session_path, TRUE); + g_boxed_free (G_TYPE_GSTRING, default_session_path); default_session_path = NULL; g_object_class_install_properties (object_class, X2GO_NET_N_PROPERTIES, net_obj_properties); @@ -145,7 +145,7 @@ static void x2goclient_network_finalize (GObject *object) { X2GoClientNetworkPrivate *priv = x2goclient_network_get_instance_private (X2GOCLIENT_NETWORK (object)); if (priv->socket_spec) { - g_string_free (priv->socket_spec, TRUE); + g_boxed_free (G_TYPE_GSTRING, priv->socket_spec); priv->socket_spec = NULL; } g_free (priv->session_path); -- Alioth's /home/x2go-admin/maintenancescripts/git/hooks/post-receive-email on /srv/git/code.x2go.org/libx2goclient.git

This is an automated email from the git hooks/post-receive script. x2go pushed a commit to branch master in repository libx2goclient. commit b74bd64a3434da8d9954baad0eac09ac9ce063cd Author: Mihai Moldovan <ionic@ionic.de> Date: Sat Jun 27 14:40:49 2020 +0200 src/x2goclient-openssh-version.c: rework std{out,err} size checking. Use loops instead of copy-pasting the same code twice, since we really need to do (almost) exactly the same thing twice. Truncate both streams on newlines. We're not interested in additional lines. Keep the (hopefully) correct size truncation around. --- src/x2goclient-openssh-version.c | 216 ++++++++++++++++++++++++++++----------- 1 file changed, 155 insertions(+), 61 deletions(-) diff --git a/src/x2goclient-openssh-version.c b/src/x2goclient-openssh-version.c index 0b10ab6..f169428 100644 --- a/src/x2goclient-openssh-version.c +++ b/src/x2goclient-openssh-version.c @@ -421,73 +421,165 @@ X2GoClientOpenSSHVersion* x2goclient_openssh_version_fetch_openssh_version (GErr 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; + /* First element is stdout, second one stderr. */ + gsize ssh_std_sizes[2] = { 0 }; + const gchar *ssh_std_strs[2] = { g_bytes_get_data (ssh_stdout, ssh_std_sizes), + g_bytes_get_data (ssh_stderr, (ssh_std_sizes + 1)) }; + int ssh_std_sizes_sanitized[2] = { 0 }; + gchar *ssh_std_strs_sanitized[2] = { NULL }; + + for (size_t i = 0; i < (sizeof (ssh_std_strs) / sizeof (*ssh_std_strs)); ++i) { + const gchar *comp = ""; + switch (i) { + case (0): + comp = "stdout"; + break; + case (1): + comp = "stderr"; + break; + default: + comp = "unknown"; + break; + } - /* - * 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); + if (ssh_std_strs[i]) { + /* + * Truncate on first newline character. + * + * Yes, we could, theoretically, just replace the first occurrence of + * the newline character with zero and continue with the truncated + * result, but we'd potentially carry a lot of other useless data + * around. + * + * Better clean it up and free up a bit of memory. + * + * N.B.: The cast here looks weird and it's arguably ugly, but it's + * actually safe. For historical reasons, free () (and + * wrappers) have been declared as taking a non-const pointer + * to data. Whether this is correct or not, and especially + * whether this should have been a const pointer from the + * beginning rather than a non-const one is still actively + * debated. Some people (like Linus Torvalds[0]) argue that + * glibc is broken for several reasons: + * - free () does not actually change any data but + * invalidates the pointer itself (and, by extension, any + * data it is pointing to, but, again, crucially is NOT + * modifying this data) + * - types should be as tight as possible to avoid the need + * of casts + * - the "const" keyword does not mean "constant data" (as + * often wrongly assumed), but actually that it should be + * impossible to modify the data via this very pointer, + * i.e., that it is a pointer property, not a data + * property. + * Then, there's this other camp of people that argue that + * constifying something is a *contract* not to change it + * (usually not even via a different pointer) and that free () + * CAN and actually WILL change the memory pointed to. The + * first argument is a "gut feeling" and not backed by any + * standard (and, IMHO, bogus), but the second argument can + * surprisingly be true. The way memory is allocated and + * accounted for is application- (or system-) defined and this + * opens the door to *a lot* of weird things. For instance, + * the system could track allocated memory (or, e.g., + * statistics) by using data after the allocated chunk of + * memory returned to the application - using the pointer to + * access and, crucially, modify this private data. This + * freedom would naturally not be possible if the input + * pointer was const. + * So much for legacy. + * But why is it safe HERE? + * The reason is quite simple: we *know* that the called + * function WILL NOT modify the input string, not even call + * free () on it, if we explicitly disable that feature via + * its boolean parameter. We could make the input string + * parameter const instead, but this would require a cast when + * calling g_free (), which seems to be frowned upon. Thus, + * we'll keep it taking a non-const pointer for the time being + * and cast data passed to it if necessary. + * + * [0] https://web.archive.org/https://yarchive.net/comp/const.html + */ + ssh_std_strs_sanitized[i] = x2goclient_strbrk_dup ((gchar **)(ssh_std_strs + i), + '\n', FALSE, + (ssh_std_sizes + i)); + + /* + * Next, truncate for size. + * + * We know that the original data is valid. + * + * The sanitized array entry might contain valid data if we already + * truncated based on a newline previously. + * + * Regardless, the original size entries will always contain our + * desired data. + */ + if (INT_MAX < ssh_std_sizes[i]) { + g_log (NULL, G_LOG_LEVEL_WARNING, "OpenSSH returned more than %d bytes on %s, this is unusual and will be truncated.", + INT_MAX, comp); + + ssh_std_sizes_sanitized[i] = INT_MAX; + } + else { + ssh_std_sizes_sanitized[i] = ssh_std_sizes[i]; + } - ssh_stdout_size_sanitized = INT_MAX; - } - else { - ssh_stdout_size_sanitized = ssh_stdout_size; - } + /* + * 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. + */ + if (ssh_std_strs_sanitized[i]) { + gchar *tmp = g_strdup_printf ("%.*s", ssh_std_sizes_sanitized[i], ssh_std_strs_sanitized[i]); + + /* Remember to not leak out old data! */ + g_free (ssh_std_strs_sanitized[i]); + + ssh_std_strs_sanitized[i] = tmp; + } + else { + /* We'll need to copy, so no leak here. */ + ssh_std_strs_sanitized[i] = g_strdup_printf ("%.*s", ssh_std_sizes_sanitized[i], ssh_std_strs[i]); + } + + g_log (NULL, G_LOG_LEVEL_DEBUG, "%s:\n>>>%.*s<<<", comp, ssh_std_sizes_sanitized[i], ssh_std_strs_sanitized[i]); + } + else { + g_log (NULL, G_LOG_LEVEL_DEBUG, "%s: no data", comp); + } - 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. + * Add additional newline unless we're at the end to hopefully make + * output more readable. */ - 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); + if (((sizeof (ssh_std_strs) / sizeof (*ssh_std_strs)) - 1) != i) { + g_log (NULL, G_LOG_LEVEL_DEBUG, ""); + } } - 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_std_sizes_sanitized[0]) { + g_log (NULL, G_LOG_LEVEL_WARNING, "OpenSSH version command wrote data on stdout, this is unexpected and will be ignored.\nData: %*.s", ssh_std_sizes_sanitized[0], ssh_std_strs_sanitized[0]); } - if (!(ssh_stderr_size_sanitized)) { + if (!(ssh_std_sizes_sanitized[1])) { 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."); } else { @@ -500,7 +592,7 @@ X2GoClientOpenSSHVersion* x2goclient_openssh_version_fetch_openssh_version (GErr /* 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); + cont = x2goclient_openssh_version_parse (ret, ssh_std_strs_sanitized[1], gerr); if (!(cont)) { /* Get rid of the version struct. It's bogus anyway. */ @@ -511,8 +603,10 @@ X2GoClientOpenSSHVersion* x2goclient_openssh_version_fetch_openssh_version (GErr } } - g_free (ssh_stdout_str_sanitized); - g_free (ssh_stderr_str_sanitized); + for (size_t i = 0; i < (sizeof (ssh_std_strs_sanitized) / sizeof (*ssh_std_strs_sanitized)); ++i) { + g_free (ssh_std_strs_sanitized[i]); + ssh_std_strs_sanitized[i] = NULL; + } g_bytes_unref (ssh_stdout); ssh_stdout = NULL; -- Alioth's /home/x2go-admin/maintenancescripts/git/hooks/post-receive-email on /srv/git/code.x2go.org/libx2goclient.git

This is an automated email from the git hooks/post-receive script. x2go pushed a commit to branch master in repository libx2goclient. commit 28833e9c5cf923897b661cf6eb3b7a3f71052cda Author: Mihai Moldovan <ionic@ionic.de> Date: Fri Jun 26 14:08:49 2020 +0200 src/x2goclient-network-ssh.c: update bugs stucture in connect function. --- src/x2goclient-network-ssh.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/x2goclient-network-ssh.c b/src/x2goclient-network-ssh.c index eb30163..5405ddc 100644 --- a/src/x2goclient-network-ssh.c +++ b/src/x2goclient-network-ssh.c @@ -877,6 +877,11 @@ static gboolean x2goclient_network_ssh_parent_connect (X2GoClientNetwork *parent /* Got version. */ g_log (NULL, G_LOG_LEVEL_INFO, "Fetched OpenSSH version: [ Major: %d, Minor: %d, Patch: %d, Additional Data: '%s' ]", self->openssh_version->major, self->openssh_version->minor, self->openssh_version->patch, self->openssh_version->addon); + /* Update/populate bugs structure. */ + ret = x2goclient_openssh_bugs_update (self->openssh_bugs, self->openssh_version, gerr); + } + + if (ret) { /* Fetch options object. */ X2GoClientNetworkOptionsSSH *options = NULL; g_object_get (G_OBJECT (self), "options", &options, NULL); -- Alioth's /home/x2go-admin/maintenancescripts/git/hooks/post-receive-email on /srv/git/code.x2go.org/libx2goclient.git

This is an automated email from the git hooks/post-receive script. x2go pushed a commit to branch master in repository libx2goclient. commit 06878dac3b3e3504c59497a6a4a7e7b858f782ba Author: Mihai Moldovan <ionic@ionic.de> Date: Fri Jun 26 13:49:30 2020 +0200 src/x2goclient-openssh-bugs.h: fix function name: x2goclient_network_update_bugs () -> x2goclient_openssh_bugs_update (). --- src/x2goclient-openssh-bugs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/x2goclient-openssh-bugs.h b/src/x2goclient-openssh-bugs.h index 9714c5a..d3b25d1 100644 --- a/src/x2goclient-openssh-bugs.h +++ b/src/x2goclient-openssh-bugs.h @@ -62,7 +62,7 @@ X2GoClientOpenSSHBugs* x2goclient_openssh_bugs_new (void); void x2goclient_openssh_bugs_free (X2GoClientOpenSSHBugs *openssh_bugs); -gboolean x2goclient_network_bugs_update (X2GoClientOpenSSHBugs *self, const X2GoClientOpenSSHVersion * const version, GError **gerr); +gboolean x2goclient_openssh_bugs_update (X2GoClientOpenSSHBugs *self, const X2GoClientOpenSSHVersion * const version, GError **gerr); /* -- Alioth's /home/x2go-admin/maintenancescripts/git/hooks/post-receive-email on /srv/git/code.x2go.org/libx2goclient.git

This is an automated email from the git hooks/post-receive script. x2go pushed a commit to branch master in repository libx2goclient. commit 37e0a13d2f11407c14807f5f98c3f73b72f57fc2 Author: Mihai Moldovan <ionic@ionic.de> Date: Fri Jun 26 13:02:30 2020 +0200 src/x2goclient-network{,-ssh}.c: add G_PARAM_STATIC_STRINGS, they are all static anyway. --- src/x2goclient-network-ssh.c | 4 ++-- src/x2goclient-network.c | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/x2goclient-network-ssh.c b/src/x2goclient-network-ssh.c index 80d523f..eb30163 100644 --- a/src/x2goclient-network-ssh.c +++ b/src/x2goclient-network-ssh.c @@ -146,13 +146,13 @@ static void x2goclient_network_ssh_class_init (X2GoClientNetworkSSHClass *klass) _("The OpenSSH client version number, parsed in a custom " "structure."), X2GOCLIENT_TYPE_OPENSSH_VERSION, - G_PARAM_READABLE); + G_PARAM_STATIC_STRINGS | G_PARAM_READABLE); net_ssh_obj_properties[X2GO_NET_SSH_PROP_OPENSSH_BUGS] = g_param_spec_boxed ("openssh-bugs", _("Bug quirks structure for OpenSSH Client"), _("A structure containing quirks bits for known OpenSSH Client " "bugs."), X2GOCLIENT_TYPE_OPENSSH_BUGS, - G_PARAM_READABLE); + G_PARAM_STATIC_STRINGS | G_PARAM_READABLE); g_object_class_install_properties (object_class, X2GO_NET_SSH_N_PROPERTIES, net_ssh_obj_properties); diff --git a/src/x2goclient-network.c b/src/x2goclient-network.c index a1a4a70..36b27bc 100644 --- a/src/x2goclient-network.c +++ b/src/x2goclient-network.c @@ -97,18 +97,18 @@ static void x2goclient_network_class_init (X2GoClientNetworkClass *klass) { _("String specification for the low-level socket network " "connection. Updates the \"socket\" property."), G_TYPE_GSTRING, - G_PARAM_WRITABLE); + G_PARAM_STATIC_STRINGS | G_PARAM_WRITABLE); net_obj_properties[X2GO_NET_PROP_SOCKET] = g_param_spec_object ("socket", _("Low-level socket"), _("Low-level socket for the network connection, the local " "part will be used for local binding if specified."), G_TYPE_SOCKET_ADDRESS, - G_PARAM_READABLE); + G_PARAM_STATIC_STRINGS | G_PARAM_READABLE); net_obj_properties[X2GO_NET_PROP_OPTIONS] = g_param_spec_object ("options", _("Socket options"), _("Specific socket options."), X2GOCLIENT_TYPE_NETWORK_OPTIONS, - G_PARAM_READWRITE); + G_PARAM_STATIC_STRINGS | G_PARAM_READWRITE); GString *default_session_path = g_string_new (g_get_home_dir ()); g_string_append (default_session_path, "/.libx2goclient/"); @@ -117,7 +117,7 @@ static void x2goclient_network_class_init (X2GoClientNetworkClass *klass) { "session data base path. SSH-related files will " "land there."), default_session_path->str, - G_PARAM_CONSTRUCT_ONLY | G_PARAM_READWRITE); + G_PARAM_STATIC_STRINGS | G_PARAM_CONSTRUCT_ONLY | G_PARAM_READWRITE); g_boxed_free (G_TYPE_GSTRING, default_session_path); default_session_path = NULL; -- Alioth's /home/x2go-admin/maintenancescripts/git/hooks/post-receive-email on /srv/git/code.x2go.org/libx2goclient.git