Libguestfs - Feb 2016 - [PATCH 1/2] daemon: glob: do not return directories with trailing slash

Do not pass GLOB_MARK as flag for glob(3) in the daemon implementation
of glob, so names of directories will not have a trailing slash.
This allows users to have filenames that can be used with other tools,
such as rm.  Add a new test to check this (based on RHBZ#1293271).

A mild behaviour change is that users of the glob API now need to append
the slash when building paths using its results.  The test-glob.sh test
of guestfish is adapted to this.

Related to RHBZ#1293271.
---
 daemon/glob.c                    |  2 +-
 fish/test-glob.sh                |  2 +-
 tests/regressions/Makefile.am    |  2 ++
 tests/regressions/rhbz1293271.sh | 70 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100755 tests/regressions/rhbz1293271.sh

diff --git a/daemon/glob.c b/daemon/glob.c
index 45fb30f..0e646bd 100644
--- a/daemon/glob.c
+++ b/daemon/glob.c
@@ -33,7 +33,7 @@ do_glob_expand (const char *pattern)
 
   /* glob(3) in glibc never calls chdir, so this seems to be safe: */
   CHROOT_IN;
-  r = glob (pattern, GLOB_MARK|GLOB_BRACE, NULL, &buf);
+  r = glob (pattern, GLOB_BRACE, NULL, &buf);
   CHROOT_OUT;
 
   if (r == GLOB_NOMATCH) {	/* Return an empty list instead of an error. */
diff --git a/fish/test-glob.sh b/fish/test-glob.sh
index c520319..854d898 100755
--- a/fish/test-glob.sh
+++ b/fish/test-glob.sh
@@ -64,7 +64,7 @@ echo end
 EOF
 
 if [ "$(cat test-glob.out)" != "files
-/foo/
+/foo
 /foo/bar1
 /foo/bar2
 /foo/not*
diff --git a/tests/regressions/Makefile.am b/tests/regressions/Makefile.am
index 3aae57b..27ad118 100644
--- a/tests/regressions/Makefile.am
+++ b/tests/regressions/Makefile.am
@@ -48,6 +48,7 @@ EXTRA_DIST = \
 	rhbz1232192.sh \
 	rhbz1232192.xml \
 	rhbz1285847.sh \
+	rhbz1293271.sh \
 	test-noexec-stack.pl
 
 TESTS = \
@@ -77,6 +78,7 @@ TESTS = \
 	rhbz1175196.sh \
 	rhbz1232192.sh \
 	rhbz1285847.sh \
+	rhbz1293271.sh \
 	test-big-heap \
 	test-noexec-stack.pl
 
diff --git a/tests/regressions/rhbz1293271.sh b/tests/regressions/rhbz1293271.sh
new file mode 100755
index 0000000..bbd334e
--- /dev/null
+++ b/tests/regressions/rhbz1293271.sh
@@ -0,0 +1,70 @@
+#!/bin/bash -
+# libguestfs
+# Copyright (C) 2016 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
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Regression test for:
+# https://bugzilla.redhat.com/show_bug.cgi?id=1293271
+# rm-rf does not remove symlinks to directories
+
+set -e
+export LANG=C
+
+if [ -n "$SKIP_TEST_RHBZ1293271_SH" ]; then
+    echo "$0: test skipped because environment variable is set."
+    exit 77
+fi
+
+rm -f rhbz1293271.img rhbz1293271.out
+
+guestfish -N rhbz1293271.img=fs -m /dev/sda1 > rhbz1293271.out <<EOF
+touch /hello
+touch /test-file
+touch /world
+mkdir /somedir
+touch /somedir/file
+ln-s somedir /test-link
+
+ls / | sort
+
+echo -----
+
+glob rm-rf /*test*
+
+ls / | sort
+
+echo ----- END
+
+EOF
+
+if [ "$(cat rhbz1293271.out)" != "hello
+lost+found
+somedir
+test-file
+test-link
+world
+-----
+hello
+lost+found
+somedir
+world
+----- END" ]; then
+    echo "$0: unexpected output:"
+    cat rhbz1293271.out
+    exit 1
+fi
+
+rm rhbz1293271.img rhbz1293271.out
-- 
2.5.0

Support globbing in paths passed to --delete, now that glob will return
paths usable for rm.

This re-adds back globbing for --delete in virt-sysprep, which was
available before the integration with common code from virt-customize.
---
 customize/customize_run.ml | 2 +-
 generator/customize.ml     | 5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/customize/customize_run.ml b/customize/customize_run.ml
index 84f634b..5c1587c 100644
--- a/customize/customize_run.ml
+++ b/customize/customize_run.ml
@@ -191,7 +191,7 @@ exec >>%s 2>&1
 
     | `Delete path ->
       message (f_"Deleting: %s") path;
-      g#rm_rf path
+      Array.iter g#rm_rf (g#glob_expand path)
 
     | `Edit (path, expr) ->
       message (f_"Editing: %s") path;
diff --git a/generator/customize.ml b/generator/customize.ml
index 19aec81..5d9a19b 100644
--- a/generator/customize.ml
+++ b/generator/customize.ml
@@ -115,6 +115,11 @@ Wildcards cannot be used.";
 Delete a file from the guest.  Or delete a directory (and all its
 contents, recursively).
 
+You can use shell glob characters in the specified path; note that such
+metacharacters might require proper escape.  For example:
+
+ virt-customize --delete '/var/log/*.log'.
+
 See also: I<--upload>, I<--scrub>.";
   };
 
-- 
2.5.0

On Wed, Feb 03, 2016 at 04:31:36PM +0100, Pino Toscano
wrote:
> Do not pass GLOB_MARK as flag for glob(3) in the daemon implementation
> of glob, so names of directories will not have a trailing slash.
> This allows users to have filenames that can be used with other tools,
> such as rm.  Add a new test to check this (based on RHBZ#1293271).
> 
> A mild behaviour change is that users of the glob API now need to append
> the slash when building paths using its results.  The test-glob.sh test
> of guestfish is adapted to this.
I don't think we can change the API behaviour like this, particularly
since it has demonstrable effects even for our own callers.

How about adding an OBool "mark" flag which controls whether to pass
the GLOB_MARK flag?  Also remember to set once_had_no_optargs = true.

Rich.
> Related to RHBZ#1293271.
> ---
>  daemon/glob.c                    |  2 +-
>  fish/test-glob.sh                |  2 +-
>  tests/regressions/Makefile.am    |  2 ++
>  tests/regressions/rhbz1293271.sh | 70
++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 74 insertions(+), 2 deletions(-)
>  create mode 100755 tests/regressions/rhbz1293271.sh
> 
> diff --git a/daemon/glob.c b/daemon/glob.c
> index 45fb30f..0e646bd 100644
> --- a/daemon/glob.c
> +++ b/daemon/glob.c
> @@ -33,7 +33,7 @@ do_glob_expand (const char *pattern)
>  
>    /* glob(3) in glibc never calls chdir, so this seems to be safe: */
>    CHROOT_IN;
> -  r = glob (pattern, GLOB_MARK|GLOB_BRACE, NULL, &buf);
> +  r = glob (pattern, GLOB_BRACE, NULL, &buf);
>    CHROOT_OUT;
>  
>    if (r == GLOB_NOMATCH) {	/* Return an empty list instead of an error. */
> diff --git a/fish/test-glob.sh b/fish/test-glob.sh
> index c520319..854d898 100755
> --- a/fish/test-glob.sh
> +++ b/fish/test-glob.sh
> @@ -64,7 +64,7 @@ echo end
>  EOF
>  
>  if [ "$(cat test-glob.out)" != "files
> -/foo/
> +/foo
>  /foo/bar1
>  /foo/bar2
>  /foo/not*
> diff --git a/tests/regressions/Makefile.am b/tests/regressions/Makefile.am
> index 3aae57b..27ad118 100644
> --- a/tests/regressions/Makefile.am
> +++ b/tests/regressions/Makefile.am
> @@ -48,6 +48,7 @@ EXTRA_DIST = \
>  	rhbz1232192.sh \
>  	rhbz1232192.xml \
>  	rhbz1285847.sh \
> +	rhbz1293271.sh \
>  	test-noexec-stack.pl
>  
>  TESTS = \
> @@ -77,6 +78,7 @@ TESTS = \
>  	rhbz1175196.sh \
>  	rhbz1232192.sh \
>  	rhbz1285847.sh \
> +	rhbz1293271.sh \
>  	test-big-heap \
>  	test-noexec-stack.pl
>  
> diff --git a/tests/regressions/rhbz1293271.sh
b/tests/regressions/rhbz1293271.sh
> new file mode 100755
> index 0000000..bbd334e
> --- /dev/null
> +++ b/tests/regressions/rhbz1293271.sh
> @@ -0,0 +1,70 @@
> +#!/bin/bash -
> +# libguestfs
> +# Copyright (C) 2016 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
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
USA.
> +
> +# Regression test for:
> +# https://bugzilla.redhat.com/show_bug.cgi?id=1293271
> +# rm-rf does not remove symlinks to directories
> +
> +set -e
> +export LANG=C
> +
> +if [ -n "$SKIP_TEST_RHBZ1293271_SH" ]; then
> +    echo "$0: test skipped because environment variable is set."
> +    exit 77
> +fi
> +
> +rm -f rhbz1293271.img rhbz1293271.out
> +
> +guestfish -N rhbz1293271.img=fs -m /dev/sda1 > rhbz1293271.out
<<EOF
> +touch /hello
> +touch /test-file
> +touch /world
> +mkdir /somedir
> +touch /somedir/file
> +ln-s somedir /test-link
> +
> +ls / | sort
> +
> +echo -----
> +
> +glob rm-rf /*test*
> +
> +ls / | sort
> +
> +echo ----- END
> +
> +EOF
> +
> +if [ "$(cat rhbz1293271.out)" != "hello
> +lost+found
> +somedir
> +test-file
> +test-link
> +world
> +-----
> +hello
> +lost+found
> +somedir
> +world
> +----- END" ]; then
> +    echo "$0: unexpected output:"
> +    cat rhbz1293271.out
> +    exit 1
> +fi
> +
> +rm rhbz1293271.img rhbz1293271.out
> -- 
> 2.5.0
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/