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