command: shell-quote when logging commands

Without this patch, logged command executions can be ambiguous if
the command contained any shell metacharacters.  This has caused
more than one person to attempt to patch clients to add unnecessary
quoting, without realizing that the command itself was run with
correct args, and only the logged output was ambiguous.

* src/util/command.c (virCommandToString): Add shell escapes.
* tests/commandtest.c (test16): Test new behavior.
* tests/commanddata/test16.log: Update expected output.
* tests/qemuxml2argvdata/qemuxml2argv-*.args: Likewise.
* tests/networkxml2argvdata/*.argv: Likewise.

src/util/command.c

@@ -1614,9 +1614,10 @@ virCommandWriteArgLog(virCommandPtr cmd, int logfd)
  * virCommandToString:
  * @cmd: the command to convert
  *
- * Call after adding all arguments and environment settings, but before
- * Run/RunAsync, to return a string representation of the environment and
- * arguments of cmd.  If virCommandRun cannot succeed (because of an
+ * Call after adding all arguments and environment settings, but
+ * before Run/RunAsync, to return a string representation of the
+ * environment and arguments of cmd, suitably quoted for pasting into
+ * a shell.  If virCommandRun cannot succeed (because of an
  * out-of-memory condition while building cmd), NULL will be returned.
  * Caller is responsible for freeing the resulting string.
  */
@@ -1639,13 +1640,25 @@ virCommandToString(virCommandPtr cmd)
     }
 
     for (i = 0; i < cmd->nenv; i++) {
-        virBufferAdd(&buf, cmd->env[i], strlen(cmd->env[i]));
+        /* In shell, a='b c' has a different meaning than 'a=b c', so
+         * we must determine where the '=' lives.  */
+        char *eq = strchr(cmd->env[i], '=');
+
+        if (!eq) {
+            virBufferFreeAndReset(&buf);
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("invalid use of command API"));
+            return NULL;
+        }
+        eq++;
+        virBufferAdd(&buf, cmd->env[i], eq - cmd->env[i]);
+        virBufferEscapeShell(&buf, eq);
         virBufferAddChar(&buf, ' ');
     }
-    virBufferAdd(&buf, cmd->args[0], strlen(cmd->args[0]));
+    virBufferEscapeShell(&buf, cmd->args[0]);
     for (i = 1; i < cmd->nargs; i++) {
         virBufferAddChar(&buf, ' ');
-        virBufferAdd(&buf, cmd->args[i], strlen(cmd->args[i]));
+        virBufferEscapeShell(&buf, cmd->args[i]);
     }
 
     if (virBufferError(&buf)) {

tests/commanddata/test16.log

@@ -1 +1 @@
-A=B true C
+A=B C=D  E true F G  H

tests/commandtest.c

@@ -607,12 +607,14 @@ static int test16(const void *unused ATTRIBUTE_UNUSED)
 {
     virCommandPtr cmd = virCommandNew("true");
     char *outactual = NULL;
-    const char *outexpect = "A=B true C";
+    const char *outexpect = "A=B C='D  E' true F 'G  H'";
     int ret = -1;
     int fd = -1;
 
     virCommandAddEnvPair(cmd, "A", "B");
-    virCommandAddArg(cmd, "C");
+    virCommandAddEnvPair(cmd, "C", "D  E");
+    virCommandAddArg(cmd, "F");
+    virCommandAddArg(cmd, "G  H");
 
     if ((outactual = virCommandToString(cmd)) == NULL) {
         virErrorPtr err = virGetLastError();

tests/networkxml2argvdata/nat-network-dns-txt-record.argv

@@ -1,6 +1,6 @@
 @DNSMASQ@ --strict-order --bind-interfaces \
 --local=// --domain-needed --filterwin2k --conf-file= \
---except-interface lo --txt-record=example,example value \
+--except-interface lo '--txt-record=example,example value' \
 --listen-address 192.168.122.1 --listen-address 192.168.123.1 \
 --listen-address 2001:db8:ac10:fe01::1 \
 --listen-address 2001:db8:ac10:fd01::1 --listen-address 10.24.10.1 \

tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd-auth.args

@@ -2,9 +2,10 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \
 /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor \
 unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -drive \
 file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive \
-file=rbd:pool/image:\
+'file=rbd:pool/image:\
 id=myname:\
 key=QVFDVm41aE82SHpGQWhBQXEwTkN2OGp0SmNJY0UrSE9CbE1RMUE=:\
 auth_supported=cephx\;none:\
-mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;mon3.example.org\:6322,\
-if=virtio,format=raw -net none -serial none -parallel none -usb
+mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;\
+mon3.example.org\:6322,\
+if=virtio,format=raw' -net none -serial none -parallel none -usb

tests/qemuxml2argvdata/qemuxml2argv-disk-drive-network-rbd.args

@@ -2,6 +2,7 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \
 /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor \
 unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -drive \
 file=/dev/HostVG/QEMUGuest1,if=ide,bus=0,unit=0 -drive \
-file=rbd:pool/image:auth_supported=none:\
-mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;mon3.example.org\:6322,\
-if=virtio,format=raw -net none -serial none -parallel none -usb
+'file=rbd:pool/image:auth_supported=none:\
+mon_host=mon1.example.org\:6321\;mon2.example.org\:6322\;\
+mon3.example.org\:6322,\
+if=virtio,format=raw' -net none -serial none -parallel none -usb

tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc.args

@@ -1,4 +1,4 @@
 LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
 /usr/bin/qemu -S -M pc -m 214 -smp 1 -monitor unix:/tmp/test-monitor,server,\
 nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none -serial none \
--parallel none -usb -vnc [2001:1:2:3:4:5:1234:1234]:3
+-parallel none -usb -vnc '[2001:1:2:3:4:5:1234:1234]:3'

tests/qemuxml2argvdata/qemuxml2argv-qemu-ns.args

@@ -1,4 +1,4 @@
-LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test NS=ns BAR= \
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test NS=ns BAR='' \
 /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor \
 unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda \
 /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb -unknown \

tests/qemuxml2argvdata/qemuxml2argv-smbios.args

@@ -1,7 +1,7 @@
 LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \
-pc -m 214 -smp 1 -smbios type=0,vendor=LENOVO,version=6FET82WW (3.12 ) -smbios \
-type=1,manufacturer=Fedora,product=Virt-Manager,version=0.8.2-3.fc14,\
+pc -m 214 -smp 1 -smbios 'type=0,vendor=LENOVO,version=6FET82WW (3.12 )' \
+-smbios 'type=1,manufacturer=Fedora,product=Virt-Manager,version=0.8.2-3.fc14,\
 serial=32dfcb37-5af1-552b-357c-be8c3aa38310,\
-uuid=c7a5fdbd-edaf-9455-926a-d65c16db1809,sku=1234567890,family=Red Hat \
+uuid=c7a5fdbd-edaf-9455-926a-d65c16db1809,sku=1234567890,family=Red Hat' \
 -nographic -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -hda \
 /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb