On Wed, Oct 12, 2011 at 05:22:02PM -0600, Eric Blake wrote: > On 10/12/2011 04:40 PM, Guido Günther wrote: > >to escape the netcat command since it's passed to the shell. Adjust > >expected test case output accordingly. > >--- > > src/rpc/virnetsocket.c | 25 ++++++++++++++++++++----- > > tests/virnetsockettest.c | 10 +++++----- > > 2 files changed, 25 insertions(+), 10 deletions(-) > > ACK with nits fixed. Still to go - it would be nice to follow up > with a 4/3 that converts virsh.c cmdEcho to use > virBufferEscapeShell. > > >@@ -636,6 +639,19 @@ int virNetSocketNewConnectSSH(const char *nodename, > > virCommandAddArgList(cmd, "-o", "StrictHostKeyChecking=no", NULL); > > > > virCommandAddArgList(cmd, nodename, "sh", "-c", NULL); > >+ > >+ virBufferEscapeShell(&buf, netcat ? netcat : "nc"); > >+ if (virBufferError(&buf)) { > >+ virBufferFreeAndReset(&buf); > >+ virReportOOMError(); > >+ return -1; > >+ } > >+ quoted = virBufferContentAndReset(&buf); > >+ if (quoted == NULL) { > > You are guaranteed that quoted is not NULL, by virtue of the fact > that you already filtered for virBufferError just beforehand. Drop > this if statement. Done. > > >+++ b/tests/virnetsockettest.c > >@@ -496,7 +496,7 @@ mymain(void) > > struct testSSHData sshData1 = { > > .nodename = "somehost", > > .path = "/tmp/socket", > >- .expectOut = "somehost sh -c 'if nc -q 2>&1 | grep \"requires an argument\">/dev/null 2>&1; then ARG=-q0;fi;nc $ARG -U /tmp/socket'\n", > >+ .expectOut = "somehost sh -c 'if 'nc' -q 2>&1 | grep \"requires an argument\">/dev/null 2>&1; then ARG=-q0;fi;'nc' $ARG -U /tmp/socket'\n", > > }; > > Rebase this on top of my comments for 1/3 (no long lines). Also, > add one more test where $NC is something that actually proves things > work with shell meta-characters, perhaps by passing 'nc -4' as the > value that eventually supplies the %s for $NC, so that .expectOut > will include: sh -c 'if ''nc -4'' -q 2>&1 .... Done too. Cheers, -- Guido > > -- > Eric Blake eblake@xxxxxxxxxx +1-801-349-2682 > Libvirt virtualization library http://libvirt.org >
>From 48650440f98ea4f975d71f97c5c050a1a4653690 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guido=20G=C3=BCnther?= <agx@xxxxxxxxxxx> Date: Thu, 13 Oct 2011 21:49:01 +0200 Subject: [PATCH 3/4] Use virBufferEscapeShell in virNetSocketNewConnectSSH to escape the netcat command since it's passed to the shell. Adjust expected test case output accordingly. --- src/rpc/virnetsocket.c | 18 +++++++++++++++--- tests/virnetsockettest.c | 34 ++++++++++++++++++++++++---------- 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 2a9bca0..e4eff49 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -612,7 +612,10 @@ int virNetSocketNewConnectSSH(const char *nodename, const char *path, virNetSocketPtr *retsock) { + char *quoted; virCommandPtr cmd; + virBuffer buf = VIR_BUFFER_INITIALIZER; + *retsock = NULL; cmd = virCommandNew(binary ? binary : "ssh"); @@ -639,6 +642,14 @@ int virNetSocketNewConnectSSH(const char *nodename, netcat = "nc"; virCommandAddArgList(cmd, nodename, "sh", "-c", NULL); + + virBufferEscapeShell(&buf, netcat); + if (virBufferError(&buf)) { + virBufferFreeAndReset(&buf); + virReportOOMError(); + return -1; + } + quoted = virBufferContentAndReset(&buf); /* * This ugly thing is a shell script to detect availability of * the -q option for 'nc': debian and suse based distros need this @@ -650,14 +661,15 @@ int virNetSocketNewConnectSSH(const char *nodename, * behavior. */ virCommandAddArgFormat(cmd, - "'if %s -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then " + "'if '%s' -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then " "ARG=-q0;" "else " "ARG=;" "fi;" - "%s $ARG -U %s'", - netcat, netcat, path); + "'%s' $ARG -U %s'", + quoted, quoted, path); + VIR_FREE(quoted); return virNetSocketNewConnectCommand(cmd, retsock); } diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index 75cc9c0..6320ce0 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -496,12 +496,12 @@ mymain(void) struct testSSHData sshData1 = { .nodename = "somehost", .path = "/tmp/socket", - .expectOut = "somehost sh -c 'if nc -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then " + .expectOut = "somehost sh -c 'if 'nc' -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then " "ARG=-q0;" "else " "ARG=;" "fi;" - "nc $ARG -U /tmp/socket'\n", + "'nc' $ARG -U /tmp/socket'\n", }; if (virtTestRun("SSH test 1", 1, testSocketSSH, &sshData1) < 0) ret = -1; @@ -515,12 +515,12 @@ mymain(void) .noVerify = false, .path = "/tmp/socket", .expectOut = "-p 9000 -l fred -T -o BatchMode=yes -e none somehost sh -c '" - "if netcat -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then " + "if 'netcat' -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then " "ARG=-q0;" "else " "ARG=;" "fi;" - "netcat $ARG -U /tmp/socket'\n", + "'netcat' $ARG -U /tmp/socket'\n", }; if (virtTestRun("SSH test 2", 1, testSocketSSH, &sshData2) < 0) ret = -1; @@ -534,12 +534,12 @@ mymain(void) .noVerify = true, .path = "/tmp/socket", .expectOut = "-p 9000 -l fred -o StrictHostKeyChecking=no somehost sh -c '" - "if netcat -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then " + "if 'netcat' -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then " "ARG=-q0;" "else " "ARG=;" "fi;" - "netcat $ARG -U /tmp/socket'\n", + "'netcat' $ARG -U /tmp/socket'\n", }; if (virtTestRun("SSH test 3", 1, testSocketSSH, &sshData3) < 0) ret = -1; @@ -556,12 +556,12 @@ mymain(void) .nodename = "crashyhost", .path = "/tmp/socket", .expectOut = "crashyhost sh -c " - "'if nc -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then " + "'if 'nc' -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then " "ARG=-q0;" "else " "ARG=;" "fi;" - "nc $ARG -U /tmp/socket'\n", + "'nc' $ARG -U /tmp/socket'\n", .dieEarly = true, }; if (virtTestRun("SSH test 5", 1, testSocketSSH, &sshData5) < 0) @@ -573,16 +573,30 @@ mymain(void) .keyfile = "/root/.ssh/example_key", .noVerify = true, .expectOut = "-i /root/.ssh/example_key -o StrictHostKeyChecking=no example.com sh -c '" - "if nc -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then " + "if 'nc' -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then " "ARG=-q0;" "else " "ARG=;" "fi;" - "nc $ARG -U /tmp/socket'\n", + "'nc' $ARG -U /tmp/socket'\n", }; if (virtTestRun("SSH test 6", 1, testSocketSSH, &sshData6) < 0) ret = -1; + struct testSSHData sshData7 = { + .nodename = "somehost", + .netcat = "nc -4", + .path = "/tmp/socket", + .expectOut = "somehost sh -c 'if ''nc -4'' -q 2>&1 | grep \"requires an argument\" >/dev/null 2>&1; then " + "ARG=-q0;" + "else " + "ARG=;" + "fi;" + "''nc -4'' $ARG -U /tmp/socket'\n", + }; + if (virtTestRun("SSH test 7", 1, testSocketSSH, &sshData7) < 0) + ret = -1; + #endif return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); -- 1.7.6.3
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list