[virt-tools-list] [PATCH virt-viewer] Use socat instead of nc if possible

[Cole Robinson]

I'd really recommend that virt-viewer just use the same shell code that
libvirt and virt-manager have used for years now, it's a crazy hack but it
should be battle hardened by now.

- Cole

On 09/03/2014 08:55 AM, Eric Blake wrote:
> On 09/02/2014 10:19 AM, Marc-André Lureau wrote:
>> It turns out that nc does not leave on server disconnect, and there
>> doesn't seem to be any option to do that, leaving client open, and
>> a bunch of idle processes.
>>
>> Replacing nc with socat solves that, client is disconnected when
>> the VM is shut down, when the sever connection is closed.
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=1030487
>> ---
> 
>> +
>> +    cat = g_string_new("if command -v socat 2>&1 >/dev/null");
> 
> You mixed up your redirections.  This says to clone the original stdout
> to stderr, then change stdout, so that success is silent but error
> messages go to stdout.  But you probably want to redirect stdout to
> /dev/null, then clone _that_ to stderr as well, so that the overall
> command is silent, whether or not it encounters an error.
> 
> 'command -v' is not universally supported (think Solaris /bin/sh).  It
> is good enough for all clients where we expect to be connecting, though,
> so I don't mind using it as written; but if you are ultra-worried about
> it being missing, you have to do the redirections outside of a subshell
> to protect from the potentially-missing builtin, as in:
> 
> if (command -v socat) >/dev/null 2>&1
> 
>> +
>> +    g_string_append(cat, "; then socat - ");
>> +    if (port)
>> +        g_string_append_printf(cat, "TCP:%s:%s", host, port);
>> +    else
>> +        g_string_append_printf(cat, "UNIX-CONNECT:%s", unixsock);
>> +
>> +    g_string_append(cat, "; else nc ");
>> +    if (port)
>> +        g_string_append_printf(cat, "%s %s", host, port);
>> +    else
>> +        g_string_append_printf(cat, "-U %s", unixsock);
> 
> Can host, port, or unixsock contain any shell metacharacters?  If so,
> remember that you are going through TWO levels of shell quoting - one
> because of the shell command, another because of ssh.  But I don't think
> this patch is any worse than the previous code at the potential for bad
> things if a user-provided input contains shell metacharacters.
>