Merge branch 'dont-rm-rf-root' into 'main'

gtestutils: Don't follow symlinks when deleting tests' tempdir

Closes #3290

See merge request GNOME/glib!4018

glib/gtestutils.c

@@ -30,6 +30,9 @@
 #include <fcntl.h>
 #include <unistd.h>
 #endif
+#ifdef HAVE_FTW_H
+#include <ftw.h>
+#endif
 #include <string.h>
 #include <stdlib.h>
 #include <stdio.h>
@@ -1407,6 +1410,61 @@ parse_args (gint    *argc_p,
   *argc_p = e;
 }
 
+#ifdef HAVE_FTW_H
+static int
+rm_rf_nftw_visitor (const char *fpath,
+                    const struct stat *sb,
+                    int typeflag,
+                    struct FTW *ftwbuf)
+{
+  switch (typeflag)
+    {
+    case FTW_DP:
+    case FTW_D:
+    case FTW_DNR:
+      if (g_rmdir (fpath) != 0)
+        {
+          int errsv = errno;
+          g_printerr ("Unable to clean up temporary directory %s: %s\n",
+                      fpath,
+                      g_strerror (errsv));
+        }
+      break;
+
+    default:
+      if (g_remove (fpath) != 0)
+        {
+          int errsv = errno;
+          g_printerr ("Unable to clean up temporary file %s: %s\n",
+                      fpath,
+                      g_strerror (errsv));
+        }
+      break;
+    }
+
+  return 0;
+}
+
+static void
+rm_rf (const gchar *path)
+{
+  /* nopenfd specifies the maximum number of directories that [n]ftw() will
+   * hold open simultaneously. Rather than attempt to determine how many file
+   * descriptors are available, we assume that 5 are available when tearing
+   * down a test case; if that assumption is invalid, the only harm is leaving
+   * a temporary directory on disk.
+   */
+  const int nopenfd = 5;
+  int ret = nftw (path, rm_rf_nftw_visitor, nopenfd, FTW_DEPTH | FTW_MOUNT | FTW_PHYS);
+  if (ret != 0)
+    {
+      int errsv = errno;
+      g_printerr ("Unable to clean up temporary directory %s: %s\n",
+                  path,
+                  g_strerror (errsv));
+    }
+}
+#else
 /* A fairly naive `rm -rf` implementation to clean up after unit tests. */
 static void
 rm_rf (const gchar *path)
@@ -1433,6 +1491,7 @@ rm_rf (const gchar *path)
 
   g_rmdir (path);
 }
+#endif
 
 /* Implement the %G_TEST_OPTION_ISOLATE_DIRS option, iff it’s enabled. Create
  * a temporary directory for this unit test (disambiguated using @test_run_name)

glib/tests/utils-isolated.c

@@ -21,6 +21,7 @@
 #include "config.h"
 
 #include <glib/glib.h>
+#include <glib/gstdio.h>
 
 /* Test that all of the well-known directories returned by GLib
  * are returned as children of test_tmpdir when running with
@@ -90,6 +91,96 @@ test_user_runtime_dir (void)
   g_assert_true (g_str_has_prefix (g_get_user_runtime_dir (), test_tmpdir));
 }
 
+static void
+test_cleanup_handles_errors (void)
+{
+  const gchar *runtime_dir = g_get_user_runtime_dir ();
+  gchar *subdir = g_build_filename (runtime_dir, "b", NULL);
+
+  if (g_test_subprocess ())
+    {
+
+      g_assert_no_errno (g_mkdir_with_parents (subdir, 0755));
+      g_assert_no_errno (g_chmod (runtime_dir, 0));
+
+      g_clear_pointer (&subdir, g_free);
+      /* Now let the harness clean up. Not being able to delete part of the
+       * test's isolated temporary directory should not cause the test to
+       * fail.
+       */
+      return;
+    }
+
+  g_test_trap_subprocess (NULL, 0, G_TEST_SUBPROCESS_INHERIT_STDERR);
+  g_test_trap_assert_passed ();
+  /* No assertion about the test logging anything to stderr: we don't
+   * guarantee this, and one of the cleanup implementations doesn't log
+   * anything.
+   */
+
+  /* Now that we have verified that a failure to delete part of the isolated
+   * temporary directory hierarchy does not cause the test to fail, clean up
+   * after ourselves.
+   */
+  g_assert_no_errno (g_chmod (runtime_dir, 0755));
+
+  g_free (subdir);
+}
+
+static void
+test_cleanup_doesnt_follow_symlinks (void)
+{
+#ifdef G_OS_WIN32
+  g_test_skip ("Symlinks not generally available on Windows");
+#else
+  const gchar *test_tmpdir = g_getenv ("G_TEST_TMPDIR");
+  const gchar *runtime_dir = g_get_user_runtime_dir ();
+  g_assert_cmpstr (test_tmpdir, !=, runtime_dir);
+  g_assert_true (g_str_has_prefix (runtime_dir, test_tmpdir));
+  gchar *symlink_path = g_build_filename (runtime_dir, "symlink", NULL);
+  gchar *target_path = g_build_filename (test_tmpdir, "target", NULL);
+  gchar *file_within_target = g_build_filename (target_path, "precious-data", NULL);
+
+  if (g_test_subprocess ())
+    {
+      g_assert_no_errno (g_mkdir_with_parents (runtime_dir, 0755));
+      g_assert_no_errno (symlink (target_path, symlink_path));
+
+      g_free (symlink_path);
+      g_free (target_path);
+      g_free (file_within_target);
+
+      return;
+    }
+  else
+    {
+      GError *error = NULL;
+
+      g_assert_no_errno (g_mkdir_with_parents (target_path, 0755));
+      g_file_set_contents (file_within_target, "Precious Data", -1, &error);
+      g_assert_no_error (error);
+
+      g_test_trap_subprocess (NULL, 0, G_TEST_SUBPROCESS_INHERIT_STDERR);
+      g_test_trap_assert_passed ();
+
+      /* There was a symbolic link in the test's isolated directory which
+       * pointed to a directory outside it. That directory and its contents
+       * should not have been deleted: the symbolic link should not have been
+       * followed.
+       */
+      g_assert_true (g_file_test (file_within_target, G_FILE_TEST_EXISTS));
+      g_assert_true (g_file_test (target_path, G_FILE_TEST_IS_DIR));
+
+      /* The symlink itself should have been deleted. */
+      g_assert_false (g_file_test (symlink_path, G_FILE_TEST_EXISTS));
+      g_assert_false (g_file_test (symlink_path, G_FILE_TEST_IS_SYMLINK));
+
+      g_free (symlink_path);
+      g_free (target_path);
+      g_free (file_within_target);
+    }
+#endif
+}
 
 int
 main (int   argc,
@@ -110,5 +201,7 @@ main (int   argc,
   g_test_add_func ("/utils-isolated/user-data-dir", test_user_data_dir);
   g_test_add_func ("/utils-isolated/user-state-dir", test_user_state_dir);
   g_test_add_func ("/utils-isolated/user-runtime-dir", test_user_runtime_dir);
+  g_test_add_func ("/utils-isolated/cleanup/handles-errors", test_cleanup_handles_errors);
+  g_test_add_func ("/utils-isolated/cleanup/doesnt-follow-symlinks", test_cleanup_doesnt_follow_symlinks);
   return g_test_run ();
 }

meson.build

@@ -385,6 +385,7 @@ headers = [
   'dirent.h', # MSC does not come with this by default
   'float.h',
   'fstab.h',
+  'ftw.h',
   'grp.h',
   'inttypes.h',
   'libproc.h',