[Libguestfs] [PATCH] guestfish -i and virt-inspector work on filenames containing spaces (RHBZ#507810).
Matthew Booth
mbooth at redhat.com
Thu May 13 14:17:56 UTC 2010
On 12/05/10 16:01, Richard W.M. Jones wrote:
>>From fb8ae842ef1195fe3896e3f83a97762d27364abb Mon Sep 17 00:00:00 2001
> From: Richard Jones <rjones at redhat.com>
> Date: Wed, 12 May 2010 15:58:00 +0100
> Subject: [PATCH] guestfish -i and virt-inspector work on filenames containing spaces (RHBZ#507810).
>
> This commit fixes a long-standing bug which prevented guestfish -i
> and virt-inspector from working on disk images which had a space
> in the filename (or other unsafe characters). It works by ensuring
> that the strings passed between guestfish -i and virt-inspector are
> quoted correctly in both directions.
>
> Note that this commit adds a dependency from virt-inspector to
> the perl module String::ShellQuote. We have previously used this
> module in virt-make-fs.
> ---
> fish/fish.c | 111 ++++++++++++++++++++++++++++++++++++----------
> inspector/virt-inspector | 9 +++-
> 2 files changed, 94 insertions(+), 26 deletions(-)
>
> diff --git a/fish/fish.c b/fish/fish.c
> index 75bc2cf..34188d4 100644
> --- a/fish/fish.c
> +++ b/fish/fish.c
> @@ -1,5 +1,5 @@
> /* guestfish - the filesystem interactive shell
> - * Copyright (C) 2009 Red Hat Inc.
> + * Copyright (C) 2009-2010 Red Hat Inc.
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> @@ -66,6 +66,7 @@ static void cmdline (char *argv[], int optind, int argc);
> static void initialize_readline (void);
> static void cleanup_readline (void);
> static void add_history_line (const char *);
> +static int print_shell_quote (FILE *stream, const char *str);
>
> /* Currently open libguestfs handle. */
> guestfs_h *g;
> @@ -362,9 +363,6 @@ main (int argc, char *argv[])
>
> /* Inspector mode invalidates most of the other arguments. */
> if (inspector) {
> - char cmd[1024];
> - int r;
> -
> if (drvs || mps || remote_control_listen || remote_control ||
> guestfs_get_selinux (g)) {
> fprintf (stderr, _("%s: cannot use -i option with -a, -m, -N, "
> @@ -379,44 +377,90 @@ main (int argc, char *argv[])
> exit (EXIT_FAILURE);
> }
>
> - strcpy (cmd, "a=`virt-inspector");
> + char *cmd;
> + size_t cmdlen;
> + FILE *fp = open_memstream (&cmd, &cmdlen);
> + if (fp == NULL) {
> + perror ("open_memstream");
> + exit (EXIT_FAILURE);
> + }
> +
> + //fprintf (fp, "virt-inspector");
> + fprintf (fp, "inspector/run-inspector-locally");
I don't think ^^^ is still supposed to be there.
> while (optind < argc) {
> - if (strlen (cmd) + strlen (argv[optind]) + strlen (real_argv0) + 60
> - >= sizeof cmd) {
> - fprintf (stderr,
> - _("%s: virt-inspector command too long for fixed-size buffer\n"),
> - program_name);
> - exit (EXIT_FAILURE);
> - }
> - strcat (cmd, " '");
> - strcat (cmd, argv[optind]);
> - strcat (cmd, "'");
> + fputc (' ', fp);
> + print_shell_quote (fp, argv[optind]);
> optind++;
> }
>
> if (read_only)
> - strcat (cmd, " --ro-fish");
> + fprintf (fp, " --ro-fish");
> else
> - strcat (cmd, " --fish");
> + fprintf (fp, " --fish");
> +
> + if (fclose (fp) == -1) {
> + perror ("fclose");
> + exit (EXIT_FAILURE);
> + }
> +
> + if (verbose)
> + fprintf (stderr,
> + "%s -i: running: %s\n", program_name, cmd);
> +
> + FILE *pp = popen (cmd, "r");
> + if (pp == NULL) {
> + perror (cmd);
> + exit (EXIT_FAILURE);
> + }
> +
> + char *cmd2;
> + fp = open_memstream (&cmd2, &cmdlen);
> + if (fp == NULL) {
> + perror ("open_memstream");
> + exit (EXIT_FAILURE);
> + }
>
> - sprintf (&cmd[strlen(cmd)], "` && %s $a", real_argv0);
> + fprintf (fp, "%s", real_argv0);
>
> if (guestfs_get_verbose (g))
> - strcat (cmd, " -v");
> + fprintf (fp, " -v");
> if (!guestfs_get_autosync (g))
> - strcat (cmd, " -n");
> + fprintf (fp, " -n");
> if (guestfs_get_trace (g))
> - strcat (cmd, " -x");
> + fprintf (fp, " -x");
> +
> + char *insp = NULL;
> + size_t insplen;
> + if (getline (&insp, &insplen, pp) == -1) {
> + perror (cmd);
> + exit (EXIT_FAILURE);
> + }
> + fprintf (fp, " %s", insp);
> +
> + if (pclose (pp) == -1) {
> + perror (cmd);
> + exit (EXIT_FAILURE);
> + }
> +
> + if (fclose (fp) == -1) {
> + perror ("fclose");
> + exit (EXIT_FAILURE);
> + }
>
> if (verbose)
> fprintf (stderr,
> - "%s -i: running virt-inspector command:\n%s\n", program_name, cmd);
> + "%s -i: running: %s\n", program_name, cmd2);
>
> - r = system (cmd);
> + int r = system (cmd2);
Incidentally, why don't we just exec() here?
> if (r == -1) {
> - perror ("system");
> + perror (cmd2);
> exit (EXIT_FAILURE);
> }
> +
> + free (cmd);
> + free (cmd2);
> + free (insp);
> +
> exit (WEXITSTATUS (r));
> }
>
> @@ -1616,3 +1660,22 @@ file_out (const char *arg)
> }
> return ret;
> }
> +
> +static int
> +print_shell_quote (FILE *stream, const char *str)
> +{
> +#define SAFE(c) (c_isalnum((c)) || \
> + (c) == '/' || (c) == '-' || (c) == '_' || (c) == '.')
> + int i, len;
> +
> + for (i = len = 0; str[i]; ++i) {
> + if (!SAFE(str[i])) {
> + putc ('\\', stream);
> + len ++;
> + }
> + putc (str[i], stream);
> + len ++;
> + }
> +
> + return len;
> +}
The return value of print_shell_quote isn't used. No need to track len.
> diff --git a/inspector/virt-inspector b/inspector/virt-inspector
> index 4428ecd..f62d21f 100755
> --- a/inspector/virt-inspector
> +++ b/inspector/virt-inspector
> @@ -27,6 +27,7 @@ use Pod::Usage;
> use Getopt::Long;
> use Data::Dumper;
> use XML::Writer;
> +use String::ShellQuote qw(shell_quote);
String::ShellQuote isn't in RHEL 5. Are you really sure you want this?
> use Locale::TextDomain 'libguestfs';
>
> # Optional:
> @@ -298,13 +299,17 @@ if ($output eq "fish" || $output eq "ro-fish") {
> print "--ro ";
> }
>
> - print "-a $_ " foreach @images;
> + foreach (@images) {
> + printf "-a %s ", shell_quote ($_);
> + }
>
> my $mounts = $oses->{$root_dev}->{mounts};
> # Have to mount / first. Luckily '/' is early in the ASCII
> # character set, so this should be OK.
> foreach (sort keys %$mounts) {
> - print "-m $mounts->{$_}:$_ " if $_ ne "swap" && $_ ne "none";
> + if ($_ ne "swap" && $_ ne "none") {
> + printf "-m %s ", shell_quote ("$mounts->{$_}:$_");
> + }
> }
> print "\n"
> }
Matt
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team
M: +44 (0)7977 267231
GPG ID: D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
More information about the Libguestfs
mailing list